Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
175 changes: 175 additions & 0 deletions docs/adrs/adr-0068-typed-records-for-release-policy-boundaries.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,175 @@
# 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. 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
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, 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

- 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)
3 changes: 3 additions & 0 deletions docs/contents.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
33 changes: 26 additions & 7 deletions docs/developers-guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -2678,6 +2681,22 @@ 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, 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
release policy, response adapters do not choose whether a run publishes, and
policy modules do not grow convenience access to these boundaries. Composition
Expand Down
Loading
Loading