From b1ff02348782d7493f97162cac4a2be15ed29cef Mon Sep 17 00:00:00 2001 From: leynos Date: Fri, 30 Oct 2026 18:57:01 +0100 Subject: [PATCH 01/16] Record release policy record-boundary decision Issue #964 asks whether release policy should decide from typed records instead of mappings. Inventory the seams on main and adopt frozen slotted records for the two external record shapes: staging targets and the trusted digest record. Domain-shaped name-to-value mappings stay mappings. Add ADR-0068, index it, and link it from the release design section, roadmap item 9.1.1, the developers' guide, and the trusted-packaging decision log. Add the issue ExecPlan with the inventory, test classification, migration cost, and decision criteria. No production behaviour changes in this commit. Co-Authored-By: Claude Code --- ...d-records-for-release-policy-boundaries.md | 166 +++++++++++ docs/contents.md | 3 + docs/developers-guide.md | 31 ++- ...d-records-for-release-policy-boundaries.md | 261 ++++++++++++++++++ docs/execplans/use-trusted-packaging.md | 15 + docs/netsuke-design.md | 9 +- docs/roadmap.md | 3 + 7 files changed, 478 insertions(+), 10 deletions(-) create mode 100644 docs/adrs/adr-0068-typed-records-for-release-policy-boundaries.md create mode 100644 docs/execplans/issue-964-evaluate-typed-records-for-release-policy-boundaries.md diff --git a/docs/adrs/adr-0068-typed-records-for-release-policy-boundaries.md b/docs/adrs/adr-0068-typed-records-for-release-policy-boundaries.md new file mode 100644 index 000000000..9ef312cf9 --- /dev/null +++ b/docs/adrs/adr-0068-typed-records-for-release-policy-boundaries.md @@ -0,0 +1,166 @@ +# Architectural decision record (ADR) 0068: Typed records at release policy boundaries + +## Status + +Accepted on 2026-10-30. Release policy decides from frozen, slotted records at +every boundary where an external record shape reaches it; mappings remain only +where the policy decides over a domain-shaped name-to-value relation that no +external format defines. + +## Date + +2026-10-30 + +## Context and Problem Statement + +Pull request [#903](https://github.com/leynos/netsuke/pull/903) moved release +publication onto a set of Python policy modules and adapters. Its policy and +adapter split is enforced by `tests/workflow_contracts/`, which rejects +network, filesystem, serialization, HTTP, and provider-format effects in the +modules listed as policy. CodeRabbit's Domain Architecture review of that pull +request found one remaining leak: `scripts/release_declaration.py` interpreted +the TOML-shaped `{"targets": ...}` mapping, read the `platform` member, and +raised configuration errors from inside a policy module. The review asked for +domain-shaped types such as `StagingTargets`, `ReleaseIdentityRecord`, and +`DigestRecord`, with TOML and JSON adapters owning field names, shape checks, +decoding, encoding, and malformed-record errors. + +The maintainer accepted the finding as valid but deferred it out of #903 so +that the trusted-publishing work could land. The deferral is recorded in the +decision log of the +[trusted packaging ExecPlan](../execplans/use-trusted-packaging.md), and this +issue is its follow-up. + +The code already contains the pattern the review asked for. +`release_candidate.ReleaseIdentity`, `release_inventory.PublishedAsset`, the +`release_reconcile_crates` registry observations, and every record under +`.github/scripts/_release_admission/` are frozen slotted dataclasses. The +question is therefore not whether the repository has a record convention, but +where the convention should stop: which remaining mappings are external record +shapes that an adapter should decode, and which are domain values that policy +may legitimately decide over. + +## Decision Drivers + +- Keep release identity exact. Commit identity stays an exact string + comparison, and tag/version agreement is not relaxed. +- Keep inventory checks exhaustive. Every declared, trusted, and downloaded + asset remains compared, including the empty-set rejections. +- Give each external record shape exactly one decoding location, owned by an + adapter or CLI that already handles its serialization. +- Preserve every existing rejection category and its type, so operators keep + the same diagnosis and the same exit status. +- Remove the duplicated staging-config decode without weakening the check that + refuses a malformed `targets` table before discovery iterates it. +- Keep the migration bounded, so the change is reviewable and its test + migrations are mechanical. +- Do not weaken the `POLICY_MODULES` boundary contract. Policy modules stay + network-free and free of filesystem and serialization effects. + +## Options considered + +### Option A: retain the mapping boundary + +Policy keeps reading mappings as it does today. The cost is zero now, and the +boundary remains as #903 left it. The cost is permanent: `assets`, `targets`, +and `platform` stay policy vocabulary, the staging configuration is decoded +twice, and a "validated mapping" is a call-order convention rather than a type +obligation. A future caller can pass an unvalidated mapping to a digest +comparison and skip the digest-format check entirely. + +### Option B: decode at the edge into frozen slotted records + +Adapters and CLIs decode each external shape into a frozen slotted record, and +policy functions accept the record. Field names, shape checks, and malformed +record errors move to the adapter that already owns the file or stream. Policy +keeps deciding what the record means: identity agreement, inventory coverage, +and digest comparison. + +This matches the repository's established value-object convention and satisfies +the boundary contract without changing what any refusal protects. + +### Option C: `TypedDict` only + +A `TypedDict` is a static annotation. It has no runtime validation, no single +decoding location, and no way to stop an unvalidated mapping from reaching a +comparison. It would satisfy a type checker while leaving every runtime gap in +Option A in place, and it would add a second, weaker convention beside the +frozen dataclasses already in use. + +| Criterion | Option A | Option B | Option C | +| ---------------------------------------- | ------------------ | -------------------- | -------- | +| One decoding location per external shape | No | Yes | No | +| Runtime rejection preserved | Yes, by convention | Yes, by construction | No | +| Removes the staging double decode | No | Yes | No | +| Type-enforced validated record | No | Yes | No | +| Fixture migration | None | Bounded | None | +| Consistent with existing records | Partly | Yes | No | + +Table 1: Comparison of the three boundary options against the decision criteria. + +## Decision Outcome + +Adopt Option B, narrowed to the external-shape seams. `TypedDict` is rejected +as a static-only mechanism that leaves the runtime gap in place. + +The adopted scope is: + +- `release_declaration` accepts a `StagingTargets` record built with + `StagingTarget` entries. The TOML walk, including the `targets` and + `platform` member names, moves to `release_declaration_cli`, which decodes + the configuration once and passes the record to both existing checks. +- `release_digest_record` accepts a `DigestRecord` for the trusted side of its + comparisons. The JSON `assets` walk moves to `release_digest_record_files`, + which already owns the file and its serialization. +- `release_identity_record` is reclassified from policy to adapter in the + boundary contract. Its field names are an external record shape, and no + production caller currently depends on it being policy. + +The following remain mappings because they are domain values rather than +external record shapes: + +- `release_reconcile_github.plan_asset_actions` takes name-to-digest mappings + produced by the declaration and by GitHub's listing adapter. Neither is a + provider payload by the time the policy sees it. +- The computed-downloads side of `require_downloads_match_record` is derived by + the verifier from bytes it hashed. Only the trusted side is a decoded + external record, so the asymmetry records provenance. +- `PublishedAsset`, `ReleaseIdentity`, and the registry observations are + already frozen slotted records and keep their current shape. + +Every existing error type is retained: `MissingTargetsError`, +`MalformedTargetError` with `MalformedTargetKind`, `DigestRecordError`, and +`RecordedDigestMismatchError`. The migration moves where they are raised, not +what they mean. + +## Consequences + +- Each external record shape has one decoding location, and a policy function + can no longer receive an undecoded staging table or digest payload through + its normal path. +- The staging configuration is decoded once per invocation. The pre-check that + refuses a malformed table before discovery runs becomes a decoded value + rather than a second pass over the same object. +- `release_identity_record` appears in `ADAPTER_MODULES` and the boundary + documentation describes it as a decoder. Its behaviour and error types do not + change. +- Payload-shape tests move from the policy tests to the decoder tests. + Policy tests construct records directly, so they test the decision rather + than the decoding. +- Adding a new external record shape now requires a decoder at the adapter and + a record beside the policy that consumes it. The record's constructor owns + the invariant, and the adapter owns the wire names. +- The containment is deliberate: domain-shaped mappings remain, so the + decision does not turn every name-to-value relation into a class. A future + change that makes one of those mappings an external payload must add a + decoder at that boundary. + +## References + +- [Issue #964: Evaluate typed records for release policy boundaries](https://github.com/leynos/netsuke/issues/964) +- [Pull request #903](https://github.com/leynos/netsuke/pull/903) +- [CodeRabbit Domain Architecture finding](https://github.com/leynos/netsuke/pull/903#discussion_r6448190491) +- [Trusted packaging ExecPlan, decision log](../execplans/use-trusted-packaging.md) +- [RFC 0005: Harden release integrity and admission](../rfcs/rfc-0005-harden-release-integrity-and-admission.md) +- [ADR-0020: Bounded release admission observability](adr-0020-bounded-release-admission-observability.md) +- [Developers' guide: release policy and adapter boundaries](../developers-guide.md#release-publication) diff --git a/docs/contents.md b/docs/contents.md index c94c310a9..76fc51017 100644 --- a/docs/contents.md +++ b/docs/contents.md @@ -318,6 +318,9 @@ operator, user, and contributor references are easier to find. moving package verification into `make test-package-verify`. - [ADR-0060](adrs/adr-0060-readable-inline-recipe-progress.md): Bounded, localized Ninja progress labels for inline recipe edges. +- [ADR-0068](adrs/adr-0068-typed-records-for-release-policy-boundaries.md): + Frozen slotted records at release policy boundaries, with adapters owning + external record shapes and domain-shaped mappings retained. - [ADR-0069](adrs/adr-0069-split-the-localization-key-tables.md): Splitting the Fluent key tables by audience under Whitaker's module line cap, while the build audit and `keys::ALL_KEYS` keep seeing one key set. diff --git a/docs/developers-guide.md b/docs/developers-guide.md index 7675ad629..4ca3dc648 100644 --- a/docs/developers-guide.md +++ b/docs/developers-guide.md @@ -2654,18 +2654,21 @@ or interpret provider-specific digest strings. The surrounding modules own those operations at the file or transport boundary: - `release_candidate` exposes a stable mapping; `release_candidate_cli` - serializes it, while `release_identity_record` validates a decoded mapping. + serializes it, while `release_identity_record` decodes a recorded identity + into `ReleaseIdentity` at the adapter boundary. - `release_digest` parses and compares sidecar text and digest strings. `release_digest_files` hashes files and reads sidecars; the package CLI, staged-digest collector, inventory verifier, and crates.io adapter are its only production callers. `release_staged_digests` scans the staged tree and hashes exactly the declared names for `release_declaration_cli`. -- `release_digest_record` validates in-memory digest mappings. - `release_digest_record_files` owns JSON and path errors; declaration writes - through it, and inventory verification and finalization read through it. -- `release_declaration` derives names from parsed target facts and staged - basenames. `release_declaration_cli` reads TOML, asks the hoist adapter for - archive names, scans the staged tree, and passes those observations in. +- `release_digest_record` compares `DigestRecord` values and keeps the + record's rejection categories. `release_digest_record_files` owns JSON, the + `assets` member, and path errors; declaration writes through it, and + inventory verification and finalization read through it. +- `release_declaration` derives names from a `StagingTargets` record and + staged basenames. `release_declaration_cli` reads TOML, decodes the `targets` + table once, asks the hoist adapter for archive names, scans the staged tree, + and passes those observations in. - `release_inventory` checks `PublishedAsset` names and sizes. Flat asset names are checked under both POSIX and Windows path rules; separators, drive-qualified names, traversal, and nested paths are refused. @@ -2678,6 +2681,20 @@ those operations at the file or transport boundary: - `finalize_github_release_cli` normalizes GitHub's `sha256:` digest prefix; `release_reconcile_github` receives bare digests and only plans actions. +External record shapes cross the boundary exactly once. A module that already +owns a file, stream, or provider response decodes its field names and shape +into a frozen slotted dataclass, and policy functions accept that record. The +record's constructor owns the value invariant, including any normalization such +as lowercasing a digest, and the adapter owns the wire names, including +`targets`, `platform`, and `assets`. `StagingTargets`, `DigestRecord`, +`ReleaseIdentity`, and `PublishedAsset` follow this rule. A mapping remains +acceptable when the policy decides over a domain-shaped name-to-value relation +that no external format defines, such as the name-to-digest plans in +`release_reconcile_github` and the computed side of a download comparison; +adding a decoder for one of those would invent a format boundary that does not +exist. See +[ADR-0068](adrs/adr-0068-typed-records-for-release-policy-boundaries.md). + Each adapter has a narrow scope and named callers. File adapters do not own release policy, response adapters do not choose whether a run publishes, and policy modules do not grow convenience access to these boundaries. Composition diff --git a/docs/execplans/issue-964-evaluate-typed-records-for-release-policy-boundaries.md b/docs/execplans/issue-964-evaluate-typed-records-for-release-policy-boundaries.md new file mode 100644 index 000000000..6f8b8e568 --- /dev/null +++ b/docs/execplans/issue-964-evaluate-typed-records-for-release-policy-boundaries.md @@ -0,0 +1,261 @@ +# Issue 964: Evaluate typed records for release policy boundaries + +Status: DRAFT + +This plan is in its evidence-gathering phase. The inventory and decision +criteria below are complete; the architectural decision record, the migration, +and the documentation updates are not yet started. No production code has +changed for this issue. + +## Purpose + +Issue [#964](https://github.com/leynos/netsuke/issues/964) asks whether the +release-publication policy should decide from frozen, slotted records instead +of the mappings it currently reads. The question follows a CodeRabbit Domain +Architecture finding on pull request +[#903](https://github.com/leynos/netsuke/pull/903), which observed that +`scripts/release_declaration.py` interprets the TOML-shaped `{"targets": ...}` +mapping and raises configuration errors from inside a policy module. The +maintainer deferred the refactor out of #903 and recorded the deferral in the +trusted-packaging decision log. + +This plan answers four questions with evidence from `main` at commit +`0a3104d44` (the merge commit for #903): + +1. Where does external record shape currently reach release policy? +2. Which of those checks protect release identity or artefact inventory, and so + must survive unchanged? +3. What would migrating the affected seams to typed records cost in call sites, + adapters, and tests? +4. Does the repository's existing evidence meet the decision criteria for + adoption, and if so, what is the narrowest useful scope? + +The evaluation is deliberately bounded. It excludes +`.github/scripts/_release_admission/`, which already uses frozen slotted +records and is the stated reference convention rather than a candidate for +change. It also excludes the `fresh` freshness contradiction and the +trusted-publishing Stage C and Stage D work, both of which remain with their +own issues. + +## Constraints + +- No production behaviour changes in the evaluation phase. The inventory and + the decision record are documentation until the migration stage begins. +- Release identity checks stay exact. Commit identity remains an exact string + comparison, and tag/version agreement is not relaxed. +- Inventory checks stay exhaustive. The declared set, the trusted record, and + the downloaded set keep their current rejections, including the empty-set + cases. +- `.github/scripts/_release_admission/` is not modified. +- `tests/workflow_contracts/release_publish_path_*.py` is not modified. +- Trusted-publishing Stage C stays partial and Stage D stays open. This issue + neither claims nor advances them. +- Policy modules stay network-free and free of filesystem, serialization, and + provider-format effects, as the existing `POLICY_MODULES` contract requires. +- Python remains the 3.14 baseline, and new code follows the repository's + frozen slotted dataclass convention for value objects. + +## Evidence base and method + +The inventory was produced by reading every release source named in +`tests/workflow_contracts/release_publication_modules.py` and by tracing each +policy function to its production callers with `grep -n` over `scripts/`. Test +fixtures were classified by reading each test and asking whether the mapping +under test is (a) an external payload the adapter decodes, or (b) a +domain-shaped value the policy legitimately decides over. + +Counts below name the command or the file set they came from. They are census +results for this commit, not invariants. A later commit that adds a call site +invalidates the number, not the conclusion, and the plan records the command so +the census can be repeated: + +```sh +grep -rn "platform_target_counts\|declared_asset_names\|validate_digest_record\ +\|require_record_matches_declaration\|require_downloads_match_record\ +\|identity_from_mapping\|plan_asset_actions\|validate_inventory" \ + --include=*.py scripts +``` + +## Inventory of policy seams + +| Seam | Interface | External shape | Producing adapter | Production callers | Validation site | Kind | Notes | +| ----------------- | ----------------------------------------------------------------------------------------------------------------- | ----------------------------------------------------------------------------- | ------------------------------------------------ | ----------------------------------- | ----------------------------------------- | ------------------ | ------------------------------------------------------------------------------------- | +| Staging targets | `release_declaration.platform_target_counts(target_config: object) -> dict[str, int]` | TOML `{"targets": {name: {"platform": str}}}` | `release_declaration_cli` (`tomllib.loads`) | `release_declaration_cli:300` | Policy, via `_mapping` and `_platform_of` | Artefact inventory | Decoded twice: once as a pre-check and again inside `declared_asset_names` | +| Staging targets | `release_declaration.declared_asset_names(*, archive_names, target_config: object, staged_package_names)` | Same TOML mapping | `release_declaration_cli` | `release_declaration_cli:308` | Policy | Artefact inventory | Re-runs `platform_target_counts`, so a malformed config is rejected twice | +| Release identity | `release_identity_record.identity_from_mapping(record: Mapping[str, object]) -> ReleaseIdentity` | Identity JSON `{tag, commit, package_name, version, bin_name, is_prerelease}` | None: the module is the decoder | None; tests only | Policy | Release identity | `ReleaseIdentity` and `CandidateFacts` already exist as frozen slotted records | +| Digest record | `release_digest_record.validate_digest_record(record: object) -> dict[str, str]` | JSON `{"assets": {name: digest}}` | `release_digest_record_files.read_digest_record` | `release_digest_record_files:60,81` | Policy | Artefact inventory | Owns the `assets` member name inside policy | +| Digest comparison | `release_digest_record.require_record_matches_declaration(digests: Mapping, *, expected)` | Domain mapping | `release_digest_record_files` | `verify_release_inventory_cli:113` | Convention only | Artefact inventory | Accepts any `Mapping`, so "validated" is not type-enforced | +| Digest comparison | `release_digest_record.require_downloads_match_record(computed: Mapping, *, trusted: Mapping)` | Domain mapping | `verify_release_inventory_cli` | `verify_release_inventory_cli:115` | Convention only | Artefact inventory | Same; the trusted side is the decoded record and the computed side is adapter-derived | +| GitHub assets | `release_reconcile_github.plan_asset_actions(*, expected: Mapping[str, str], present: Mapping[str, str \| None])` | Domain mapping of name to digest | `finalize_github_release_cli` | `finalize_github_release_cli:263` | Not applicable | Artefact inventory | Domain-shaped, not an external payload. Retain as a mapping. | +| Inventory | `release_inventory.validate_inventory(present: Iterable[PublishedAsset], *, expected)` | Already typed | `verify_release_inventory_cli` | `verify_release_inventory_cli:112` | `PublishedAsset` | Artefact inventory | Already a frozen slotted record. Retain. | +| Registry state | `release_reconcile_crates.classify_registry_state`, `plan_crates_action` | Already typed | `release_crates_io_adapter` | `publish_crates_io_cli` | Typed observations | Artefact inventory | Already frozen slotted records. Retain. | + +### Repeated validation and unvalidated reads + +- `release_declaration_cli` calls `platform_target_counts` as a pre-check and + then `declared_asset_names` decodes the same `config` object again. Both + calls can raise the same `MalformedTargetError`. The duplication exists so + the discovery adapter does not iterate an unvalidated `targets` table; the + pre-check is a workaround for the fact that the decoder is not a separate + step. +- `release_identity_record` coerces fields with `str(...)` before calling + `resolve_identity`. A non-string `tag` or `commit` is therefore normalized by + the policy decoder rather than rejected at the adapter. The `is_prerelease` + flag is the one field that is checked for type, because a string `"false"` + would otherwise be truthy. +- `require_record_matches_declaration` and `require_downloads_match_record` + declare `Mapping` parameters but perform no record-shape validation. A caller + can pass a mapping that never passed `validate_digest_record`; the digest + format check is then skipped. Every current caller does decode first, so this + is a latent gap rather than an observed defect. +- No policy function performs an unvalidated key read on an external payload + after this inventory: `release_declaration` validates before `.get`, and + `release_digest_record` validates before comparison. The gap is that the + validation is call-order convention, not a type obligation. + +## Test classification + +| Test file | Tests touching a mapping fixture | Classification | +| ---------------------------------------------------------- | ----------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `scripts/tests/test_release_declaration.py` | 10 | Mixed: 6 policy behaviour over a shared `TARGET_CONFIG`, 4 payload-shape cases (non-mapping config, missing `targets`, target without `platform`, unknown platform) | +| `scripts/tests/test_release_declaration_invalid_config.py` | 1 parametrized test, 2 payloads | Payload shape: raw TOML strings through the CLI | +| `scripts/tests/test_release_declaration_cli.py` | Behavioural fixtures via `release_staging_support` | Adapter, writes real TOML | +| `scripts/tests/test_release_identity_record.py` | 4 Hypothesis properties | Payload shape and policy behaviour: round trip, missing field, non-boolean flag, contradictory flag | +| `scripts/tests/test_release_candidate.py` | 1 round-trip assertion | Policy identity: proves `as_record` and `identity_from_mapping` agree | +| `scripts/tests/test_release_digest_record.py` | 14 | Mixed: 5 adapter tests (round trip, sorted, lowercased, malformed payloads, unreadable path) and 9 policy comparison tests | +| `scripts/tests/test_release_digest_record_files.py` | 3 | Adapter: round trip, unreadable path, JSON shape error | +| `scripts/tests/test_verify_release_inventory_command.py` | 2 fixtures plus one malformed trusted record | Adapter end-to-end; the malformed record is payload shape | +| `scripts/tests/test_verify_release_inventory_cli.py` | Trusted-record fixtures | Adapter | +| `scripts/tests/test_finalize_github_release_cli.py` | 6 fixture sites, 1 parametrized malformed-record test | Adapter; the malformed cases are payload shape | +| `scripts/tests/test_release_reconcile_github.py` | 9 | Policy behaviour over domain-shaped name-to-digest mappings. Retain as mappings. | + +## Migration cost for adopted option + +Scope is the two external-shape seams only: staging targets and the trusted +digest record. Identity reclassification moves a module between the contract +lists without moving code, because `ReleaseIdentity` already exists. + +| Item | Count | Detail | +| --------------------------------------------- | -------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| Policy functions whose signature changes | 4 | `platform_target_counts`, `declared_asset_names`, `require_record_matches_declaration`, `require_downloads_match_record` | +| Decoders moved to adapters | 2 | `platform_target_counts`'s mapping walk and `validate_digest_record`'s `assets` walk | +| Production adapters or CLIs edited | 4 | `release_declaration_cli`, `release_digest_record_files`, `verify_release_inventory_cli`, `finalize_github_release_cli` | +| Contract list edits | 1 | Reclassify `release_identity_record` from `POLICY_MODULES` to `ADAPTER_MODULES` | +| Test files edited | 7 | `test_release_declaration.py`, `test_release_declaration_invalid_config.py`, `test_release_identity_record.py`, `test_release_digest_record.py`, `test_release_digest_record_files.py`, `test_verify_release_inventory_command.py`, `test_finalize_github_release_cli.py` | +| Test cases whose fixture construction changes | about 24 | 10 declaration, 4 identity, 14 digest record (overlapping adapter and policy), plus the CLI payload-shape cases | +| New behaviour to implement | None | The migration is structural; every rejection category already exists | + +## Field-loss and normalization risks + +- `ReleaseIdentity.as_record()` sorts keys and emits exactly the six fields + the decoder requires. A record type must keep all six; dropping + `is_prerelease` would lose the only check that a recorded flag agrees with + the version. +- `validate_digest_record` lowercases digests. A record type that stores the + original case would change comparison behaviour for records written in + uppercase, so normalization stays at construction. +- `platform_target_counts` collapses target names into a platform histogram. + The target name itself is not used downstream, so a `StagingTarget` record + needs only the platform, not the table key. Recording the key as well would + add an unused field and invite comparisons that the policy does not make. +- The staging decoder currently accepts any `dict` with string keys, including + a mapping that has extra members beside `targets`. The decoder may keep + ignoring unknown members; rejecting them would be a behaviour change and + needs its own justification. +- `write_digest_record` currently accepts a plain mapping and validates it + internally. If it takes a `DigestRecord`, the producer in + `release_staged_digests` must build one, which would push record knowledge + into another adapter. Keeping the writer's public parameter a mapping and + constructing the record inside the adapter avoids that. + +## Decision criteria + +Adoption is justified only when all four hold: + +1. **One decoding location.** Each external record shape is decoded once, at a + named adapter or CLI, and policy receives a domain value. +2. **Preserved rejection categories.** Every existing refusal keeps its type + and its message semantics, and the tests that prove each refusal still pass. +3. **Fewer duplicated checks.** The staging-target double decode disappears, + and the digest comparison can no longer be called with an unvalidated + mapping through the normal path. +4. **Bounded fixture migration.** No more than roughly two dozen test cases + change construction, and no test loses a rejection it currently proves. + +## Options considered + +- **Option A: retain the mapping boundary.** Cost is zero now. The cost is + permanent: `assets`, `targets`, and `platform` stay policy vocabulary, the + staging config is decoded twice, and "validated mapping" stays a convention + rather than a type. This is the status quo recorded in the trusted-packaging + decision log. +- **Option B: decode at the edge into frozen slotted records.** Adapters and + CLIs own field names and shape checks; policy functions take `StagingTargets` + and `DigestRecord`. Cost is the table above. Benefit is criteria 1 through 3. + This matches `CandidateFacts`, `ReleaseIdentity`, `PublishedAsset`, + `RegistryVersion`, and `_release_admission/records.py`. +- **Option C: `TypedDict` only.** Rejected. A `TypedDict` is a static + annotation with no runtime validation, no single decoding location, and no + way to prevent an unvalidated mapping from reaching a comparison. It would + satisfy the type checker while leaving every runtime gap in place. + +The evidence supports **Option B, narrowed to the external-shape seams**. +Domain-shaped mappings are retained where the policy legitimately decides over +a name-to-value relation that no external format defines: `plan_asset_actions` +and the computed-downloads side of `require_downloads_match_record`. +`PublishedAsset`, `ReleaseIdentity`, and the registry observations are already +records and are unchanged. + +## Delivery plan + +1. Record the decision as ADR-0068, indexed from `docs/contents.md`, linked + from the release automation design section and roadmap task 9.1.1, and + appended to the trusted-packaging decision log. Add the boundary convention + to the developers' guide. +2. Add characterization tests for the staging and digest decoders at their + current seams, so the migration has a before-and-after oracle. +3. Introduce `StagingTarget` and `StagingTargets` beside + `release_package_formats`, and `DigestRecord` beside + `release_digest_record`, as frozen slotted dataclasses with validating + constructors. +4. Move the staging TOML walk and the digest `assets` walk into + `release_declaration_cli` and `release_digest_record_files`. Preserve + `MissingTargetsError`, `MalformedTargetError`, `MalformedTargetKind`, + `DigestRecordError`, and `RecordedDigestMismatchError` unchanged. +5. Change the four policy signatures, keep every identity and inventory check, + and remove the staging double decode. +6. Reclassify `release_identity_record` as an adapter in the contract lists and + update the boundary documentation. +7. Update the ExecPlan Progress, Outcomes, Status, and the roadmap checkbox. + +## Progress + +- [x] Inventory policy seams and production callers on `main` at `0a3104d44`. +- [x] Classify mapping fixtures as policy behaviour or payload shape. +- [x] Estimate Option B call-site, fixture, and adapter cost. +- [x] Allocate the next unreserved ADR number across remote branches and open + pull requests. ADR-0068 is free across all 77 remote heads and every + open pull request. +- [x] Write ADR-0068 and update the documentation index, design section, + roadmap item, developers' guide, and trusted-packaging decision log. +- [ ] Add characterization tests. +- [ ] Migrate the staging target and digest record seams. +- [ ] Reclassify the identity decoder and update the boundary contracts. +- [ ] Run the release-publication, workflow-contract, typecheck, lint, format, + Markdown, and spelling gates. +- [ ] Update this plan's Outcomes and Status. + +## Outcomes and retrospective + +Not yet written. This section will record the ADR number, the migration result, +any rejection that proved harder to preserve than expected, and the final gate +evidence. + +## References + +- [Issue #964](https://github.com/leynos/netsuke/issues/964) +- [Pull request #903](https://github.com/leynos/netsuke/pull/903) +- [CodeRabbit review comment 6448190491](https://github.com/leynos/netsuke/pull/903#discussion_r6448190491) +- [Trusted packaging ExecPlan](use-trusted-packaging.md) +- [RFC 0005: Harden release integrity and admission](../rfcs/rfc-0005-harden-release-integrity-and-admission.md) +- [ADR-0020: Bounded release admission observability](../adrs/adr-0020-bounded-release-admission-observability.md) diff --git a/docs/execplans/use-trusted-packaging.md b/docs/execplans/use-trusted-packaging.md index b610e94f4..ca1084854 100644 --- a/docs/execplans/use-trusted-packaging.md +++ b/docs/execplans/use-trusted-packaging.md @@ -3969,6 +3969,21 @@ Dependencies: none new. `rust-lang/crates-io-auth-action` at issue and does not change this PR's accepted boundary. Date/Author: 2026-10-28, maintainer instruction relayed by implementation agent. +- Decision: **decode external record shapes at the adapter and decide over + frozen slotted records** (2026-10-30). Issue + [#964](https://github.com/leynos/netsuke/issues/964) evaluates the deferred + typed-record proposal and adopts it for the seams where an external shape + reaches policy: staging targets become `StagingTargets`, and the trusted + digest record becomes `DigestRecord`. Domain-shaped name-to-value mappings + remain where no external format defines them. Every existing rejection + category and every identity and inventory check is retained. This resolves + the 2026-10-28 deferral and does not change the trusted-publishing Stage C or + Stage D status: Stage C remains partial and Stage D remains open. See + [ADR-0068](../adrs/adr-0068-typed-records-for-release-policy-boundaries.md) + and + [the issue 964 plan](issue-964-evaluate-typed-records-for-release-policy-boundaries.md). + Date/Author: 2026-10-30, implementation agent. + - Decision: keep GitHub's asset-name rewrite in its own boundary adapter. Rationale: `release_declaration` is policy and must not normalize a name based on one hosting provider. `release_github_assets_adapter` is the single diff --git a/docs/netsuke-design.md b/docs/netsuke-design.md index 4f932f2ee..c369f595c 100644 --- a/docs/netsuke-design.md +++ b/docs/netsuke-design.md @@ -4155,9 +4155,12 @@ artefacts and attaches the assets to the draft, prefixing names with their staging directories to avoid collisions. Dry-run staging writes its operation summary but does not upload diagnostic artefacts; publication telemetry and the full observability policy are documented in -[ADR-0020](adrs/adr-0020-bounded-release-admission-observability.md). This -automated pipeline guarantees parity across Windows, Linux, and macOS through -the shared actions. +[ADR-0020](adrs/adr-0020-bounded-release-admission-observability.md). Release +policy decides from frozen slotted records at each boundary where an external +record shape arrives, while adapters and CLIs own field names and decoding, as +[ADR-0068](adrs/adr-0068-typed-records-for-release-policy-boundaries.md) +records. This automated pipeline guarantees parity across Windows, Linux, and +macOS through the shared actions. ### 8.7 Release Notes diff --git a/docs/roadmap.md b/docs/roadmap.md index db03a545a..d6113584a 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -2022,6 +2022,9 @@ must share. first-version evidence needs external attestation. - Record substantive decisions as ADRs and update release architecture and developer documentation. + - See + [ADR-0068](adrs/adr-0068-typed-records-for-release-policy-boundaries.md) + for the release policy boundary decision. - See [RFC 0005 §Open questions](rfcs/rfc-0005-harden-release-integrity-and-admission.md#open-questions) and From 7f06bf43865f421c38bb1c590f9fff02ac5ce85c Mon Sep 17 00:00:00 2001 From: leynos Date: Fri, 30 Oct 2026 19:01:31 +0100 Subject: [PATCH 02/16] Characterize staging and digest record rejections Pin every malformed staging table the declaration CLI must refuse and every decoded digest-record shape the file adapter must refuse, before moving the decoders. The new cases pass against the current implementation, so the migration that follows has a before-and-after oracle for each rejection category. Co-Authored-By: Claude Code --- ...test_release_declaration_invalid_config.py | 28 +++++++++++-- .../tests/test_release_digest_record_files.py | 39 +++++++++++++++++++ 2 files changed, 64 insertions(+), 3 deletions(-) diff --git a/scripts/tests/test_release_declaration_invalid_config.py b/scripts/tests/test_release_declaration_invalid_config.py index 6fc81e319..7a7646588 100644 --- a/scripts/tests/test_release_declaration_invalid_config.py +++ b/scripts/tests/test_release_declaration_invalid_config.py @@ -9,9 +9,31 @@ from pathlib import Path -@pytest.mark.parametrize("config", ['targets = "bad"\n', '[targets]\nlinux = "bad"\n']) +@pytest.mark.parametrize( + ("config", "reason"), + [ + ('targets = "bad"\n', "the targets member is not a table"), + ("targets = []\n", "a list is not a target table"), + ("targets = {}\n", "an empty table declares no release target"), + ("common = {}\n", "an absent targets table declares no release target"), + ('[targets]\nlinux = "bad"\n', "a target entry is not a table"), + ("[targets]\nlinux = 1\n", "a target entry is not a table"), + ("[targets]\nlinux = {}\n", "a target table has no platform"), + ("[targets]\nlinux = { platform = 1 }\n", "a platform must be a string"), + ], + ids=[ + "targets-not-a-table", + "targets-is-a-list", + "targets-empty", + "targets-absent", + "entry-is-a-string", + "entry-is-an-integer", + "entry-has-no-platform", + "platform-is-an-integer", + ], +) def test_malformed_targets_are_reported_as_input_faults( - tmp_path: Path, config: str, capsys: pytest.CaptureFixture[str] + tmp_path: Path, config: str, reason: str, capsys: pytest.CaptureFixture[str] ) -> None: """Reject malformed target tables without a traceback or output.""" staging = tmp_path / "staging.toml" @@ -32,7 +54,7 @@ def test_malformed_targets_are_reported_as_input_faults( "--digest-record", str(record), ]) - assert code == EXIT_INPUT_UNUSABLE, f"malformed targets returned {code}" + assert code == EXIT_INPUT_UNUSABLE, f"{reason}: malformed targets returned {code}" assert "inputs unusable" in capsys.readouterr().err, "missing input diagnostic" assert not output.exists(), "malformed targets wrote a declaration" assert not record.exists(), "malformed targets wrote a digest record" diff --git a/scripts/tests/test_release_digest_record_files.py b/scripts/tests/test_release_digest_record_files.py index 68f00cce8..3a34e1c79 100644 --- a/scripts/tests/test_release_digest_record_files.py +++ b/scripts/tests/test_release_digest_record_files.py @@ -43,3 +43,42 @@ def test_json_shape_errors_remain_policy_errors(tmp_path: pathlib.Path) -> None: with pytest.raises(release_digest_record.DigestRecordError): read_digest_record(path) + + +@pytest.mark.parametrize( + ("payload", "fragment"), + [ + ("not json at all", "not usable JSON text"), + ("[]", "must be a mapping"), + ('{"assets": null}', "must carry an 'assets' mapping"), + ('{"assets": []}', "must carry an 'assets' mapping"), + ('{"assets": {}}', "declares no assets"), + ('{"assets": {"a": 1}}', "map asset names to digests as strings"), + ('{"assets": {"a": "nothex"}}', "holds a non-digest"), + ('{"assets": {"a": "ab"}}', "holds a non-digest"), + ( + '{"assets": {"a": "' + DIGEST + '", "b": 2}}', + "map asset names to digests as strings", + ), + ], + ids=[ + "not-json", + "json-array", + "assets-null", + "assets-list", + "assets-empty", + "digest-is-an-integer", + "digest-not-hex", + "digest-too-short", + "one-bad-entry", + ], +) +def test_a_malformed_record_is_refused( + tmp_path: pathlib.Path, payload: str, fragment: str +) -> None: + """Refuse every decoded shape that cannot support a comparison.""" + path = tmp_path / "record.json" + _ = path.write_text(payload, encoding="utf-8") + + with pytest.raises(release_digest_record.DigestRecordError, match=fragment): + read_digest_record(path) From 666fa214879ad38915fd4bbb0e51d21212d74722 Mon Sep 17 00:00:00 2001 From: leynos Date: Fri, 30 Oct 2026 19:25:18 +0100 Subject: [PATCH 03/16] Decode release policy inputs into frozen records Move the two external record shapes across the policy boundary as decoded values, as ADR-0068 records. `StagingTargets` replaces the raw `targets` table and `DigestRecord` replaces the trusted digest mapping; `release_declaration_cli` and `release_digest_record_files` now own the `targets`, `platform`, and `assets` member names and decode once at the edge. The value invariants move with the values. `StagingTargets.from_targets` refuses an empty configuration, `DigestRecord.from_mapping` refuses an empty, non-string, or non-hexadecimal entry, and both keep the existing `MissingTargetsError`, `MalformedTargetError`, `DigestRecordError`, and `RecordedDigestMismatchError` categories so each caller's diagnosis is unchanged. No identity or inventory check is weakened or removed. `release_identity_record` is reclassified as an adapter: it already owned the record's field names and decoded them into a validated identity. Policy tests now construct records directly, and the payload shapes each decoder must refuse are asserted at the decoder seam. Co-Authored-By: Claude Code --- scripts/finalize_github_release_cli.py | 13 +- scripts/release_declaration.py | 112 +++++++----- scripts/release_declaration_cli.py | 77 +++++++- scripts/release_digest_record.py | 159 ++++++++-------- scripts/release_digest_record_files.py | 112 ++++++++++-- scripts/release_identity_record.py | 10 +- scripts/tests/release_staging_support.py | 17 +- scripts/tests/test_release_declaration.py | 43 ++--- scripts/tests/test_release_declaration_cli.py | 7 +- scripts/tests/test_release_digest_record.py | 170 +++++++----------- .../tests/test_release_digest_record_files.py | 33 +++- .../test_verify_release_inventory_cli.py | 9 +- .../test_verify_release_inventory_command.py | 7 +- scripts/verify_release_inventory_cli.py | 13 +- .../release_publication_modules.py | 2 +- 15 files changed, 482 insertions(+), 302 deletions(-) diff --git a/scripts/finalize_github_release_cli.py b/scripts/finalize_github_release_cli.py index 5a27db327..dcd6da4c6 100644 --- a/scripts/finalize_github_release_cli.py +++ b/scripts/finalize_github_release_cli.py @@ -117,7 +117,9 @@ def _normalise_github_digest(reported: object) -> str | None: return digest or None -def _expected(identity_path: pathlib.Path) -> dict[str, str]: +def _expected( + identity_path: pathlib.Path, +) -> release_digest_record.DigestRecord: """Return the digest each declared asset must have, keyed by name. The declared set is the record's assets rather than the listing's, so an @@ -131,8 +133,9 @@ def _expected(identity_path: pathlib.Path) -> dict[str, str]: Returns ------- - dict[str, str] - Asset name to its expected bare hexadecimal digest. + release_digest_record.DigestRecord + The validated record, mapping each asset name to its expected bare + hexadecimal digest. Raises ------ @@ -165,7 +168,7 @@ def _expected(identity_path: pathlib.Path) -> dict[str, str]: ... _ = good.write_text( ... json.dumps({"assets": {"a.tar.gz": digest}}), encoding="utf-8" ... ) - ... _expected(good) == {"a.tar.gz": digest} + ... _expected(good).as_mapping() == {"a.tar.gz": digest} True >>> with tempfile.TemporaryDirectory() as tmp: ... empty = Path(tmp) / "empty.json" @@ -226,7 +229,7 @@ def main(argv: list[str] | None = None) -> int: except InputUnreadableError as error: print(f"digest record unreadable: {error}", file=sys.stderr) return EXIT_INPUT_UNREADABLE - return _reconcile_listing(expected, arguments.assets, arguments.output) + return _reconcile_listing(expected.as_mapping(), arguments.assets, arguments.output) def _reconcile_listing( diff --git a/scripts/release_declaration.py b/scripts/release_declaration.py index e8b5d3be2..a4ace0eec 100644 --- a/scripts/release_declaration.py +++ b/scripts/release_declaration.py @@ -1,9 +1,11 @@ """Declare the release assets from in-memory names and target facts. This module owns release naming and package-count policy. The declaration CLI -reads TOML, asks the hoist adapter for archive names, and scans the staged tree; -it passes those facts here as mappings and file names. Keeping paths and config -parsing at that boundary makes the declaration independently testable. +reads TOML, decodes the staging targets into a :class:`StagingTargets` record, +asks the hoist adapter for archive names, and scans the staged tree; it passes +those facts here as a typed record and file names. Keeping paths, config +parsing, and the TOML field names at that boundary makes the declaration +independently testable. Three external shared actions own the names of the operating-system packages, so their names are observed from the tree rather than reconstructed. The @@ -14,6 +16,7 @@ ``scripts/tests/test_release_declaration.py``. """ +import dataclasses import typing as typ from release_package_formats import ( @@ -22,7 +25,6 @@ DeclarationError, DuplicatePackageError, MalformedTargetError, - MalformedTargetKind, MissingPackageError, MissingTargetsError, StagedTreeError, @@ -44,19 +46,62 @@ "MissingPackageError", "MissingTargetsError", "StagedTreeError", + "StagingTarget", + "StagingTargets", "UnknownPlatformError", "archive_asset_names", "declared_asset_names", "platform_target_counts", ] -#: Name of the target mapping in the parsed staging configuration. -TARGETS_TABLE = "targets" - #: Suffix appended to every published archive name. SIDECAR_SUFFIX = ".sha256" +@dataclasses.dataclass(frozen=True, slots=True) +class StagingTarget: + """One configured staging target, reduced to the platform it builds for.""" + + #: The platform value that selects this target's package formats. + platform: str + + +@dataclasses.dataclass(frozen=True, slots=True) +class StagingTargets: + """The configured staging targets, in configuration order.""" + + #: One entry per configured target. + targets: tuple[StagingTarget, ...] + + @classmethod + def from_targets(cls, targets: cabc.Iterable[StagingTarget]) -> StagingTargets: + """Build the record, refusing a configuration that names no target. + + Returns + ------- + StagingTargets + The decoded staging targets. + + Raises + ------ + MissingTargetsError + If ``targets`` is empty. + + Examples + -------- + >>> StagingTargets.from_targets([StagingTarget("linux")]).targets + (StagingTarget(platform='linux'),) + >>> StagingTargets.from_targets([]) # doctest: +ELLIPSIS + Traceback (most recent call last): + ... + release_package_formats.MissingTargetsError: staging configuration defines... + """ + entries = tuple(targets) + if not entries: + raise MissingTargetsError + return cls(entries) + + def archive_asset_names(archives: cabc.Iterable[str]) -> tuple[str, ...]: """Pair each configured archive name with its checksum-sidecar name. @@ -75,50 +120,27 @@ def archive_asset_names(archives: cabc.Iterable[str]) -> tuple[str, ...]: ) -def platform_target_counts(target_config: object) -> dict[str, int]: - """Count configured targets by platform from an already parsed mapping. +def platform_target_counts(targets: StagingTargets) -> dict[str, int]: + """Count decoded staging targets by the platform each one builds for. Returns ------- dict[str, int] - The number of configured targets for each platform. - - Raises - ------ - MissingTargetsError - If the ``targets`` mapping is absent or empty. + The number of targets for each platform. Examples -------- - >>> platform_target_counts({"targets": {"linux": {"platform": "linux"}}}) - {'linux': 1} + >>> platform_target_counts( + ... StagingTargets((StagingTarget("linux"), StagingTarget("linux"))) + ... ) + {'linux': 2} """ - config = _mapping(target_config) - targets = config.get(TARGETS_TABLE) - if not isinstance(targets, dict) or not targets: - raise MissingTargetsError counts: dict[str, int] = {} - for entry in targets.values(): - platform = _platform_of(entry) - counts[platform] = counts.get(platform, 0) + 1 + for target in targets.targets: + counts[target.platform] = counts.get(target.platform, 0) + 1 return counts -def _mapping(value: object) -> cabc.Mapping[str, object]: - """Return a string-keyed mapping or refuse the malformed config value.""" - if not isinstance(value, dict) or not all(isinstance(key, str) for key in value): - raise MalformedTargetError(MalformedTargetKind.INVALID_MAPPING) - return value - - -def _platform_of(entry: object) -> str: - """Return one target table's string platform value.""" - platform = entry.get("platform") if isinstance(entry, dict) else None - if not isinstance(platform, str): - raise MalformedTargetError(MalformedTargetKind.MISSING_PLATFORM) - return platform - - def _packages_for_platform( staged_names: tuple[str, ...], platform: str, target_count: int ) -> list[str]: @@ -152,7 +174,7 @@ def _declared_packages( def declared_asset_names( *, archive_names: cabc.Iterable[str], - target_config: object, + targets: StagingTargets, staged_package_names: cabc.Iterable[str], ) -> tuple[str, ...]: """Declare the flat consumer set from archive and package observations. @@ -162,8 +184,8 @@ def declared_asset_names( archive_names Archive names resolved by the hoist adapter from the manifest and staging configuration. - target_config - Parsed staging TOML used to count package outputs by platform. + targets + Decoded staging targets, used to count package outputs by platform. staged_package_names GitHub-visible basenames observed below the downloaded artifact tree. @@ -176,20 +198,20 @@ def declared_asset_names( -------- >>> declared_asset_names( ... archive_names=["crate.tar.gz"], - ... target_config={"targets": {"linux": {"platform": "linux"}}}, + ... targets=StagingTargets((StagingTarget("linux"),)), ... staged_package_names=["crate.deb", "crate.rpm"], ... ) ('crate.deb', 'crate.rpm', 'crate.tar.gz', 'crate.tar.gz.sha256') >>> declared_asset_names( ... archive_names=["crate.tar.gz"], - ... target_config={"targets": {"linux": {"platform": "linux"}}}, + ... targets=StagingTargets((StagingTarget("linux"),)), ... staged_package_names=[], ... ) Traceback (most recent call last): ... release_package_formats.MissingPackageError: platform 'linux' ... """ - counts = platform_target_counts(target_config) + counts = platform_target_counts(targets) staged_names = tuple(staged_package_names) packages = _declared_packages(staged_names, counts) return tuple(sorted([*archive_asset_names(archive_names), *packages])) diff --git a/scripts/release_declaration_cli.py b/scripts/release_declaration_cli.py index 4cfe780ef..847c0c7f2 100644 --- a/scripts/release_declaration_cli.py +++ b/scripts/release_declaration_cli.py @@ -40,6 +40,7 @@ import release_declaration import release_digest_record_files import release_github_assets_adapter +import release_package_formats import release_staged_digests EXIT_OK = 0 @@ -47,6 +48,76 @@ EXIT_INPUT_UNUSABLE = 2 EXIT_OUTPUT_UNWRITABLE = 3 +#: Name of the target table in the parsed staging configuration. +TARGETS_TABLE = "targets" + +#: Member naming one target's package platform. +PLATFORM_MEMBER = "platform" + + +def staging_targets(config: object) -> release_declaration.StagingTargets: + """Decode the staging configuration's target tables into typed facts. + + Parameters + ---------- + config + Parsed staging TOML, as :func:`tomllib.loads` returns it. + + Returns + ------- + release_declaration.StagingTargets + One entry per configured target, carrying only its platform. + + Raises + ------ + release_declaration.MalformedTargetError + If the configuration or a target table is not a string-keyed mapping, + or a target names no string platform. + release_declaration.MissingTargetsError + If the configuration declares no target. + + Examples + -------- + >>> staging_targets({"targets": {"linux": {"platform": "linux"}}}).targets + (StagingTarget(platform='linux'),) + """ + if not isinstance(config, dict) or not all(isinstance(key, str) for key in config): + raise release_declaration.MalformedTargetError( + release_package_formats.MalformedTargetKind.INVALID_MAPPING + ) + declared = config.get(TARGETS_TABLE) + if not isinstance(declared, dict): + raise release_declaration.MissingTargetsError + return release_declaration.StagingTargets.from_targets( + _staging_target(entry) for entry in declared.values() + ) + + +def _staging_target(entry: object) -> release_declaration.StagingTarget: + """Return one target table's platform as a typed staging target. + + Returns + ------- + release_declaration.StagingTarget + The target's platform. + + Raises + ------ + release_declaration.MalformedTargetError + If the entry is not a table with a string ``platform`` member. + + Examples + -------- + >>> _staging_target({"platform": "macos"}) + StagingTarget(platform='macos') + """ + platform = entry.get(PLATFORM_MEMBER) if isinstance(entry, dict) else None + if not isinstance(platform, str): + raise release_declaration.MalformedTargetError( + release_package_formats.MalformedTargetKind.MISSING_PLATFORM + ) + return release_declaration.StagingTarget(platform) + def build_parser() -> argparse.ArgumentParser: """Build the parser for the declared-name producer. @@ -296,8 +367,8 @@ def _compute_declaration( refused """ config = tomllib.loads(arguments.staging_config.read_text(encoding="utf-8")) - # Validate target tables before the discovery adapter iterates them. - _ = release_declaration.platform_target_counts(config) + # Decode once, before discovery, so a malformed table fails on the config. + targets = staging_targets(config) archive_names = hoist_binstall_discovery.expected_archive_names( arguments.staging_config, arguments.manifest, arguments.version ) @@ -307,7 +378,7 @@ def _compute_declaration( ] names = release_declaration.declared_asset_names( archive_names=archive_names, - target_config=config, + targets=targets, staged_package_names=staged_package_names, ) digests = release_staged_digests.staged_digests(arguments.dist_dir, expected=names) diff --git a/scripts/release_digest_record.py b/scripts/release_digest_record.py index ccbaa0ccc..5875a288e 100644 --- a/scripts/release_digest_record.py +++ b/scripts/release_digest_record.py @@ -1,8 +1,9 @@ -"""Validate trusted digest-record values and compare their coverage. +"""Represent trusted digest records and compare their coverage. The release record maps published asset names to SHA-256 digests. This module -owns that in-memory shape and the comparisons made against it. JSON parsing, -serialization, and file access belong to :mod:`release_digest_record_files`. +owns the validated :class:`DigestRecord` and the comparisons made against it. +JSON parsing, the record's ``assets`` member, serialization, and file access +belong to :mod:`release_digest_record_files`. The record is written before upload and remains independent of downloaded bytes. Comparing downloads to it is what makes replacing an archive or package @@ -12,6 +13,7 @@ ``scripts/tests/test_release_digest_record.py``. """ +import dataclasses import re import typing as typ @@ -23,9 +25,6 @@ #: One hexadecimal SHA-256 digest, in either case. _DIGEST_TEXT = re.compile(rf"\A[0-9a-fA-F]{{{release_digest.DIGEST_HEX_LENGTH}}}\Z") -#: The member holding the name-to-digest mapping. -ASSETS_MEMBER = "assets" - class DigestRecordError(ValueError): """Report a digest record that cannot support a verification decision.""" @@ -37,73 +36,81 @@ class RecordedDigestMismatchError( """Report downloaded bytes that differ from the trusted record.""" -def validate_digest_record(record: object) -> dict[str, str]: - """Return a validated name-to-lowercase-digest mapping. +@dataclasses.dataclass(frozen=True, slots=True) +class DigestRecord: + """A non-empty, validated set of published asset names and digests.""" + + #: Asset names paired with their lowercase SHA-256 digest, sorted by name. + entries: tuple[tuple[str, str], ...] + + @classmethod + def from_mapping(cls, declared: cabc.Mapping[str, object]) -> DigestRecord: + """Validate decoded entries and build the trusted record. + + Parameters + ---------- + declared + Asset names mapped to digests, before validation. + + Returns + ------- + DigestRecord + The entries with lowercase digests, sorted by asset name. + + Raises + ------ + DigestRecordError + If the mapping is empty or an entry is not a string name and a + valid hexadecimal digest. + + Examples + -------- + >>> DigestRecord.from_mapping({"a": "AB" * 32}).as_mapping()["a"] == "ab" * 32 + True + >>> DigestRecord.from_mapping({}) # doctest: +ELLIPSIS + Traceback (most recent call last): + ... + release_digest_record.DigestRecordError: the digest record declares no assets... + """ + entries = tuple(sorted(_validated_entries(declared))) + if not entries: + msg = "the digest record declares no assets, so there is nothing to verify" + raise DigestRecordError(msg) + return cls(entries) - Parameters - ---------- - record - Decoded record value, normally parsed by the JSON adapter. + def as_mapping(self) -> dict[str, str]: + """Return the record as a name-to-digest mapping. - Returns - ------- - dict[str, str] - Asset name mapped to its lowercase SHA-256 digest. + Returns + ------- + dict[str, str] + Each asset name mapped to its lowercase digest. - Notes - ----- - The validation helpers' :class:`DigestRecordError` reaches the caller - unchanged, preserving the precise reason the decoded record was refused. + Examples + -------- + >>> DigestRecord.from_mapping({"a": "ab" * 32}).as_mapping()["a"] == "ab" * 32 + True + """ + return dict(self.entries) - Examples - -------- - >>> validate_digest_record({"assets": {"a": "AB" * 32}})["a"] == "ab" * 32 - True - >>> validate_digest_record({"assets": {}}) # doctest: +ELLIPSIS - Traceback (most recent call last): - ... - release_digest_record.DigestRecordError: the digest record declares no assets... - """ - declared = _asset_mapping(record) - return dict(_validated_entries(declared)) + def names(self) -> set[str]: + """Return the asset names the record covers. + Returns + ------- + set[str] + The asset names, without their digests. -def _asset_mapping(record: object) -> dict[object, object]: - """Return the non-empty asset mapping from a decoded record. - - Returns - ------- - dict[object, object] - The unvalidated entries from the record's ``assets`` member. - - Raises - ------ - DigestRecordError - If the record is not a non-empty mapping of asset entries. - - Examples - -------- - >>> _asset_mapping({"assets": {"a": "ab"}}) - {'a': 'ab'} - """ - if not isinstance(record, dict): - msg = f"the digest record must be a mapping, got {type(record).__name__}" - raise DigestRecordError(msg) - declared = record.get(ASSETS_MEMBER) - if not isinstance(declared, dict): - msg = ( - f"the digest record must carry an '{ASSETS_MEMBER}' mapping, got " - f"{type(declared).__name__}" - ) - raise DigestRecordError(msg) - if not declared: - msg = "the digest record declares no assets, so there is nothing to verify" - raise DigestRecordError(msg) - return declared + Examples + -------- + >>> DigestRecord.from_mapping({"a": "ab" * 32}).names() + {'a'} + """ + return {name for name, _ in self.entries} def _validated_entries( - declared: dict[object, object], + declared: cabc.Mapping[str, object], ) -> cabc.Iterable[tuple[str, str]]: """Yield each asset name and lowercase digest after validating its types. @@ -115,7 +122,7 @@ def _validated_entries( Raises ------ DigestRecordError - If an entry is not a string name and a valid hexadecimal digest. + If a digest is not a string or not a valid hexadecimal value. Examples -------- @@ -123,7 +130,7 @@ def _validated_entries( True """ for name, digest in declared.items(): - if not isinstance(name, str) or not isinstance(digest, str): + if not isinstance(digest, str): msg = "the digest record must map asset names to digests as strings" raise DigestRecordError(msg) yield name, _validated_digest(name, digest) @@ -154,7 +161,7 @@ def _validated_digest(name: str, digest: str) -> str: def require_record_matches_declaration( - digests: cabc.Mapping[str, str], *, expected: cabc.Iterable[str] + record: DigestRecord, *, expected: cabc.Iterable[str] ) -> None: """Require the record to cover exactly the assets the release declares. @@ -166,9 +173,11 @@ def require_record_matches_declaration( Examples -------- - >>> require_record_matches_declaration({"a": "ab"}, expected=["a"]) + >>> require_record_matches_declaration( + ... DigestRecord.from_mapping({"a": "ab" * 32}), expected=["a"] + ... ) >>> require_record_matches_declaration( # doctest: +ELLIPSIS - ... {"a": "ab"}, expected=["a", "b"] + ... DigestRecord.from_mapping({"a": "ab" * 32}), expected=["a", "b"] ... ) Traceback (most recent call last): ... @@ -178,7 +187,7 @@ def require_record_matches_declaration( if not declared: msg = "the declaration names no asset, so there is nothing to verify" raise DigestRecordError(msg) - _require_complete_coverage(declared, set(digests)) + _require_complete_coverage(declared, record.names()) def _require_complete_coverage(declared: set[str], recorded: set[str]) -> None: @@ -195,7 +204,7 @@ def _require_complete_coverage(declared: set[str], recorded: set[str]) -> None: def require_downloads_match_record( - computed: cabc.Mapping[str, str], *, trusted: cabc.Mapping[str, str] + computed: cabc.Mapping[str, str], *, trusted: DigestRecord ) -> None: """Require every downloaded asset digest to match the trusted record. @@ -207,15 +216,17 @@ def require_downloads_match_record( Examples -------- - >>> require_downloads_match_record({"a": "ab"}, trusted={"a": "ab"}) + >>> require_downloads_match_record( + ... {"a": "ab" * 32}, trusted=DigestRecord.from_mapping({"a": "ab" * 32}) + ... ) >>> require_downloads_match_record( # doctest: +ELLIPSIS - ... {"a": "cd"}, trusted={"a": "ab"} + ... {"a": "cd" * 32}, trusted=DigestRecord.from_mapping({"a": "ab" * 32}) ... ) Traceback (most recent call last): ... release_digest_record.RecordedDigestMismatchError: uploaded assets do not match... """ - wrong = _mismatched_download_names(computed, trusted) + wrong = _mismatched_download_names(computed, trusted.as_mapping()) if not wrong: return msg = f"uploaded assets do not match the staged candidate: {wrong!r}" diff --git a/scripts/release_digest_record_files.py b/scripts/release_digest_record_files.py index 5af96c09a..cd1d540c9 100644 --- a/scripts/release_digest_record_files.py +++ b/scripts/release_digest_record_files.py @@ -1,8 +1,9 @@ """Read and write the trusted digest record at release boundaries. -This adapter owns JSON serialization and path errors for digest records. It -delegates the record shape to :mod:`release_digest_record`, so every consumer -uses the same validation rules after decoding. +This adapter owns JSON serialization, the record's ``assets`` member, and path +errors for digest records. It delegates entry validation to +:class:`release_digest_record.DigestRecord`, so every consumer receives the +same validated record after decoding. Run via the declaration, inventory-verification, and finalization CLIs; behavioural coverage lives in ``scripts/tests/test_release_digest_record_files.py``. @@ -17,18 +18,23 @@ import collections.abc as cabc import pathlib +#: The JSON member holding the name-to-digest mapping. +ASSETS_MEMBER = "assets" + class UnreadableDigestRecordError(release_digest_record.DigestRecordError): """Report that a trusted record file could not be read.""" -def read_digest_record(source: pathlib.Path) -> dict[str, str]: - """Read JSON and validate the trusted digest mapping. +def read_digest_record( + source: pathlib.Path, +) -> release_digest_record.DigestRecord: + """Read JSON and decode the trusted digest record. Returns ------- - dict[str, str] - Asset name to lowercase SHA-256 digest. + release_digest_record.DigestRecord + The validated record with lowercase digests. Raises ------ @@ -46,7 +52,7 @@ def read_digest_record(source: pathlib.Path) -> dict[str, str]: ... _ = record.write_text( ... '{"assets": {"a": "' + "ab" * 32 + '"}}', encoding="utf-8" ... ) - ... read_digest_record(record)["a"] == "ab" * 32 + ... read_digest_record(record).as_mapping()["a"] == "ab" * 32 True """ try: @@ -57,7 +63,81 @@ def read_digest_record(source: pathlib.Path) -> dict[str, str]: except (UnicodeDecodeError, json.JSONDecodeError) as error: msg = f"the digest record {source} is not usable JSON text: {error}" raise release_digest_record.DigestRecordError(msg) from error - return release_digest_record.validate_digest_record(decoded) + return validate_digest_record(decoded) + + +def validate_digest_record( + record: object, +) -> release_digest_record.DigestRecord: + """Decode a record's ``assets`` member into a validated digest record. + + Parameters + ---------- + record + Decoded JSON value. + + Returns + ------- + release_digest_record.DigestRecord + The validated record with lowercase digests. + + Notes + ----- + The value invariant belongs to + :meth:`release_digest_record.DigestRecord.from_mapping`, whose + ``DigestRecordError`` propagates unchanged. What stays here is the member + name and the shape of the value that carries it. + + Examples + -------- + >>> record = validate_digest_record({"assets": {"a": "AB" * 32}}) + >>> record.as_mapping()["a"] == "ab" * 32 + True + >>> validate_digest_record({"assets": {}}) # doctest: +ELLIPSIS + Traceback (most recent call last): + ... + release_digest_record.DigestRecordError: the digest record declares no assets... + """ + return release_digest_record.DigestRecord.from_mapping(_asset_mapping(record)) + + +def _asset_mapping(record: object) -> dict[str, object]: + """Return the non-empty asset mapping from a decoded record. + + Returns + ------- + dict[str, object] + The unvalidated digest values from the record's ``assets`` member, + keyed by asset name. + + Raises + ------ + release_digest_record.DigestRecordError + If the record is not a non-empty mapping of string-keyed asset + entries. + + Examples + -------- + >>> _asset_mapping({"assets": {"a": "ab"}}) + {'a': 'ab'} + """ + if not isinstance(record, dict): + msg = f"the digest record must be a mapping, got {type(record).__name__}" + raise release_digest_record.DigestRecordError(msg) + declared = record.get(ASSETS_MEMBER) + if not isinstance(declared, dict): + msg = ( + f"the digest record must carry an '{ASSETS_MEMBER}' mapping, got " + f"{type(declared).__name__}" + ) + raise release_digest_record.DigestRecordError(msg) + if not declared: + msg = "the digest record declares no assets, so there is nothing to verify" + raise release_digest_record.DigestRecordError(msg) + if not all(isinstance(name, str) for name in declared): + msg = "the digest record must map asset names to digests as strings" + raise release_digest_record.DigestRecordError(msg) + return typ.cast("dict[str, object]", declared) def write_digest_record( @@ -68,6 +148,12 @@ def write_digest_record( The JSON is sorted, indented, and newline-terminated so identical input mappings always produce identical artefacts. + Notes + ----- + The mapping is validated by building a + :class:`release_digest_record.DigestRecord`, so a writer cannot emit a + record its own reader would refuse. + Examples -------- >>> import tempfile @@ -75,13 +161,11 @@ def write_digest_record( >>> with tempfile.TemporaryDirectory() as tmp: ... output = Path(tmp) / "record.json" ... write_digest_record({"a": "ab" * 32}, output) - ... read_digest_record(output)["a"] == "ab" * 32 + ... read_digest_record(output).as_mapping()["a"] == "ab" * 32 True """ - validated = release_digest_record.validate_digest_record({ - release_digest_record.ASSETS_MEMBER: dict(digests) - }) + validated = release_digest_record.DigestRecord.from_mapping(digests) serialized = json.dumps( - {release_digest_record.ASSETS_MEMBER: validated}, indent=2, sort_keys=True + {ASSETS_MEMBER: validated.as_mapping()}, indent=2, sort_keys=True ) destination.write_text(serialized + "\n", encoding="utf-8") diff --git a/scripts/release_identity_record.py b/scripts/release_identity_record.py index b7bce5c1b..1e639fae5 100644 --- a/scripts/release_identity_record.py +++ b/scripts/release_identity_record.py @@ -2,11 +2,11 @@ :mod:`release_candidate` decides which candidate a run may publish, and :meth:`release_candidate.ReleaseIdentity.as_record` exposes that decision as a -mapping. The CLI adapter serializes it, and this module validates the decoded -mapping on read. The two are separate because their inputs differ: a resolver -sees facts the workflow has just gathered, while a reader sees a value that may -have been hand-edited, truncated by a failed write, or left over from an -earlier run. +mapping. The CLI adapter serializes it, and this adapter owns the record's +field names and decodes them back into a validated identity on read. The two +are separate because their inputs differ: a resolver sees facts the workflow +has just gathered, while a reader sees a value that may have been hand-edited, +truncated by a failed write, or left over from an earlier run. That difference is why later stages call this rather than handing a mapping to :func:`release_candidate.resolve_identity`. A stored ``is_prerelease`` flag is diff --git a/scripts/tests/release_staging_support.py b/scripts/tests/release_staging_support.py index ca158a852..6f65ea98f 100644 --- a/scripts/tests/release_staging_support.py +++ b/scripts/tests/release_staging_support.py @@ -19,6 +19,7 @@ import typing as typ import release_digest_files +import release_digest_record import release_digest_record_files if typ.TYPE_CHECKING: @@ -102,7 +103,7 @@ def write_declaration(destination: pathlib.Path, names: list[str]) -> pathlib.Pa return destination -def trusted_digests(staging: pathlib.Path) -> dict[str, str]: +def trusted_digests(staging: pathlib.Path) -> release_digest_record.DigestRecord: """Record the staged tree's own digests, as the build would before upload. Parameters @@ -112,8 +113,8 @@ def trusted_digests(staging: pathlib.Path) -> dict[str, str]: Returns ------- - dict[str, str] - Asset name to the digest of that staged file's own bytes. + release_digest_record.DigestRecord + Each staged file's name and the digest of its own bytes. Notes ----- @@ -121,15 +122,15 @@ def trusted_digests(staging: pathlib.Path) -> dict[str, str]: substitution is compared against an independent record and not against the module under test's idea of one. """ - return { + return release_digest_record.DigestRecord.from_mapping({ path.name: release_digest_files.digest_of_file(path) for path in staging.iterdir() if path.is_file() - } + }) def write_trusted_record( - destination: pathlib.Path, digests: dict[str, str] + destination: pathlib.Path, digests: release_digest_record.DigestRecord ) -> pathlib.Path: """Write a trusted digest record the CLIs can read. @@ -138,7 +139,7 @@ def write_trusted_record( destination File to write. digests - Asset name to recorded digest. + The validated record to serialize. Returns ------- @@ -146,5 +147,5 @@ def write_trusted_record( ``destination``, so a caller can pass the result straight to an invocation's argument list. """ - release_digest_record_files.write_digest_record(digests, destination) + release_digest_record_files.write_digest_record(digests.as_mapping(), destination) return destination diff --git a/scripts/tests/test_release_declaration.py b/scripts/tests/test_release_declaration.py index c80e64def..97210c901 100644 --- a/scripts/tests/test_release_declaration.py +++ b/scripts/tests/test_release_declaration.py @@ -21,26 +21,22 @@ "netsuke-0.1.0.beta4-1_arm64.deb", "netsuke-0.1.0-beta4-arm64.msi", ) -TARGET_CONFIG = { - "targets": { - "linux-x86_64": {"platform": "linux"}, - "linux-aarch64": {"platform": "linux"}, - "windows-x86_64": {"platform": "windows"}, - "windows-aarch64": {"platform": "windows"}, - "macos-x86_64": {"platform": "macos"}, - "macos-aarch64": {"platform": "macos"}, - } -} +STAGING_TARGETS = release_declaration.StagingTargets( + tuple( + release_declaration.StagingTarget(platform) + for platform in ("linux", "linux", "windows", "windows", "macos", "macos") + ) +) def _declare( staged_packages: tuple[str, ...] = PACKAGES, - target_config: object = TARGET_CONFIG, + targets: release_declaration.StagingTargets = STAGING_TARGETS, ) -> tuple[str, ...]: """Declare the consumer set from typed facts, without reading files.""" return release_declaration.declared_asset_names( archive_names=ARCHIVES, - target_config=target_config, + targets=targets, staged_package_names=staged_packages, ) @@ -103,29 +99,16 @@ def test_a_platform_with_no_declared_format_is_refused() -> None: with pytest.raises(release_declaration.UnknownPlatformError): _declare( (), - {"targets": {"solaris": {"platform": "solaris"}}}, + release_declaration.StagingTargets(( + release_declaration.StagingTarget("solaris"), + )), ) def test_a_configuration_without_targets_is_refused() -> None: - """Refuse an empty target mapping rather than declare a smaller release.""" + """Refuse an empty target set rather than declare a smaller release.""" with pytest.raises(release_declaration.MissingTargetsError): - _declare((), {}) - - -def test_a_non_mapping_configuration_is_refused() -> None: - """Refuse malformed configuration before counting platform targets.""" - with pytest.raises( - release_declaration.MalformedTargetError, - match="not a string-keyed mapping", - ): - _declare((), []) - - -def test_a_target_without_a_platform_is_refused() -> None: - """Refuse a target the policy cannot count under a package platform.""" - with pytest.raises(release_declaration.MalformedTargetError): - _declare((), {"targets": {"linux": {}}}) + _ = release_declaration.StagingTargets.from_targets(()) def test_package_names_are_observed_from_the_supplied_tree_facts() -> None: diff --git a/scripts/tests/test_release_declaration_cli.py b/scripts/tests/test_release_declaration_cli.py index 0434a19fb..115ea1aba 100644 --- a/scripts/tests/test_release_declaration_cli.py +++ b/scripts/tests/test_release_declaration_cli.py @@ -360,13 +360,14 @@ def test_the_producer_records_a_digest_for_every_declared_asset( declared = workspace["output"].read_text(encoding="utf-8").splitlines() recorded = release_digest_record_files.read_digest_record(workspace["record"]) + digests = recorded.as_mapping() - assert sorted(recorded) == sorted(declared), ( + assert recorded.names() == set(declared), ( "the record must cover exactly the declared assets, or the verifier's " - f"exact-set check will refuse a correct release: {sorted(recorded)}" + f"exact-set check will refuse a correct release: {sorted(recorded.names())}" ) for name in declared: - assert recorded[name] == release_digest_files.digest_of_file( + assert digests[name] == release_digest_files.digest_of_file( workspace["dist"] / name ), f"the recorded digest for {name} must describe its staged bytes" diff --git a/scripts/tests/test_release_digest_record.py b/scripts/tests/test_release_digest_record.py index ced2f6f1c..638481229 100644 --- a/scripts/tests/test_release_digest_record.py +++ b/scripts/tests/test_release_digest_record.py @@ -1,145 +1,104 @@ -"""Exercise the trusted digest record's reader, writer and comparisons. - -Every refusal in :mod:`scripts.release_digest_record` exists because the -alternative is a vacuous success, so each one is proved here rather than left -implied by the happy path. The record is permission to act on a release; a test -that only showed a well-formed record being accepted would not say what happens -when one is not. +"""Specify the trusted digest record's invariants and comparisons. + +The record is permission to act on a release, so every refusal in +:mod:`scripts.release_digest_record` exists because the alternative is a +vacuous success and is proved here rather than left implied by the happy path. +The record is constructed directly, as a decoded adapter would construct it; +:mod:`scripts.tests.test_release_digest_record_files` covers the JSON shapes +that produce each refusal. """ -import json -import typing as typ - import pytest import release_digest -from release_digest_record import ( - DigestRecordError, - RecordedDigestMismatchError, - require_downloads_match_record, - require_record_matches_declaration, -) -from release_digest_record_files import ( - UnreadableDigestRecordError, - read_digest_record, - write_digest_record, -) - -if typ.TYPE_CHECKING: - import pathlib +import release_digest_record DIGEST = "ab" * 32 OTHER = "cd" * 32 -def _record(destination: pathlib.Path, digests: dict[str, str]) -> pathlib.Path: - """Write a well-formed record.""" - write_digest_record(digests, destination) - return destination +def _trusted(digests: dict[str, object]) -> release_digest_record.DigestRecord: + """Build the record an adapter decodes from an external payload.""" + return release_digest_record.DigestRecord.from_mapping(digests) -def test_a_written_record_round_trips(tmp_path: pathlib.Path) -> None: - """Read back exactly what was written.""" - source = _record(tmp_path / "record.json", {"a": DIGEST, "b": OTHER}) +def test_a_record_normalises_digest_case() -> None: + """Accept a digest written in either case, normalising on the way in.""" + record = _trusted({"a": DIGEST.upper()}) - assert read_digest_record(source) == {"a": DIGEST, "b": OTHER}, ( - "the reader must return the mapping the writer produced" + assert record.as_mapping() == {"a": DIGEST}, ( + "an uppercase digest describes the same bytes and must compare equal" ) -def test_the_record_is_written_in_sorted_order(tmp_path: pathlib.Path) -> None: - """Write a stable file, so two runs over one tree are byte-identical.""" - source = _record(tmp_path / "record.json", {"b": OTHER, "a": DIGEST}) +def test_an_empty_record_is_refused() -> None: + """Refuse a record that would verify nothing.""" + with pytest.raises(release_digest_record.DigestRecordError): + _ = _trusted({}) - assert ( - source.read_text(encoding="utf-8") - == json.dumps({"assets": {"a": DIGEST, "b": OTHER}}, indent=2, sort_keys=True) - + "\n" - ), "the record must be sorted and newline-terminated" +def test_a_digest_that_is_not_a_string_is_refused() -> None: + """Refuse a value the comparison could never match.""" + with pytest.raises(release_digest_record.DigestRecordError): + _ = _trusted({"a": 1}) -def test_digests_are_lowercased_on_read(tmp_path: pathlib.Path) -> None: - """Accept a record written in either case, normalising on the way in.""" - source = tmp_path / "record.json" - upper = {"assets": {"a": DIGEST.upper()}} - _ = source.write_text(json.dumps(upper), encoding="utf-8") - assert read_digest_record(source) == {"a": DIGEST}, ( - "an uppercase digest describes the same bytes and must compare equal" - ) +@pytest.mark.parametrize("digest", ["nothex", "ab"], ids=["not-hex", "too-short"]) +def test_a_digest_that_is_not_a_digest_is_refused(digest: str) -> None: + """Refuse a malformed digest rather than comparing it as text.""" + with pytest.raises(release_digest_record.DigestRecordError): + _ = _trusted({"a": digest}) -@pytest.mark.parametrize( - ("payload", "reason"), - [ - ("not json at all", "a file that is not JSON must be refused"), - ("[]", "a JSON array is not a record"), - ('{"assets": null}', "an absent mapping is not an empty one"), - ('{"assets": []}', "a list is not a name-to-digest mapping"), - ('{"assets": {}}', "an empty record would verify nothing"), - ('{"assets": {"a": 1}}', "a digest must be a string"), - ('{"assets": {"a": "nothex"}}', "a non-digest must be refused, not compared"), - ('{"assets": {"a": "ab"}}', "a short digest must be refused"), - ( - '{"assets": {"a": "' + "ab" * 32 + '", "b": 2}}', - "one bad entry refuses the whole record", - ), - ], -) -def test_a_malformed_record_is_refused( - tmp_path: pathlib.Path, payload: str, reason: str -) -> None: - """Refuse every shape that cannot support a comparison, with a reason.""" - source = tmp_path / "record.json" - _ = source.write_text(payload, encoding="utf-8") - - with pytest.raises(DigestRecordError): - _ = read_digest_record(source) - - -def test_an_unreadable_record_is_refused(tmp_path: pathlib.Path) -> None: - """Refuse a path that does not exist rather than treating it as empty. - - The narrower type is asserted, not merely the base one. A consumer that - reports this as a rejected release rather than an unreadable input sends an - operator to the upload for a fault in a path, so which subclass is raised - is part of the contract rather than an implementation detail. - """ - with pytest.raises(UnreadableDigestRecordError): - _ = read_digest_record(tmp_path / "absent.json") +def test_one_bad_entry_refuses_the_whole_record() -> None: + """Refuse the record, not merely the entry, so no asset is left unchecked.""" + with pytest.raises(release_digest_record.DigestRecordError): + _ = _trusted({"a": DIGEST, "b": 2}) def test_a_covering_record_is_accepted() -> None: """Accept a record whose names are exactly the declaration's.""" - require_record_matches_declaration({"a": DIGEST}, expected=["a"]) + release_digest_record.require_record_matches_declaration( + _trusted({"a": DIGEST}), expected=["a"] + ) def test_a_record_that_omits_a_declared_asset_is_refused() -> None: """Refuse a record that would leave an asset uncompared.""" - with pytest.raises(DigestRecordError): - require_record_matches_declaration({"a": DIGEST}, expected=["a", "b"]) + with pytest.raises(release_digest_record.DigestRecordError): + release_digest_record.require_record_matches_declaration( + _trusted({"a": DIGEST}), expected=["a", "b"] + ) def test_a_record_that_adds_an_undeclared_asset_is_refused() -> None: """Refuse a record naming an asset the release never declared.""" - with pytest.raises(DigestRecordError): - require_record_matches_declaration({"a": DIGEST, "b": OTHER}, expected=["a"]) + with pytest.raises(release_digest_record.DigestRecordError): + release_digest_record.require_record_matches_declaration( + _trusted({"a": DIGEST, "b": OTHER}), expected=["a"] + ) def test_an_empty_declaration_is_refused() -> None: """Refuse the vacuous case, where set equality holds over two empty sets.""" - with pytest.raises(DigestRecordError): - require_record_matches_declaration({}, expected=[]) + with pytest.raises(release_digest_record.DigestRecordError): + release_digest_record.require_record_matches_declaration( + _trusted({"a": DIGEST}), expected=[] + ) def test_matching_downloads_are_accepted() -> None: """Accept downloads whose computed digests equal the recorded ones.""" - require_downloads_match_record({"a": DIGEST}, trusted={"a": DIGEST}) + release_digest_record.require_downloads_match_record( + {"a": DIGEST}, trusted=_trusted({"a": DIGEST}) + ) def test_a_differing_download_is_refused() -> None: """Refuse a download whose bytes differ from the recorded bytes.""" - with pytest.raises(RecordedDigestMismatchError): - require_downloads_match_record({"a": OTHER}, trusted={"a": DIGEST}) + with pytest.raises(release_digest_record.RecordedDigestMismatchError): + release_digest_record.require_downloads_match_record( + {"a": OTHER}, trusted=_trusted({"a": DIGEST}) + ) def test_a_missing_download_is_refused() -> None: @@ -148,15 +107,18 @@ def test_a_missing_download_is_refused() -> None: Iterating the downloads would miss this entirely, which is why the comparison runs over the trusted names. """ - with pytest.raises(RecordedDigestMismatchError): - require_downloads_match_record({}, trusted={"a": DIGEST}) + with pytest.raises(release_digest_record.RecordedDigestMismatchError): + release_digest_record.require_downloads_match_record( + {}, trusted=_trusted({"a": DIGEST}) + ) def test_a_mismatch_names_every_wrong_asset() -> None: """Report all offenders in one message rather than only the first.""" - with pytest.raises(RecordedDigestMismatchError) as caught: - require_downloads_match_record( - {"a": OTHER, "b": "ef" * 32}, trusted={"a": DIGEST, "b": OTHER} + with pytest.raises(release_digest_record.RecordedDigestMismatchError) as caught: + release_digest_record.require_downloads_match_record( + {"a": OTHER, "b": "ef" * 32}, + trusted=_trusted({"a": DIGEST, "b": OTHER}), ) assert "['a', 'b']" in str(caught.value), ( @@ -167,6 +129,8 @@ def test_a_mismatch_names_every_wrong_asset() -> None: def test_a_mismatch_is_catchable_through_the_digest_hierarchy() -> None: """Derive from both hierarchies, so a caller guarding either catches it.""" digest_family = release_digest.DigestMismatchError - assert issubclass(RecordedDigestMismatchError, digest_family), ( + mismatch = release_digest_record.RecordedDigestMismatchError + + assert issubclass(mismatch, digest_family), ( "a caller guarding DigestMismatchError must not need a second clause" ) diff --git a/scripts/tests/test_release_digest_record_files.py b/scripts/tests/test_release_digest_record_files.py index 3a34e1c79..b6a8a65de 100644 --- a/scripts/tests/test_release_digest_record_files.py +++ b/scripts/tests/test_release_digest_record_files.py @@ -1,5 +1,6 @@ """Test JSON and filesystem adaptation for digest records.""" +import json import typing as typ import pytest @@ -15,6 +16,7 @@ DIGEST = "ab" * 32 +OTHER = "cd" * 32 def test_record_file_round_trips_a_valid_mapping(tmp_path: pathlib.Path) -> None: @@ -22,7 +24,7 @@ def test_record_file_round_trips_a_valid_mapping(tmp_path: pathlib.Path) -> None path = tmp_path / "record.json" write_digest_record({"a": DIGEST}, path) - assert read_digest_record(path) == {"a": DIGEST}, ( + assert read_digest_record(path).as_mapping() == {"a": DIGEST}, ( "the JSON adapter must round-trip the validated digest mapping" ) assert path.read_text(encoding="utf-8").endswith("\n"), ( @@ -30,20 +32,43 @@ def test_record_file_round_trips_a_valid_mapping(tmp_path: pathlib.Path) -> None ) +def test_the_record_is_written_in_sorted_order(tmp_path: pathlib.Path) -> None: + """Write a stable file, so two runs over one tree are byte-identical.""" + path = tmp_path / "record.json" + write_digest_record({"b": OTHER, "a": DIGEST}, path) + + assert ( + path.read_text(encoding="utf-8") + == json.dumps({"assets": {"a": DIGEST, "b": OTHER}}, indent=2, sort_keys=True) + + "\n" + ), "the record must be sorted and newline-terminated" + + def test_unreadable_path_uses_the_adapter_error(tmp_path: pathlib.Path) -> None: """Distinguish a missing file from invalid record contents.""" with pytest.raises(UnreadableDigestRecordError): read_digest_record(tmp_path / "missing.json") -def test_json_shape_errors_remain_policy_errors(tmp_path: pathlib.Path) -> None: - """Keep parsed-value validation in the pure policy module.""" +def test_json_shape_errors_are_policy_errors_not_adapter_errors( + tmp_path: pathlib.Path, +) -> None: + """Decode the member here and leave value validation to the record. + + The two error families send an operator to different places: an unusable + path is a wiring fault, while a record that parses but declares the wrong + shape is a rejected release. The adapter must not blur them. + """ path = tmp_path / "record.json" _ = path.write_text("[]", encoding="utf-8") - with pytest.raises(release_digest_record.DigestRecordError): + with pytest.raises(release_digest_record.DigestRecordError) as caught: read_digest_record(path) + assert not isinstance(caught.value, UnreadableDigestRecordError), ( + "a parsed payload is a policy rejection, not an unreadable input" + ) + @pytest.mark.parametrize( ("payload", "fragment"), diff --git a/scripts/tests/test_verify_release_inventory_cli.py b/scripts/tests/test_verify_release_inventory_cli.py index 511a74e25..637d58f16 100644 --- a/scripts/tests/test_verify_release_inventory_cli.py +++ b/scripts/tests/test_verify_release_inventory_cli.py @@ -22,6 +22,11 @@ import pathlib +def _record(digests: dict[str, str]) -> release_digest_record.DigestRecord: + """Build the trusted record an adapter decodes from a written payload.""" + return release_digest_record.DigestRecord.from_mapping(digests) + + def test_verify_accepts_the_declared_assets(tmp_path: pathlib.Path) -> None: """Accept an archive with a matching sidecar and report each asset's own digest. @@ -70,7 +75,7 @@ def test_verify_reports_a_missing_archive(tmp_path: pathlib.Path) -> None: _ = verify( staging, expected=[ARCHIVE, SIDECAR], - trusted={ARCHIVE: "ab" * 32, SIDECAR: "ab" * 32}, + trusted=_record({ARCHIVE: "ab" * 32, SIDECAR: "ab" * 32}), ) @@ -152,7 +157,7 @@ def test_verify_rejects_a_record_that_omits_a_declared_asset( """ staging = tmp_path / "dist" _stage(staging) - partial = {ARCHIVE: _trusted(staging)[ARCHIVE]} + partial = _record({ARCHIVE: _trusted(staging).as_mapping()[ARCHIVE]}) with pytest.raises(release_digest_record.DigestRecordError): _ = verify(staging, expected=[ARCHIVE, SIDECAR], trusted=partial) diff --git a/scripts/tests/test_verify_release_inventory_command.py b/scripts/tests/test_verify_release_inventory_command.py index e4bbb9b10..140133aec 100644 --- a/scripts/tests/test_verify_release_inventory_command.py +++ b/scripts/tests/test_verify_release_inventory_command.py @@ -9,6 +9,7 @@ from unittest import mock import pytest +import release_digest_record import verify_release_inventory_cli as verifier from release_staging_support import ARCHIVE, PACKAGE, SIDECAR from release_staging_support import stage_archive as _stage @@ -97,7 +98,11 @@ def test_the_cli_rejects_an_incomplete_release_and_writes_no_record( _ = (staging / SIDECAR).write_text("ab dist/x\n", encoding="utf-8") declaration = _declaration(tmp_path / "declared.txt", [ARCHIVE, SIDECAR]) trusted = _write_record( - tmp_path / "trusted.json", {ARCHIVE: "ab" * 32, SIDECAR: "ab" * 32} + tmp_path / "trusted.json", + release_digest_record.DigestRecord.from_mapping({ + ARCHIVE: "ab" * 32, + SIDECAR: "ab" * 32, + }), ) destination = tmp_path / "verified.json" diff --git a/scripts/verify_release_inventory_cli.py b/scripts/verify_release_inventory_cli.py index 64c79dbd8..1cfc5a0cb 100644 --- a/scripts/verify_release_inventory_cli.py +++ b/scripts/verify_release_inventory_cli.py @@ -54,7 +54,7 @@ def verify( directory: pathlib.Path, *, expected: list[str], - trusted: cabc.Mapping[str, str], + trusted: release_digest_record.DigestRecord, ) -> dict[str, str]: """Verify the inventory, every sidecar, and every trusted digest. @@ -65,8 +65,8 @@ def verify( expected Asset names the release declared. trusted - Published name to the digest the build recorded for the staged - candidate before upload. + The record the build wrote for the staged candidate before upload, + mapping each published name to its digest. Returns ------- @@ -229,7 +229,12 @@ def _write_verified_record(output: pathlib.Path, verified: dict[str, str]) -> No ) as tmp: staged = pathlib.Path(tmp) / output.name _ = staged.write_text( - json.dumps({"assets": verified}, indent=2, sort_keys=True) + "\n", + json.dumps( + {release_digest_record_files.ASSETS_MEMBER: verified}, + indent=2, + sort_keys=True, + ) + + "\n", encoding="utf-8", ) _ = staged.replace(output) diff --git a/tests/workflow_contracts/release_publication_modules.py b/tests/workflow_contracts/release_publication_modules.py index 2e8aab831..787b3030e 100644 --- a/tests/workflow_contracts/release_publication_modules.py +++ b/tests/workflow_contracts/release_publication_modules.py @@ -66,7 +66,6 @@ #: Sources that decide from already parsed, in-memory values. POLICY_MODULES = ( "scripts/release_candidate.py", - "scripts/release_identity_record.py", "scripts/release_semver.py", "scripts/release_asset_names.py", "scripts/release_package_formats.py", @@ -82,6 +81,7 @@ #: Sources that parse formats, touch files, or compose policy at the CLI edge. ADAPTER_MODULES = ( "scripts/release_candidate_cli.py", + "scripts/release_identity_record.py", "scripts/package_crate_cli.py", "scripts/publish_crates_io_cli.py", "scripts/release_crates_io_adapter.py", From 5e5f617fb9ea562317ad1326666cf0ec54f46901 Mon Sep 17 00:00:00 2001 From: leynos Date: Fri, 30 Oct 2026 21:41:55 +0100 Subject: [PATCH 04/16] Record release policy migration outcome Close out the issue-964 ExecPlan: mark the delivery steps done, set the status to IN PROGRESS while review and the pull request are outstanding, and record what landed, what stayed a mapping, and the gate evidence bound to migration commit 716f9b914. Check the roadmap 9.1.1 policy-boundary sub-item against ADR-0068. The parent item stays open, because its success criterion also covers thresholds, freshness rules, waivers, and retention. Co-Authored-By: Claude Code --- ...d-records-for-release-policy-boundaries.md | 78 +++++++++++++++---- docs/roadmap.md | 6 +- 2 files changed, 67 insertions(+), 17 deletions(-) diff --git a/docs/execplans/issue-964-evaluate-typed-records-for-release-policy-boundaries.md b/docs/execplans/issue-964-evaluate-typed-records-for-release-policy-boundaries.md index 6f8b8e568..1713f101c 100644 --- a/docs/execplans/issue-964-evaluate-typed-records-for-release-policy-boundaries.md +++ b/docs/execplans/issue-964-evaluate-typed-records-for-release-policy-boundaries.md @@ -1,11 +1,11 @@ # Issue 964: Evaluate typed records for release policy boundaries -Status: DRAFT +Status: IN PROGRESS -This plan is in its evidence-gathering phase. The inventory and decision -criteria below are complete; the architectural decision record, the migration, -and the documentation updates are not yet started. No production code has -changed for this issue. +Implementation is complete and verified: the decision is recorded as ADR-0068, +the migration has landed, and the full deterministic gate set is green on the +migration commit `716f9b914`. Review and the draft pull request are +outstanding, so the plan is not yet complete. ## Purpose @@ -238,18 +238,68 @@ records and are unchanged. open pull request. - [x] Write ADR-0068 and update the documentation index, design section, roadmap item, developers' guide, and trusted-packaging decision log. -- [ ] Add characterization tests. -- [ ] Migrate the staging target and digest record seams. -- [ ] Reclassify the identity decoder and update the boundary contracts. -- [ ] Run the release-publication, workflow-contract, typecheck, lint, format, - Markdown, and spelling gates. -- [ ] Update this plan's Outcomes and Status. +- [x] Add characterization tests. Commit `032c503ec` pins eight malformed + staging configurations and nine decoded digest-record shapes at the + decoders' current seams, all passing against the pre-migration code. +- [x] Migrate the staging target and digest record seams. Commit `716f9b914`. +- [x] Reclassify the identity decoder and update the boundary contracts. +- [x] Run the release-publication, workflow-contract, typecheck, lint, format, + Markdown, and spelling gates. All seven pass on `716f9b914`. +- [x] Update this plan's Outcomes and Status, and the roadmap 9.1.1 checkbox. ## Outcomes and retrospective -Not yet written. This section will record the ADR number, the migration result, -any rejection that proved harder to preserve than expected, and the final gate -evidence. +The evaluation adopted Option B, narrowed to the external-shape seams, and +recorded it as +[ADR-0068](../adrs/adr-0068-typed-records-for-release-policy-boundaries.md). +Two records now cross the boundary: `StagingTargets` (with `StagingTarget` +entries) in `release_declaration`, and `DigestRecord` in +`release_digest_record`. `release_declaration_cli` owns the `targets` and +`platform` member names and decodes the staging configuration once, before +discovery runs; `release_digest_record_files` owns `assets` and decodes the +trusted record into a value. `release_identity_record` moved from +`POLICY_MODULES` to `ADAPTER_MODULES` without a code change, because it already +decoded the record's field names into a validated identity. + +Nothing was weakened. Every rejection category survives with its original type +and diagnosis: `MissingTargetsError`, `MalformedTargetError` with +`MalformedTargetKind`, `DigestRecordError`, and `RecordedDigestMismatchError`. +The empty-record, exact-coverage, missing- download, and multi-asset mismatch +checks are unchanged, and commit identity is still an exact string comparison. +The staging decode still runs before discovery, so a malformed table fails on +the configuration rather than on a later adapter. + +Three points were less mechanical than the estimate suggested: + +- `Mapping` is invariant in its key type, so `Mapping[str, str]` cannot be + passed where `Mapping[str, object]` is expected. `DigestRecord.from_mapping` + therefore accepts `Mapping[str, object]`, and the adapter narrows the decoded + `assets` mapping with a string-key check before the cast. +- Moving the decode moved a check with it. The digest record's "names must be + strings" rule now lives in the adapter's shape check, because JSON object + keys are always strings and a non-string key can only arrive as a decoded + value. The policy record's remaining invariant is that each digest is a + string and a well-formed hexadecimal value. +- The fixture module that both verifier suites share now produces a + `DigestRecord` rather than a dict, so `write_trusted_record` serializes the + record instead of revalidating a mapping. + +The regression evidence is pinned rather than incidental. The nine +previously-rejected digest shapes and eight staging shapes were committed +before the migration and still pass after it, now asserted through the new +decoders; the policy suites construct records directly, so they test the +decision rather than the decoding. + +Gate evidence, bound to the migration commit `716f9b914`: `make check-fmt` (366 +Python files, 209 Markdown files unchanged), `make typecheck`, `make lint` +(Clippy, Whitaker, Ruff, Pylint, df12 lints, ambrleaks, Interrogate at 100%), +`make doc-coverage` (98.21% against the 80% threshold), `make markdownlint` +(209 files, no issues), `make test` (4804 tests passed, 7 skipped, doctests +clean), and `make test-workflow-contracts` (1665 passed, 3 skipped). The +documentation commit that carries this update is re-gated on the branch head. +Roadmap item 9.1.1 stays open, because its success criterion covers thresholds, +freshness rules, waivers, and retention as well; its policy-boundary sub-item +is checked against ADR-0068. ## References diff --git a/docs/roadmap.md b/docs/roadmap.md index d6113584a..c82f6d3a2 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -2022,9 +2022,9 @@ must share. first-version evidence needs external attestation. - Record substantive decisions as ADRs and update release architecture and developer documentation. - - See - [ADR-0068](adrs/adr-0068-typed-records-for-release-policy-boundaries.md) - for the release policy boundary decision. + - [x] Decide the release policy boundary: frozen slotted records at every + external-shape seam, with domain-shaped mappings retained. See + [ADR-0068](adrs/adr-0068-typed-records-for-release-policy-boundaries.md). - See [RFC 0005 §Open questions](rfcs/rfc-0005-harden-release-integrity-and-admission.md#open-questions) and From b3aa2504a3906a5d6ccfaf76bb9f7fab98be065f Mon Sep 17 00:00:00 2001 From: leynos Date: Fri, 30 Oct 2026 22:29:46 +0100 Subject: [PATCH 05/16] Bind record invariants at construction CodeRabbit found that `StagingTargets` and `DigestRecord` enforced their invariants only inside their factories. Both records are accepted by policy functions that never call those factories, so a directly constructed record could reach a comparison unvalidated: an empty staging set would declare archives and sidecars only, and an empty digest record would verify nothing. Move the invariant into `__post_init__` on both records, leaving `from_mapping` and `from_targets` as normalizing and wrapping factories. The digest record's type and hexadecimal checks are now shared by one `_validated_entry` helper, which the factory also uses, so the normalization path and the canonical-form rule stay in step. A non-string asset key is still refused before `sorted` can raise `TypeError`. Tests cover both construction paths: the staging suite is parametrized over direct construction and the factory, and the digest suite rebuilds each refusal without the factory. Co-Authored-By: Claude Code --- ...d-records-for-release-policy-boundaries.md | 21 +++-- scripts/release_declaration.py | 40 +++++++-- scripts/release_digest_record.py | 86 +++++++++++++++---- scripts/tests/test_release_declaration.py | 28 +++++- scripts/tests/test_release_digest_record.py | 43 +++++++++- 5 files changed, 181 insertions(+), 37 deletions(-) diff --git a/docs/execplans/issue-964-evaluate-typed-records-for-release-policy-boundaries.md b/docs/execplans/issue-964-evaluate-typed-records-for-release-policy-boundaries.md index 1713f101c..ba02b5156 100644 --- a/docs/execplans/issue-964-evaluate-typed-records-for-release-policy-boundaries.md +++ b/docs/execplans/issue-964-evaluate-typed-records-for-release-policy-boundaries.md @@ -264,7 +264,7 @@ decoded the record's field names into a validated identity. Nothing was weakened. Every rejection category survives with its original type and diagnosis: `MissingTargetsError`, `MalformedTargetError` with `MalformedTargetKind`, `DigestRecordError`, and `RecordedDigestMismatchError`. -The empty-record, exact-coverage, missing- download, and multi-asset mismatch +The empty-record, exact-coverage, missing-download, and multi-asset mismatch checks are unchanged, and commit identity is still an exact string comparison. The staging decode still runs before discovery, so a malformed table fails on the configuration rather than on a later adapter. @@ -275,15 +275,24 @@ Three points were less mechanical than the estimate suggested: passed where `Mapping[str, object]` is expected. `DigestRecord.from_mapping` therefore accepts `Mapping[str, object]`, and the adapter narrows the decoded `assets` mapping with a string-key check before the cast. -- Moving the decode moved a check with it. The digest record's "names must be - strings" rule now lives in the adapter's shape check, because JSON object - keys are always strings and a non-string key can only arrive as a decoded - value. The policy record's remaining invariant is that each digest is a - string and a well-formed hexadecimal value. +- Moving the decode moved a check with it. The adapter rejects a payload whose + `assets` keys are not strings, because JSON object keys are always strings + and a non-string key can only arrive as an already decoded value. The record + keeps the same rule in `__post_init__` as well, so `from_mapping` fails with + `DigestRecordError` rather than `TypeError` when a mixed-type key set would + break `sorted`. - The fixture module that both verifier suites share now produces a `DigestRecord` rather than a dict, so `write_trusted_record` serializes the record instead of revalidating a mapping. +Review of the migration tightened where those invariants live. Both records now +validate in `__post_init__` as well as in their factories, so a directly +constructed record cannot reach the comparison functions unvalidated; the +staging suite exercises both construction paths, and the digest suite rebuilds +each refusal without the factory. One `_validated_entry` helper holds the +digest record's type and hexadecimal checks, so the factory's case +normalization and the record's canonical-form rule cannot drift apart. + The regression evidence is pinned rather than incidental. The nine previously-rejected digest shapes and eight staging shapes were committed before the migration and still pass after it, now asserted through the new diff --git a/scripts/release_declaration.py b/scripts/release_declaration.py index a4ace0eec..4966cb03c 100644 --- a/scripts/release_declaration.py +++ b/scripts/release_declaration.py @@ -73,19 +73,44 @@ class StagingTargets: #: One entry per configured target. targets: tuple[StagingTarget, ...] + def __post_init__(self) -> None: + """Refuse a record that names no target. + + Emptiness is refused here rather than in the factory, because + :func:`declared_asset_names` accepts any record. A directly built + empty record would declare archives and sidecars only, which is the + smaller release this check exists to prevent. + + Raises + ------ + MissingTargetsError + If the record carries no target. + + Examples + -------- + >>> StagingTargets((StagingTarget("linux"),)).targets + (StagingTarget(platform='linux'),) + >>> StagingTargets(()) # doctest: +ELLIPSIS + Traceback (most recent call last): + ... + release_package_formats.MissingTargetsError: staging configuration defines... + """ + if not self.targets: + raise MissingTargetsError + @classmethod def from_targets(cls, targets: cabc.Iterable[StagingTarget]) -> StagingTargets: - """Build the record, refusing a configuration that names no target. + """Build the record from a decoded configuration. Returns ------- StagingTargets The decoded staging targets. - Raises - ------ - MissingTargetsError - If ``targets`` is empty. + Notes + ----- + The non-empty invariant belongs to :meth:`__post_init__`, so every + construction path refuses an empty configuration. Examples -------- @@ -96,10 +121,7 @@ def from_targets(cls, targets: cabc.Iterable[StagingTarget]) -> StagingTargets: ... release_package_formats.MissingTargetsError: staging configuration defines... """ - entries = tuple(targets) - if not entries: - raise MissingTargetsError - return cls(entries) + return cls(tuple(targets)) def archive_asset_names(archives: cabc.Iterable[str]) -> tuple[str, ...]: diff --git a/scripts/release_digest_record.py b/scripts/release_digest_record.py index 5875a288e..46c36210f 100644 --- a/scripts/release_digest_record.py +++ b/scripts/release_digest_record.py @@ -43,6 +43,37 @@ class DigestRecord: #: Asset names paired with their lowercase SHA-256 digest, sorted by name. entries: tuple[tuple[str, str], ...] + def __post_init__(self) -> None: + """Refuse a record that is empty or carries a malformed entry. + + The comparison functions accept any :class:`DigestRecord`, so the + guarantee that a record is usable has to hold for every construction + path rather than only for the factory. Validation here is what makes + "validated record" a property of the type. + + Raises + ------ + DigestRecordError + If the record declares no asset, names an asset with a non-string + key, or carries a digest that is not a lowercase hexadecimal value. + + Examples + -------- + >>> DigestRecord((("a", "ab" * 32),)).names() + {'a'} + >>> DigestRecord(()) # doctest: +ELLIPSIS + Traceback (most recent call last): + ... + release_digest_record.DigestRecordError: the digest record declares no assets... + """ + if not self.entries: + msg = "the digest record declares no assets, so there is nothing to verify" + raise DigestRecordError(msg) + for name, digest in self.entries: + if _validated_entry(name, digest) != (name, digest): + msg = f"the digest record must carry lowercase digests, got {digest!r}" + raise DigestRecordError(msg) + @classmethod def from_mapping(cls, declared: cabc.Mapping[str, object]) -> DigestRecord: """Validate decoded entries and build the trusted record. @@ -57,11 +88,11 @@ def from_mapping(cls, declared: cabc.Mapping[str, object]) -> DigestRecord: DigestRecord The entries with lowercase digests, sorted by asset name. - Raises - ------ - DigestRecordError - If the mapping is empty or an entry is not a string name and a - valid hexadecimal digest. + Notes + ----- + This factory normalizes case and sorts; the value invariant itself is + enforced by :meth:`__post_init__`, whose ``DigestRecordError`` reaches + the caller unchanged. Examples -------- @@ -72,11 +103,7 @@ def from_mapping(cls, declared: cabc.Mapping[str, object]) -> DigestRecord: ... release_digest_record.DigestRecordError: the digest record declares no assets... """ - entries = tuple(sorted(_validated_entries(declared))) - if not entries: - msg = "the digest record declares no assets, so there is nothing to verify" - raise DigestRecordError(msg) - return cls(entries) + return cls(tuple(sorted(_validated_entries(declared)))) def as_mapping(self) -> dict[str, str]: """Return the record as a name-to-digest mapping. @@ -119,21 +146,46 @@ def _validated_entries( tuple[str, str] One asset name and its lowercase digest. + Notes + ----- + :func:`_validated_entry` raises ``DigestRecordError`` for a malformed + entry. Its check runs as each entry is yielded, so a malformed key is + refused before ``sorted`` compares it against a string key. Sorting first + would raise ``TypeError`` for mixed key types instead of the record's own + error. + + Examples + -------- + >>> list(_validated_entries({"a": "AB" * 32}))[0][1] == "ab" * 32 + True + """ + for name, digest in declared.items(): + yield _validated_entry(name, digest) + + +def _validated_entry(name: object, digest: object) -> tuple[str, str]: + """Return one asset name and its lowercase digest, or refuse the entry. + + Returns + ------- + tuple[str, str] + The asset name and its lowercase digest. + Raises ------ DigestRecordError - If a digest is not a string or not a valid hexadecimal value. + If either value is not a string, or the digest is not a valid + hexadecimal value. Examples -------- - >>> list(_validated_entries({"a": "AB" * 32}))[0][1] == "ab" * 32 + >>> _validated_entry("a", "AB" * 32)[1] == "ab" * 32 True """ - for name, digest in declared.items(): - if not isinstance(digest, str): - msg = "the digest record must map asset names to digests as strings" - raise DigestRecordError(msg) - yield name, _validated_digest(name, digest) + if not isinstance(name, str) or not isinstance(digest, str): + msg = "the digest record must map asset names to digests as strings" + raise DigestRecordError(msg) + return name, _validated_digest(name, digest) def _validated_digest(name: str, digest: str) -> str: diff --git a/scripts/tests/test_release_declaration.py b/scripts/tests/test_release_declaration.py index 97210c901..a1e99a227 100644 --- a/scripts/tests/test_release_declaration.py +++ b/scripts/tests/test_release_declaration.py @@ -1,8 +1,13 @@ """Specify the pure release-asset naming and package-count policy.""" +import typing as typ + import pytest import release_declaration +if typ.TYPE_CHECKING: + import collections.abc as cabc + ARCHIVES = ( "netsuke-build-0.1.0-beta4-aarch64-apple-darwin.tar.gz", "netsuke-build-0.1.0-beta4-aarch64-pc-windows-msvc.tar.gz", @@ -105,10 +110,27 @@ def test_a_platform_with_no_declared_format_is_refused() -> None: ) -def test_a_configuration_without_targets_is_refused() -> None: - """Refuse an empty target set rather than declare a smaller release.""" +@pytest.mark.parametrize( + "build", + [ + pytest.param(release_declaration.StagingTargets, id="direct-construction"), + pytest.param(release_declaration.StagingTargets.from_targets, id="factory"), + ], +) +def test_a_configuration_without_targets_is_refused( + build: cabc.Callable[ + [tuple[release_declaration.StagingTarget, ...]], + release_declaration.StagingTargets, + ], +) -> None: + """Refuse an empty target set rather than declare a smaller release. + + Both construction paths are exercised: the invariant belongs to the + record, so a caller that skips the factory must not obtain a record the + policy would then under-declare from. + """ with pytest.raises(release_declaration.MissingTargetsError): - _ = release_declaration.StagingTargets.from_targets(()) + _ = build(()) def test_package_names_are_observed_from_the_supplied_tree_facts() -> None: diff --git a/scripts/tests/test_release_digest_record.py b/scripts/tests/test_release_digest_record.py index 638481229..72ecfb1aa 100644 --- a/scripts/tests/test_release_digest_record.py +++ b/scripts/tests/test_release_digest_record.py @@ -8,6 +8,8 @@ that produce each refusal. """ +import typing as typ + import pytest import release_digest import release_digest_record @@ -21,8 +23,8 @@ def _trusted(digests: dict[str, object]) -> release_digest_record.DigestRecord: return release_digest_record.DigestRecord.from_mapping(digests) -def test_a_record_normalises_digest_case() -> None: - """Accept a digest written in either case, normalising on the way in.""" +def test_a_record_normalizes_digest_case() -> None: + """Accept a digest written in either case, normalizing on the way in.""" record = _trusted({"a": DIGEST.upper()}) assert record.as_mapping() == {"a": DIGEST}, ( @@ -30,6 +32,43 @@ def test_a_record_normalises_digest_case() -> None: ) +@pytest.mark.parametrize( + ("entries", "message"), + [ + pytest.param((), "declares no assets", id="empty"), + pytest.param(((1, DIGEST),), "as strings", id="non-string-name"), + pytest.param((("a", DIGEST.upper()),), "lowercase", id="un-normalized"), + pytest.param((("a", "nothex"),), "non-digest", id="not-hexadecimal"), + pytest.param((("a", 1),), "as strings", id="non-string-digest"), + ], +) +def test_a_directly_constructed_record_is_validated( + entries: tuple[tuple[object, object], ...], message: str +) -> None: + """Refuse an invalid record however the caller built it. + + The comparison functions accept any record, so the invariant has to hold at + construction rather than at the factory. These cases skip + :meth:`DigestRecord.from_mapping` entirely. + """ + malformed = typ.cast("tuple[tuple[str, str], ...]", entries) + + with pytest.raises(release_digest_record.DigestRecordError, match=message): + _ = release_digest_record.DigestRecord(malformed) + + +def test_a_non_string_asset_name_is_refused_before_sorting() -> None: + """Raise the record's own error, not ``TypeError``, for a bad key type. + + Sorting a mixed-type key set raises ``TypeError``, which callers do not + catch. The contract is that a malformed mapping is a rejected record. + """ + malformed = typ.cast("dict[str, object]", {1: DIGEST}) + + with pytest.raises(release_digest_record.DigestRecordError, match="as strings"): + _ = _trusted(malformed) + + def test_an_empty_record_is_refused() -> None: """Refuse a record that would verify nothing.""" with pytest.raises(release_digest_record.DigestRecordError): From fce37c2a8b7d67813aaa6d94a7983dfb193d47fc Mon Sep 17 00:00:00 2001 From: leynos Date: Fri, 30 Oct 2026 23:18:34 +0100 Subject: [PATCH 06/16] Refuse repeated asset names in the digest record CodeRabbit found that a directly constructed `DigestRecord` could list one asset name twice. `from_mapping` cannot produce that state, because a mapping cannot repeat a key, but direct construction can, and every accessor collapses the repetition: `as_mapping` keeps the last digest and `require_downloads_match_record` therefore compares the later digest while the earlier one is never checked. Refuse the repetition in `__post_init__`, alongside the other invariants that make a validated record a property of the type. The parametrized direct-construction table gains a `duplicate-name` case. Co-Authored-By: Claude Code --- ...d-records-for-release-policy-boundaries.md | 9 +++-- scripts/release_digest_record.py | 33 ++++++++++++++++++- scripts/tests/test_release_digest_record.py | 5 +++ 3 files changed, 43 insertions(+), 4 deletions(-) diff --git a/docs/execplans/issue-964-evaluate-typed-records-for-release-policy-boundaries.md b/docs/execplans/issue-964-evaluate-typed-records-for-release-policy-boundaries.md index ba02b5156..4fdeae36b 100644 --- a/docs/execplans/issue-964-evaluate-typed-records-for-release-policy-boundaries.md +++ b/docs/execplans/issue-964-evaluate-typed-records-for-release-policy-boundaries.md @@ -289,9 +289,12 @@ Review of the migration tightened where those invariants live. Both records now validate in `__post_init__` as well as in their factories, so a directly constructed record cannot reach the comparison functions unvalidated; the staging suite exercises both construction paths, and the digest suite rebuilds -each refusal without the factory. One `_validated_entry` helper holds the -digest record's type and hexadecimal checks, so the factory's case -normalization and the record's canonical-form rule cannot drift apart. +each refusal without the factory. The digest record also refuses a repeated +asset name on that path, because a mapping cannot repeat a key and every +accessor collapses the repetition, so the earlier digest would otherwise never +be compared. One `_validated_entry` helper holds the digest record's type and +hexadecimal checks, so the factory's case normalization and the record's +canonical-form rule cannot drift apart. The regression evidence is pinned rather than incidental. The nine previously-rejected digest shapes and eight staging shapes were committed diff --git a/scripts/release_digest_record.py b/scripts/release_digest_record.py index 46c36210f..04a102adb 100644 --- a/scripts/release_digest_record.py +++ b/scripts/release_digest_record.py @@ -55,7 +55,8 @@ def __post_init__(self) -> None: ------ DigestRecordError If the record declares no asset, names an asset with a non-string - key, or carries a digest that is not a lowercase hexadecimal value. + key, lists an asset name more than once, or carries a digest that + is not a lowercase hexadecimal value. Examples -------- @@ -73,6 +74,7 @@ def __post_init__(self) -> None: if _validated_entry(name, digest) != (name, digest): msg = f"the digest record must carry lowercase digests, got {digest!r}" raise DigestRecordError(msg) + _require_unique_names(self.entries) @classmethod def from_mapping(cls, declared: cabc.Mapping[str, object]) -> DigestRecord: @@ -163,6 +165,35 @@ def _validated_entries( yield _validated_entry(name, digest) +def _require_unique_names(entries: cabc.Iterable[tuple[str, str]]) -> None: + """Refuse a record that names one asset more than once. + + A repeated name is reachable only by constructing the record directly, + because a mapping cannot repeat a key. Every accessor collapses the + repetition, so only the later digest would be compared and the earlier + digest would never be checked against anything. + + Raises + ------ + DigestRecordError + If two entries share an asset name. + + Examples + -------- + >>> _require_unique_names([("a", "ab" * 32), ("b", "ab" * 32)]) + >>> _require_unique_names( # doctest: +ELLIPSIS + ... [("a", "ab" * 32), ("a", "ab" * 32)] + ... ) + Traceback (most recent call last): + ... + release_digest_record.DigestRecordError: the digest record must list each... + """ + names = [name for name, _ in entries] + if len(set(names)) != len(names): + msg = "the digest record must list each asset name once" + raise DigestRecordError(msg) + + def _validated_entry(name: object, digest: object) -> tuple[str, str]: """Return one asset name and its lowercase digest, or refuse the entry. diff --git a/scripts/tests/test_release_digest_record.py b/scripts/tests/test_release_digest_record.py index 72ecfb1aa..71cd1a720 100644 --- a/scripts/tests/test_release_digest_record.py +++ b/scripts/tests/test_release_digest_record.py @@ -40,6 +40,11 @@ def test_a_record_normalizes_digest_case() -> None: pytest.param((("a", DIGEST.upper()),), "lowercase", id="un-normalized"), pytest.param((("a", "nothex"),), "non-digest", id="not-hexadecimal"), pytest.param((("a", 1),), "as strings", id="non-string-digest"), + pytest.param( + (("a", DIGEST), ("a", OTHER)), + "each asset name once", + id="duplicate-name", + ), ], ) def test_a_directly_constructed_record_is_validated( From a7d5a0aa260b3ecc18611fec5c6fb04959562fbb Mon Sep 17 00:00:00 2001 From: leynos Date: Sat, 31 Oct 2026 00:07:51 +0100 Subject: [PATCH 07/16] Parametrize the digest-record refusal cases CodeRabbit noted that four tests built a record through `_trusted` and expected `DigestRecordError`, differing only in the input. The repository asks for duplicated tests to be replaced with parameterized cases. Fold the empty, non-string-digest, malformed-digest, and one-bad-entry cases into one table with a case-specific id and expected message for each row, and keep the reasoning that made them distinct as the docstring. Co-Authored-By: Claude Code --- scripts/tests/test_release_digest_record.py | 42 ++++++++++----------- 1 file changed, 20 insertions(+), 22 deletions(-) diff --git a/scripts/tests/test_release_digest_record.py b/scripts/tests/test_release_digest_record.py index 71cd1a720..681a002f3 100644 --- a/scripts/tests/test_release_digest_record.py +++ b/scripts/tests/test_release_digest_record.py @@ -74,29 +74,27 @@ def test_a_non_string_asset_name_is_refused_before_sorting() -> None: _ = _trusted(malformed) -def test_an_empty_record_is_refused() -> None: - """Refuse a record that would verify nothing.""" - with pytest.raises(release_digest_record.DigestRecordError): - _ = _trusted({}) - - -def test_a_digest_that_is_not_a_string_is_refused() -> None: - """Refuse a value the comparison could never match.""" - with pytest.raises(release_digest_record.DigestRecordError): - _ = _trusted({"a": 1}) - - -@pytest.mark.parametrize("digest", ["nothex", "ab"], ids=["not-hex", "too-short"]) -def test_a_digest_that_is_not_a_digest_is_refused(digest: str) -> None: - """Refuse a malformed digest rather than comparing it as text.""" - with pytest.raises(release_digest_record.DigestRecordError): - _ = _trusted({"a": digest}) - +@pytest.mark.parametrize( + ("declared", "message"), + [ + pytest.param({}, "declares no assets", id="empty"), + pytest.param({"a": 1}, "as strings", id="non-string-digest"), + pytest.param({"a": "nothex"}, "non-digest", id="not-hex"), + pytest.param({"a": "ab"}, "non-digest", id="too-short"), + pytest.param({"a": DIGEST, "b": 2}, "as strings", id="one-bad-entry"), + ], +) +def test_an_unusable_mapping_refuses_the_whole_record( + declared: dict[str, object], message: str +) -> None: + """Refuse the record, not merely the entry, so no asset is left unchecked. -def test_one_bad_entry_refuses_the_whole_record() -> None: - """Refuse the record, not merely the entry, so no asset is left unchecked.""" - with pytest.raises(release_digest_record.DigestRecordError): - _ = _trusted({"a": DIGEST, "b": 2}) + An empty mapping would verify nothing, a value the comparison could never + match would silently pass an equality check, and a malformed digest must be + diagnosed rather than compared as text. + """ + with pytest.raises(release_digest_record.DigestRecordError, match=message): + _ = _trusted(declared) def test_a_covering_record_is_accepted() -> None: From 470cd52431616fa63dc1ed5a90fd6ec2c45969a3 Mon Sep 17 00:00:00 2001 From: leynos Date: Sat, 31 Oct 2026 00:38:34 +0100 Subject: [PATCH 08/16] Correct the variance note and sharpen the sorting guard CodeRabbit found two defects in the review-fix follow-up. The ExecPlan said `Mapping` is invariant in its key type, which is irrelevant here because both key types are `str`. The reason `DigestRecord.from_mapping` accepts `Mapping[str, object]` is that it receives unvalidated decoded JSON values; `Mapping` is covariant in its value type, so the wide value type is about admitting the payload, not about key variance. The sorting guard used a one-key fixture, and `sorted` never compares a single element, so a sort-before-validate regression would have passed. Adding a string key beside the integer key makes that regression raise `TypeError` and fail the test. Co-Authored-By: Claude Code --- ...uate-typed-records-for-release-policy-boundaries.md | 10 ++++++---- scripts/tests/test_release_digest_record.py | 2 +- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/docs/execplans/issue-964-evaluate-typed-records-for-release-policy-boundaries.md b/docs/execplans/issue-964-evaluate-typed-records-for-release-policy-boundaries.md index 4fdeae36b..4cf03a180 100644 --- a/docs/execplans/issue-964-evaluate-typed-records-for-release-policy-boundaries.md +++ b/docs/execplans/issue-964-evaluate-typed-records-for-release-policy-boundaries.md @@ -271,10 +271,12 @@ the configuration rather than on a later adapter. Three points were less mechanical than the estimate suggested: -- `Mapping` is invariant in its key type, so `Mapping[str, str]` cannot be - passed where `Mapping[str, object]` is expected. `DigestRecord.from_mapping` - therefore accepts `Mapping[str, object]`, and the adapter narrows the decoded - `assets` mapping with a string-key check before the cast. +- `DigestRecord.from_mapping` receives unvalidated decoded JSON values, so its + parameter accepts `Mapping[str, object]` rather than `Mapping[str, str]`. + `Mapping` is covariant in its value type, so an already typed mapping would + also be accepted; the wide value type is what lets the decoded payload in. + The adapter narrows the decoded `assets` mapping with a string-key check + before the cast. - Moving the decode moved a check with it. The adapter rejects a payload whose `assets` keys are not strings, because JSON object keys are always strings and a non-string key can only arrive as an already decoded value. The record diff --git a/scripts/tests/test_release_digest_record.py b/scripts/tests/test_release_digest_record.py index 681a002f3..3773de6b8 100644 --- a/scripts/tests/test_release_digest_record.py +++ b/scripts/tests/test_release_digest_record.py @@ -68,7 +68,7 @@ def test_a_non_string_asset_name_is_refused_before_sorting() -> None: Sorting a mixed-type key set raises ``TypeError``, which callers do not catch. The contract is that a malformed mapping is a rejected record. """ - malformed = typ.cast("dict[str, object]", {1: DIGEST}) + malformed = typ.cast("dict[str, object]", {"a": DIGEST, 1: DIGEST}) with pytest.raises(release_digest_record.DigestRecordError, match="as strings"): _ = _trusted(malformed) From d2ea10a6d88faa66855753591d1d314eb0854ea2 Mon Sep 17 00:00:00 2001 From: leynos Date: Sat, 31 Oct 2026 01:07:56 +0100 Subject: [PATCH 09/16] Record the review follow-up in the ExecPlan Keep the plan authoritative for anyone resuming the work: the Progress list now names the invariants bound at construction, the duplicate-name refusal, the parametrized refusal table, the corrected variance note, and the sharpened sorting guard. Co-Authored-By: Claude Code --- ...4-evaluate-typed-records-for-release-policy-boundaries.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/docs/execplans/issue-964-evaluate-typed-records-for-release-policy-boundaries.md b/docs/execplans/issue-964-evaluate-typed-records-for-release-policy-boundaries.md index 4cf03a180..67dacd402 100644 --- a/docs/execplans/issue-964-evaluate-typed-records-for-release-policy-boundaries.md +++ b/docs/execplans/issue-964-evaluate-typed-records-for-release-policy-boundaries.md @@ -246,6 +246,11 @@ records and are unchanged. - [x] Run the release-publication, workflow-contract, typecheck, lint, format, Markdown, and spelling gates. All seven pass on `716f9b914`. - [x] Update this plan's Outcomes and Status, and the roadmap 9.1.1 checkbox. +- [x] Address review findings and re-gate. Invariants are bound at + construction, a repeated asset name is refused, the refusal cases are + parametrized, the variance note is corrected, and the sorting guard now + uses a mixed-type key set. The full gate set is re-run on the branch + head. ## Outcomes and retrospective From 39c2565e47d9967399730430a41c65d09261b222 Mon Sep 17 00:00:00 2001 From: leynos Date: Sat, 31 Oct 2026 03:24:22 +0100 Subject: [PATCH 10/16] Bind the staging target and sharpen the config diagnostics CodeRabbit found three defects in the record boundary. `StagingTarget` accepted any value, so `StagingTarget(None)` beside a string platform reached `sorted` as a mixed-type key set and raised `TypeError` instead of a configuration diagnosis. The target now refuses a platform that is not a non-empty string with the existing `MalformedTargetKind.MISSING_PLATFORM` category; an unknown platform *name* still reaches `UnknownPlatformError` in policy, so no rejection category changes. Every `ConfigurationError` exits with the same code, so the malformed configuration suite could not distinguish a missing targets table from an unusable target entry. Each case now carries its expected diagnosis and asserts it against standard error, with the cases collected in a `Case` named tuple so the test keeps three parameters. The developers' guide said the record's constructor owns normalization. It does not: `from_mapping` lowercases a digest while `__post_init__` refuses one that is not already lowercase. The guide now states that split. Co-Authored-By: Claude Code --- docs/developers-guide.md | 20 ++-- ...d-records-for-release-policy-boundaries.md | 12 +- scripts/release_declaration.py | 27 +++++ scripts/tests/test_release_declaration.py | 20 ++++ ...test_release_declaration_invalid_config.py | 109 ++++++++++++++---- 5 files changed, 153 insertions(+), 35 deletions(-) diff --git a/docs/developers-guide.md b/docs/developers-guide.md index 4ca3dc648..77be6125a 100644 --- a/docs/developers-guide.md +++ b/docs/developers-guide.md @@ -2684,15 +2684,17 @@ those operations at the file or transport boundary: External record shapes cross the boundary exactly once. A module that already owns a file, stream, or provider response decodes its field names and shape into a frozen slotted dataclass, and policy functions accept that record. The -record's constructor owns the value invariant, including any normalization such -as lowercasing a digest, and the adapter owns the wire names, including -`targets`, `platform`, and `assets`. `StagingTargets`, `DigestRecord`, -`ReleaseIdentity`, and `PublishedAsset` follow this rule. A mapping remains -acceptable when the policy decides over a domain-shaped name-to-value relation -that no external format defines, such as the name-to-digest plans in -`release_reconcile_github` and the computed side of a download comparison; -adding a decoder for one of those would invent a format boundary that does not -exist. See +record's constructor owns the value invariant, a decoding factory owns any +normalization, and the adapter owns the wire names, including `targets`, +`platform`, and `assets`. `DigestRecord` shows the split: `from_mapping` +lowercases a digest, while `__post_init__` refuses one that is not already +lowercase, so a directly constructed record carries the same invariant as a +decoded one. `StagingTargets`, `ReleaseIdentity`, and `PublishedAsset` follow +the same rule. A mapping remains acceptable when the policy decides over a +domain-shaped name-to-value relation that no external format defines, such as +the name-to-digest plans in `release_reconcile_github` and the computed side of +a download comparison; adding a decoder for one of those would invent a format +boundary that does not exist. See [ADR-0068](adrs/adr-0068-typed-records-for-release-policy-boundaries.md). Each adapter has a narrow scope and named callers. File adapters do not own diff --git a/docs/execplans/issue-964-evaluate-typed-records-for-release-policy-boundaries.md b/docs/execplans/issue-964-evaluate-typed-records-for-release-policy-boundaries.md index 67dacd402..dc40dc3db 100644 --- a/docs/execplans/issue-964-evaluate-typed-records-for-release-policy-boundaries.md +++ b/docs/execplans/issue-964-evaluate-typed-records-for-release-policy-boundaries.md @@ -299,10 +299,18 @@ staging suite exercises both construction paths, and the digest suite rebuilds each refusal without the factory. The digest record also refuses a repeated asset name on that path, because a mapping cannot repeat a key and every accessor collapses the repetition, so the earlier digest would otherwise never -be compared. One `_validated_entry` helper holds the digest record's type and -hexadecimal checks, so the factory's case normalization and the record's +be compared. `StagingTarget` likewise refuses a platform that is not a +non-empty string, since counting and sorting the configured platforms would +otherwise compare a mixed-type key set; an unknown platform *name* remains a +policy refusal. One `_validated_entry` helper holds the digest record's type +and hexadecimal checks, so the factory's case normalization and the record's canonical-form rule cannot drift apart. +Every configuration error exits with the same code, so the malformed +configuration suite now asserts the category-specific diagnosis on standard +error as well. Without it, treating a malformed target entry as a missing +targets table would satisfy the exit-code assertion for both cases. + The regression evidence is pinned rather than incidental. The nine previously-rejected digest shapes and eight staging shapes were committed before the migration and still pass after it, now asserted through the new diff --git a/scripts/release_declaration.py b/scripts/release_declaration.py index 4966cb03c..6800fc590 100644 --- a/scripts/release_declaration.py +++ b/scripts/release_declaration.py @@ -25,6 +25,7 @@ DeclarationError, DuplicatePackageError, MalformedTargetError, + MalformedTargetKind, MissingPackageError, MissingTargetsError, StagedTreeError, @@ -65,6 +66,32 @@ class StagingTarget: #: The platform value that selects this target's package formats. platform: str + def __post_init__(self) -> None: + """Refuse a target that names no platform string. + + The declaration counts platforms and sorts them, so a non-string + platform reaches ``sorted`` as a mixed-type key set and fails with + ``TypeError`` rather than a configuration diagnosis. An unknown + platform *name* stays a policy refusal, raised by + :func:`declared_asset_names`. + + Raises + ------ + MalformedTargetError + If the platform is not a non-empty string. + + Examples + -------- + >>> StagingTarget("linux").platform + 'linux' + >>> StagingTarget("") # doctest: +ELLIPSIS + Traceback (most recent call last): + ... + release_package_formats.MalformedTargetError: staging configuration has... + """ + if not isinstance(self.platform, str) or not self.platform: + raise MalformedTargetError(MalformedTargetKind.MISSING_PLATFORM) + @dataclasses.dataclass(frozen=True, slots=True) class StagingTargets: diff --git a/scripts/tests/test_release_declaration.py b/scripts/tests/test_release_declaration.py index a1e99a227..4312b2cbf 100644 --- a/scripts/tests/test_release_declaration.py +++ b/scripts/tests/test_release_declaration.py @@ -133,6 +133,26 @@ def test_a_configuration_without_targets_is_refused( _ = build(()) +@pytest.mark.parametrize( + "platform", + [ + pytest.param(None, id="absent"), + pytest.param(1, id="integer"), + pytest.param("", id="empty"), + ], +) +def test_a_target_without_a_platform_string_is_refused(platform: object) -> None: + """Refuse a target whose platform cannot key a package-format table. + + Counting and sorting the configured platforms would compare a mixed-type + key set and raise ``TypeError``, so the target refuses the value itself. + """ + malformed = typ.cast("str", platform) + + with pytest.raises(release_declaration.MalformedTargetError): + _ = release_declaration.StagingTarget(malformed) + + def test_package_names_are_observed_from_the_supplied_tree_facts() -> None: """Carry an action-provided package name without predicting its spelling.""" renamed = (*PACKAGES[:-1], "renamed-by-the-action.msi") diff --git a/scripts/tests/test_release_declaration_invalid_config.py b/scripts/tests/test_release_declaration_invalid_config.py index 7a7646588..5fa00c45c 100644 --- a/scripts/tests/test_release_declaration_invalid_config.py +++ b/scripts/tests/test_release_declaration_invalid_config.py @@ -9,35 +9,92 @@ from pathlib import Path +class Case(typ.NamedTuple): + """One malformed staging configuration and the diagnosis it must produce.""" + + config: str + diagnosis: str + reason: str + + @pytest.mark.parametrize( - ("config", "reason"), + "case", [ - ('targets = "bad"\n', "the targets member is not a table"), - ("targets = []\n", "a list is not a target table"), - ("targets = {}\n", "an empty table declares no release target"), - ("common = {}\n", "an absent targets table declares no release target"), - ('[targets]\nlinux = "bad"\n', "a target entry is not a table"), - ("[targets]\nlinux = 1\n", "a target entry is not a table"), - ("[targets]\nlinux = {}\n", "a target table has no platform"), - ("[targets]\nlinux = { platform = 1 }\n", "a platform must be a string"), - ], - ids=[ - "targets-not-a-table", - "targets-is-a-list", - "targets-empty", - "targets-absent", - "entry-is-a-string", - "entry-is-an-integer", - "entry-has-no-platform", - "platform-is-an-integer", + pytest.param( + Case( + 'targets = "bad"\n', + "no release targets", + "the targets member is not a table", + ), + id="targets-not-a-table", + ), + pytest.param( + Case( + "targets = []\n", "no release targets", "a list is not a target table" + ), + id="targets-is-a-list", + ), + pytest.param( + Case( + "targets = {}\n", + "no release targets", + "an empty table declares no release target", + ), + id="targets-empty", + ), + pytest.param( + Case( + "common = {}\n", + "no release targets", + "an absent targets table declares no release target", + ), + id="targets-absent", + ), + pytest.param( + Case( + '[targets]\nlinux = "bad"\n', + "target without a platform", + "a target entry is not a table", + ), + id="entry-is-a-string", + ), + pytest.param( + Case( + "[targets]\nlinux = 1\n", + "target without a platform", + "a target entry is not a table", + ), + id="entry-is-an-integer", + ), + pytest.param( + Case( + "[targets]\nlinux = {}\n", + "target without a platform", + "a target table has no platform", + ), + id="entry-has-no-platform", + ), + pytest.param( + Case( + "[targets]\nlinux = { platform = 1 }\n", + "target without a platform", + "a platform must be a string", + ), + id="platform-is-an-integer", + ), ], ) def test_malformed_targets_are_reported_as_input_faults( - tmp_path: Path, config: str, reason: str, capsys: pytest.CaptureFixture[str] + tmp_path: Path, case: Case, capsys: pytest.CaptureFixture[str] ) -> None: - """Reject malformed target tables without a traceback or output.""" + """Reject malformed target tables without a traceback or output. + + Every configuration error exits with the same code, so the diagnosis is + what distinguishes a missing targets table from an unusable target entry. + Asserting it keeps each preserved rejection category under test. + """ staging = tmp_path / "staging.toml" - _ = staging.write_text(config, encoding="utf-8") + _ = staging.write_text(case.config, encoding="utf-8") output = tmp_path / "names" record = tmp_path / "record" code = main([ @@ -54,7 +111,11 @@ def test_malformed_targets_are_reported_as_input_faults( "--digest-record", str(record), ]) - assert code == EXIT_INPUT_UNUSABLE, f"{reason}: malformed targets returned {code}" - assert "inputs unusable" in capsys.readouterr().err, "missing input diagnostic" + assert code == EXIT_INPUT_UNUSABLE, ( + f"{case.reason}: malformed targets returned {code}" + ) + stderr = capsys.readouterr().err + assert "inputs unusable" in stderr, "missing input diagnostic" + assert case.diagnosis in stderr, f"{case.reason}: wrong input diagnostic" assert not output.exists(), "malformed targets wrote a declaration" assert not record.exists(), "malformed targets wrote a digest record" From c8a1aac01c1f2e2bb3e126a101abf404273afbaf Mon Sep 17 00:00:00 2001 From: leynos Date: Sat, 31 Oct 2026 04:02:19 +0100 Subject: [PATCH 11/16] Mark the typed-records evaluation complete The decision is recorded as ADR-0068, the migration is landed, and the final CodeRabbit review on 7a68b38dd reported no findings across its 24 reviewed files. Set the plan's status to COMPLETE accordingly, and correct the gate-evidence paragraph so it describes the full gate set re-run on the branch head rather than an earlier migration commit. The branch is now ready for its draft pull request; merging is outside this plan's scope. Co-Authored-By: Claude Code --- ...d-records-for-release-policy-boundaries.md | 32 ++++++++++--------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/docs/execplans/issue-964-evaluate-typed-records-for-release-policy-boundaries.md b/docs/execplans/issue-964-evaluate-typed-records-for-release-policy-boundaries.md index dc40dc3db..9bf025f1a 100644 --- a/docs/execplans/issue-964-evaluate-typed-records-for-release-policy-boundaries.md +++ b/docs/execplans/issue-964-evaluate-typed-records-for-release-policy-boundaries.md @@ -1,11 +1,11 @@ # Issue 964: Evaluate typed records for release policy boundaries -Status: IN PROGRESS +Status: COMPLETE -Implementation is complete and verified: the decision is recorded as ADR-0068, -the migration has landed, and the full deterministic gate set is green on the -migration commit `716f9b914`. Review and the draft pull request are -outstanding, so the plan is not yet complete. +The decision is recorded as ADR-0068, the migration has landed, the full +deterministic gate set is green on the branch head, and CodeRabbit reported no +findings on the final review. The branch is ready for its draft pull request; +merging is outside this plan's scope. ## Purpose @@ -251,6 +251,8 @@ records and are unchanged. parametrized, the variance note is corrected, and the sorting guard now uses a mixed-type key set. The full gate set is re-run on the branch head. +- [x] Run the final CodeRabbit review on `7a68b38dd`. It reported no findings + across 24 reviewed files. ## Outcomes and retrospective @@ -317,16 +319,16 @@ before the migration and still pass after it, now asserted through the new decoders; the policy suites construct records directly, so they test the decision rather than the decoding. -Gate evidence, bound to the migration commit `716f9b914`: `make check-fmt` (366 -Python files, 209 Markdown files unchanged), `make typecheck`, `make lint` -(Clippy, Whitaker, Ruff, Pylint, df12 lints, ambrleaks, Interrogate at 100%), -`make doc-coverage` (98.21% against the 80% threshold), `make markdownlint` -(209 files, no issues), `make test` (4804 tests passed, 7 skipped, doctests -clean), and `make test-workflow-contracts` (1665 passed, 3 skipped). The -documentation commit that carries this update is re-gated on the branch head. -Roadmap item 9.1.1 stays open, because its success criterion covers thresholds, -freshness rules, waivers, and retention as well; its policy-boundary sub-item -is checked against ADR-0068. +Gate evidence, first bound to the migration commit `716f9b914`: +`make check-fmt` (366 Python files, 209 Markdown files unchanged), +`make typecheck`, `make lint` (Clippy, Whitaker, Ruff, Pylint, df12 lints, +ambrleaks, Interrogate at 100%), `make doc-coverage` (98.21% against the 80% +threshold), `make markdownlint` (209 files, no issues), `make test` (4804 tests +passed, 7 skipped, doctests clean), and `make test-workflow-contracts` (1665 +passed, 3 skipped). The full set is re-run on the branch head that carries the +review fixes and this update. Roadmap item 9.1.1 stays open, because its +success criterion covers thresholds, freshness rules, waivers, and retention as +well; its policy-boundary sub-item is checked against ADR-0068. ## References From 1077fc0b3400827cc2a9a26f3ea1647c37cde53a Mon Sep 17 00:00:00 2001 From: leynos Date: Sat, 31 Oct 2026 04:45:07 +0100 Subject: [PATCH 12/16] Record the draft pull request in the ExecPlan The plan's status paragraph said the branch was ready for a draft pull request; pull request #1052 now carries it. Point the header and the reference list at that pull request so the living plan matches the published state. Co-Authored-By: Claude Code --- ...-evaluate-typed-records-for-release-policy-boundaries.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/docs/execplans/issue-964-evaluate-typed-records-for-release-policy-boundaries.md b/docs/execplans/issue-964-evaluate-typed-records-for-release-policy-boundaries.md index 9bf025f1a..6f3401ab3 100644 --- a/docs/execplans/issue-964-evaluate-typed-records-for-release-policy-boundaries.md +++ b/docs/execplans/issue-964-evaluate-typed-records-for-release-policy-boundaries.md @@ -4,8 +4,9 @@ Status: COMPLETE The decision is recorded as ADR-0068, the migration has landed, the full deterministic gate set is green on the branch head, and CodeRabbit reported no -findings on the final review. The branch is ready for its draft pull request; -merging is outside this plan's scope. +findings on the final review. Draft pull request +[#1052](https://github.com/leynos/netsuke/pull/1052) carries the branch for +maintainer review; merging is outside this plan's scope. ## Purpose @@ -333,6 +334,7 @@ well; its policy-boundary sub-item is checked against ADR-0068. ## References - [Issue #964](https://github.com/leynos/netsuke/issues/964) +- [Pull request #1052](https://github.com/leynos/netsuke/pull/1052) - [Pull request #903](https://github.com/leynos/netsuke/pull/903) - [CodeRabbit review comment 6448190491](https://github.com/leynos/netsuke/pull/903#discussion_r6448190491) - [Trusted packaging ExecPlan](use-trusted-packaging.md) From d31ba707761c7ada708da8974a410e19e41fc810 Mon Sep 17 00:00:00 2001 From: leynos Date: Sat, 31 Oct 2026 05:42:09 +0100 Subject: [PATCH 13/16] Split the digest-record shape checks into helpers CodeScene's advisory code-health gate flagged `_asset_mapping` as a complex method (cyclomatic complexity 10, score 10.00 -> 9.69). The function mixed three concerns: proving the record is a mapping, proving the `assets` member is a mapping, and proving every asset key is a string. Extract `_require_mapping` and `_string_keyed` so each function holds a single check. The refusal messages, their types, and their order are unchanged: `_require_mapping` takes the description of the malformed part, so the two shape refusals still name the record and the `assets` member respectively. The adapter suite already asserts all four messages, including the non-string-key case, so the split stays covered. Co-Authored-By: Claude Code --- scripts/release_digest_record_files.py | 68 ++++++++++++++++++++++---- 1 file changed, 58 insertions(+), 10 deletions(-) diff --git a/scripts/release_digest_record_files.py b/scripts/release_digest_record_files.py index cd1d540c9..de9511e4e 100644 --- a/scripts/release_digest_record_files.py +++ b/scripts/release_digest_record_files.py @@ -121,19 +121,67 @@ def _asset_mapping(record: object) -> dict[str, object]: >>> _asset_mapping({"assets": {"a": "ab"}}) {'a': 'ab'} """ - if not isinstance(record, dict): - msg = f"the digest record must be a mapping, got {type(record).__name__}" - raise release_digest_record.DigestRecordError(msg) - declared = record.get(ASSETS_MEMBER) - if not isinstance(declared, dict): - msg = ( - f"the digest record must carry an '{ASSETS_MEMBER}' mapping, got " - f"{type(declared).__name__}" - ) - raise release_digest_record.DigestRecordError(msg) + container = _require_mapping(record, "the digest record must be a mapping") + declared = _require_mapping( + container.get(ASSETS_MEMBER), + f"the digest record must carry an '{ASSETS_MEMBER}' mapping", + ) if not declared: msg = "the digest record declares no assets, so there is nothing to verify" raise release_digest_record.DigestRecordError(msg) + return _string_keyed(declared) + + +def _require_mapping(value: object, description: str) -> dict[object, object]: + """Return ``value`` as a mapping, or refuse the record shape. + + The description names which part of the record is malformed, so each + refusal keeps the diagnosis it carried before the shape checks were + split apart. + + Returns + ------- + dict[object, object] + The value, once it is known to be a mapping. + + Raises + ------ + release_digest_record.DigestRecordError + If the value is not a mapping. + + Examples + -------- + >>> _require_mapping({"a": "ab"}, "the record must be a mapping") + {'a': 'ab'} + """ + if not isinstance(value, dict): + msg = f"{description}, got {type(value).__name__}" + raise release_digest_record.DigestRecordError(msg) + return value + + +def _string_keyed(declared: dict[object, object]) -> dict[str, object]: + """Return the asset mapping after refusing a non-string asset name. + + JSON object keys are always strings, so a non-string key can only arrive + as an already decoded value; the check belongs to this adapter because a + mixed-type key set would later break ``sorted``. + + Returns + ------- + dict[str, object] + The same entries, narrowed to string keys. + + Raises + ------ + release_digest_record.DigestRecordError + If any key is not a string. + + Examples + -------- + >>> _string_keyed({"a": "ab"}) + {'a': 'ab'} + """ if not all(isinstance(name, str) for name in declared): msg = "the digest record must map asset names to digests as strings" raise release_digest_record.DigestRecordError(msg) From ee431dcaad8b512b17b5fe7e10b3a8616bb45015 Mon Sep 17 00:00:00 2001 From: leynos Date: Sat, 31 Oct 2026 07:09:18 +0100 Subject: [PATCH 14/16] Record the rebase onto origin/main Replay the branch onto 9bfb3e117, which lands the 4.3.1 Proptest and Ninja determinism work, the Cranelift pin status note, the CLI help metadata and localization repairs, and the generated spelling refresh. Main advanced three times while the branch was under review, so earlier replays onto 08d0a918 and f09b85315 were repeated onto the current head. The third replay conflicted once, in the docs/contents.md ADR index, where main adds ADR-0069 and the branch adds ADR-0068; the resolution keeps both entries in ADR-number order. The branch's own generated-typos refresh became empty because main had already landed the same regeneration, so it was dropped rather than recreated. Neither the target commits nor the resolution touches a release-policy seam, so the only follow-up is a Progress record rather than a code change; the plan states the current base, the dropped commit, and the driver-selection decision. Co-Authored-By: Claude Code --- ...d-records-for-release-policy-boundaries.md | 25 ++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/docs/execplans/issue-964-evaluate-typed-records-for-release-policy-boundaries.md b/docs/execplans/issue-964-evaluate-typed-records-for-release-policy-boundaries.md index 6f3401ab3..9d72463ce 100644 --- a/docs/execplans/issue-964-evaluate-typed-records-for-release-policy-boundaries.md +++ b/docs/execplans/issue-964-evaluate-typed-records-for-release-policy-boundaries.md @@ -2,9 +2,10 @@ Status: COMPLETE -The decision is recorded as ADR-0068, the migration has landed, the full -deterministic gate set is green on the branch head, and CodeRabbit reported no -findings on the final review. Draft pull request +The decision is recorded as ADR-0068, the migration has landed, and the branch +is rebased onto `origin/main` at `9bfb3e117` with the deterministic gate set +re-run on the rebased head. CodeRabbit reported no findings on the final review +before the first rebase. Draft pull request [#1052](https://github.com/leynos/netsuke/pull/1052) carries the branch for maintainer review; merging is outside this plan's scope. @@ -254,6 +255,24 @@ records and are unchanged. head. - [x] Run the final CodeRabbit review on `7a68b38dd`. It reported no findings across 24 reviewed files. +- [x] Rebase onto `origin/main`, first at `08d0a918` ("Add Proptest coverage + for deterministic Ninja emission (4.3.1)"), then at `f09b85315` + ("Record the Cranelift exception status on the current pin"), and + finally at `9bfb3e117` ("Refresh generated typos.toml from the pinned + builder (#1062)"), because main advanced while the branch was under + review. The first two replays were conflict-free; the third conflicted + once, in the `docs/contents.md` ADR index, where main adds ADR-0069 and + the branch adds ADR-0068. The resolution keeps both entries in + ADR-number order. The branch's own "Refresh the generated typos + configuration" commit became empty, because main had already landed the + same regeneration: `git diff 9bfb3e117:typos.toml fb36476f0:typos.toml` + is empty, so the commit was dropped rather than recreated. `range-diff` + reports every other commit unchanged and `git diff --check` is clean. + All three replays used Git's built-in text merge with `zdiff3` because + Weave was selected only by the host's global attributes file, not by a + tracked rule. Beyond the ADR index, the target commits touch CLI help + metadata, localization keys, and generated spelling configuration; none + reaches a release-policy seam, so no further branch change follows. ## Outcomes and retrospective From c1a608460d6190996cbcb4ec8822e16ab0835f23 Mon Sep 17 00:00:00 2001 From: leynos Date: Tue, 3 Nov 2026 03:58:06 +0100 Subject: [PATCH 15/16] Pin the empty-platform diagnosis and record the tightening Review found that the migration tightened one diagnosis: an empty `platform` string used to pass the CLI's string check and reach policy, which refused it as `UnknownPlatformError`. `StagingTarget` now enforces its constructor invariant and refuses the value earlier as `MalformedTargetError(MISSING_PLATFORM)`. Add a `platform-is-empty` case to the malformed-configuration table and amend the claims in ADR-0068, the issue-964 ExecPlan, and the trusted-packaging decision log so they identify the tightening rather than asserting every rejection is unchanged. The configuration still exits with the same code and no identity or inventory check is weakened. Co-Authored-By: Claude Code --- ...d-records-for-release-policy-boundaries.md | 13 ++++++- ...d-records-for-release-policy-boundaries.md | 38 ++++++++++++++++--- docs/execplans/use-trusted-packaging.md | 9 +++-- ...test_release_declaration_invalid_config.py | 8 ++++ 4 files changed, 57 insertions(+), 11 deletions(-) diff --git a/docs/adrs/adr-0068-typed-records-for-release-policy-boundaries.md b/docs/adrs/adr-0068-typed-records-for-release-policy-boundaries.md index 9ef312cf9..c7c035cd7 100644 --- a/docs/adrs/adr-0068-typed-records-for-release-policy-boundaries.md +++ b/docs/adrs/adr-0068-typed-records-for-release-policy-boundaries.md @@ -49,7 +49,11 @@ may legitimately decide over. - Give each external record shape exactly one decoding location, owned by an adapter or CLI that already handles its serialization. - Preserve every existing rejection category and its type, so operators keep - the same diagnosis and the same exit status. + the same diagnosis and the same exit status. One tightening is accepted: an + empty `platform` string is refused by the record as + `MalformedTargetError(MISSING_PLATFORM)` instead of reaching policy as + `UnknownPlatformError`. The configuration is still rejected with the same + exit status, and no release check is weakened. - Remove the duplicated staging-config decode without weakening the check that refuses a malformed `targets` table before discovery iterates it. - Keep the migration bounded, so the change is reviewable and its test @@ -131,7 +135,12 @@ external record shapes: Every existing error type is retained: `MissingTargetsError`, `MalformedTargetError` with `MalformedTargetKind`, `DigestRecordError`, and `RecordedDigestMismatchError`. The migration moves where they are raised, not -what they mean. +what they mean, with one deliberate exception. An empty `platform` string used +to pass the CLI's string check and was rejected later by policy as +`UnknownPlatformError`; the record's constructor now refuses it as +`MalformedTargetError(MISSING_PLATFORM)`. The rejection moves earlier and +changes category, but the configuration still fails the same CLI exit status +and no inventory or identity check is relaxed. ## Consequences diff --git a/docs/execplans/issue-964-evaluate-typed-records-for-release-policy-boundaries.md b/docs/execplans/issue-964-evaluate-typed-records-for-release-policy-boundaries.md index 9d72463ce..e2c4eae55 100644 --- a/docs/execplans/issue-964-evaluate-typed-records-for-release-policy-boundaries.md +++ b/docs/execplans/issue-964-evaluate-typed-records-for-release-policy-boundaries.md @@ -3,11 +3,14 @@ Status: COMPLETE The decision is recorded as ADR-0068, the migration has landed, and the branch -is rebased onto `origin/main` at `9bfb3e117` with the deterministic gate set -re-run on the rebased head. CodeRabbit reported no findings on the final review -before the first rebase. Draft pull request -[#1052](https://github.com/leynos/netsuke/pull/1052) carries the branch for -maintainer review; merging is outside this plan's scope. +is rebased onto `origin/main` at `e5d57c583`, whose tree is identical to the +previous target `9bfb3e117`, with the deterministic gate set re-run on the +rebased head. Review found one edge case where the new record refuses an empty +`platform` string earlier than the old policy did; the tightening is recorded +under *Outcomes and retrospective* and pinned by a malformed-configuration +case. CodeRabbit reported no findings on the final review before the first +rebase. Draft pull request [#1052](https://github.com/leynos/netsuke/pull/1052) +carries the branch for maintainer review; merging is outside this plan's scope. ## Purpose @@ -178,6 +181,10 @@ Adoption is justified only when all four hold: named adapter or CLI, and policy receives a domain value. 2. **Preserved rejection categories.** Every existing refusal keeps its type and its message semantics, and the tests that prove each refusal still pass. + One exception was accepted during delivery: an empty `platform` string is + refused earlier as `MalformedTargetError` rather than later as + `UnknownPlatformError`, with the same CLI exit status. It is recorded under + *Outcomes and retrospective*. 3. **Fewer duplicated checks.** The staging-target double decode disappears, and the digest comparison can no longer be called with an unvalidated mapping through the normal path. @@ -273,6 +280,15 @@ records and are unchanged. tracked rule. Beyond the ADR index, the target commits touch CLI help metadata, localization keys, and generated spelling configuration; none reaches a release-policy seam, so no further branch change follows. +- [x] Replay once more onto `e5d57c583`, whose tree is identical to + `9bfb3e117`. The replay re-parented all 14 commits with no content + change: the new head's tree hash equals the old head's, `range-diff` + reports every commit unchanged, and `git diff --check` is clean. +- [x] Address the post-review qualification about `platform = ""`. The + malformed-configuration table gains a `platform-is-empty` case, and + ADR-0068, this plan, and the trusted-packaging decision log now record + that the record refuses the value earlier than policy did, with the same + exit status and no weakened release check. ## Outcomes and retrospective @@ -296,6 +312,15 @@ checks are unchanged, and commit identity is still an exact string comparison. The staging decode still runs before discovery, so a malformed table fails on the configuration rather than on a later adapter. +One diagnosis was deliberately tightened. An empty `platform` string used to +pass the CLI's string check and reach policy, which rejected it as +`UnknownPlatformError`. The record's constructor now refuses it earlier as +`MalformedTargetError(MISSING_PLATFORM)`. The configuration is still rejected +with the same CLI exit status and no release protection is weakened, but the +exception category, the diagnosis, and the validation timing are not identical +to the former behaviour. The malformed-configuration table pins the new +diagnosis as a `platform-is-empty` case. + Three points were less mechanical than the estimate suggested: - `DigestRecord.from_mapping` receives unvalidated decoded JSON values, so its @@ -337,7 +362,8 @@ The regression evidence is pinned rather than incidental. The nine previously-rejected digest shapes and eight staging shapes were committed before the migration and still pass after it, now asserted through the new decoders; the policy suites construct records directly, so they test the -decision rather than the decoding. +decision rather than the decoding. The empty-platform case is a ninth staging +row, added to pin the tightened diagnosis described above. Gate evidence, first bound to the migration commit `716f9b914`: `make check-fmt` (366 Python files, 209 Markdown files unchanged), diff --git a/docs/execplans/use-trusted-packaging.md b/docs/execplans/use-trusted-packaging.md index ca1084854..0f380dc37 100644 --- a/docs/execplans/use-trusted-packaging.md +++ b/docs/execplans/use-trusted-packaging.md @@ -3975,9 +3975,12 @@ Dependencies: none new. `rust-lang/crates-io-auth-action` at typed-record proposal and adopts it for the seams where an external shape reaches policy: staging targets become `StagingTargets`, and the trusted digest record becomes `DigestRecord`. Domain-shaped name-to-value mappings - remain where no external format defines them. Every existing rejection - category and every identity and inventory check is retained. This resolves - the 2026-10-28 deferral and does not change the trusted-publishing Stage C or + remain where no external format defines them. Every identity and inventory + check is retained, and every rejection category keeps its type except one + deliberate tightening: an empty `platform` string is now refused by the + record as `MalformedTargetError` rather than by policy as + `UnknownPlatformError`, with the same CLI exit status. This resolves the + 2026-10-28 deferral and does not change the trusted-publishing Stage C or Stage D status: Stage C remains partial and Stage D remains open. See [ADR-0068](../adrs/adr-0068-typed-records-for-release-policy-boundaries.md) and diff --git a/scripts/tests/test_release_declaration_invalid_config.py b/scripts/tests/test_release_declaration_invalid_config.py index 5fa00c45c..c888c3859 100644 --- a/scripts/tests/test_release_declaration_invalid_config.py +++ b/scripts/tests/test_release_declaration_invalid_config.py @@ -82,6 +82,14 @@ class Case(typ.NamedTuple): ), id="platform-is-an-integer", ), + pytest.param( + Case( + '[targets]\nlinux = { platform = "" }\n', + "target without a platform", + "an empty platform names no platform", + ), + id="platform-is-empty", + ), ], ) def test_malformed_targets_are_reported_as_input_faults( From 52f0a4337e6f62a8824f6d7210dff320e106f1a8 Mon Sep 17 00:00:00 2001 From: leynos Date: Tue, 3 Nov 2026 06:05:02 +0100 Subject: [PATCH 16/16] Drop the adapter's duplicate empty-assets refusal DigestRecord.__post_init__ refuses an empty record with the same message on every construction path, so the adapter's copy duplicated an invariant it does not own. Removing it keeps the malformed-JSON behaviour: an empty ``assets`` object still reaches from_mapping and raises the same error. Also correct the _string_keyed note. It claimed the adapter check was needed because a mixed-type key set would break ``sorted``, but _validated_entry already refuses a non-string key, with the same message, before sorting happens. The docstring now records the real reason the check exists. Both points were raised by the CodeRabbit review of c1a608460. Co-Authored-By: Claude Code --- ...ed-records-for-release-policy-boundaries.md | 7 +++++++ scripts/release_digest_record_files.py | 18 +++++------------- 2 files changed, 12 insertions(+), 13 deletions(-) diff --git a/docs/execplans/issue-964-evaluate-typed-records-for-release-policy-boundaries.md b/docs/execplans/issue-964-evaluate-typed-records-for-release-policy-boundaries.md index e2c4eae55..b2e5e4d01 100644 --- a/docs/execplans/issue-964-evaluate-typed-records-for-release-policy-boundaries.md +++ b/docs/execplans/issue-964-evaluate-typed-records-for-release-policy-boundaries.md @@ -289,6 +289,13 @@ records and are unchanged. ADR-0068, this plan, and the trusted-packaging decision log now record that the record refuses the value earlier than policy did, with the same exit status and no weakened release check. +- [x] Remove the adapter's duplicate empty-assets refusal, raised by the + CodeRabbit review of `c1a608460`. `DigestRecord.__post_init__` already + refuses an empty record with the same message on every construction + path, so `_asset_mapping` keeps only the member name and the mapping + shape checks. The `_string_keyed` note that called its check necessary + for `sorted` was wrong, because `_validated_entry` refuses a non-string + key in the same message before sorting; the docstring now says so. ## Outcomes and retrospective diff --git a/scripts/release_digest_record_files.py b/scripts/release_digest_record_files.py index de9511e4e..159a5e237 100644 --- a/scripts/release_digest_record_files.py +++ b/scripts/release_digest_record_files.py @@ -102,19 +102,14 @@ def validate_digest_record( def _asset_mapping(record: object) -> dict[str, object]: - """Return the non-empty asset mapping from a decoded record. + """Return the asset mapping from a decoded record. Returns ------- dict[str, object] The unvalidated digest values from the record's ``assets`` member, - keyed by asset name. - - Raises - ------ - release_digest_record.DigestRecordError - If the record is not a non-empty mapping of string-keyed asset - entries. + keyed by asset name. Each shape check raises the error of the helper + that performs it. Examples -------- @@ -126,9 +121,6 @@ def _asset_mapping(record: object) -> dict[str, object]: container.get(ASSETS_MEMBER), f"the digest record must carry an '{ASSETS_MEMBER}' mapping", ) - if not declared: - msg = "the digest record declares no assets, so there is nothing to verify" - raise release_digest_record.DigestRecordError(msg) return _string_keyed(declared) @@ -164,8 +156,8 @@ def _string_keyed(declared: dict[object, object]) -> dict[str, object]: """Return the asset mapping after refusing a non-string asset name. JSON object keys are always strings, so a non-string key can only arrive - as an already decoded value; the check belongs to this adapter because a - mixed-type key set would later break ``sorted``. + as an already decoded value. ``DigestRecord`` refuses such a key with the + same message. Returns -------