Skip to content

Evaluate typed records for release policy boundaries (#964) - #1052

Open
leynos wants to merge 16 commits into
mainfrom
issue-964-evaluate-typed-records-for-release-policy-boundaries
Open

leynos wants to merge 16 commits into
mainfrom
issue-964-evaluate-typed-records-for-release-policy-boundaries

Conversation

@leynos

@leynos leynos commented Oct 31, 2026 •

Copy link
Copy Markdown
Owner

Summary

Evaluate whether the release-publication policy should decide from typed
records rather than from validated in-memory mappings, record the decision, and
migrate the external-shape seams that the decision covers.

Closes #964.

Decision

The evaluation adopts frozen slotted dataclasses decoded at the adapter
boundary
, with domain-shaped mappings retained where the policy genuinely
needs a mapping. It is recorded as
ADR-0068,
which compares three options: retained mappings, adapter-decoded frozen
records, and TypedDict (rejected, because it validates keys and value types
but not values, and it is erased at runtime).

Two records now cross the boundary:

  • StagingTargets (with StagingTarget entries) in release_declaration;
  • 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 moves from POLICY_MODULES to
ADAPTER_MODULES without a code change, because it already decoded the
record's field names into a validated identity.

Both records enforce their invariants in __post_init__ as well as in their
factories, so a directly constructed record cannot reach a policy comparison
unvalidated.

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.

One diagnosis is deliberately tightened. An empty platform string used to
pass the CLI's string check and reach policy, which refused it as
UnknownPlatformError. StagingTarget.__post_init__ 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 differ from the
former behaviour. ADR-0068, the ExecPlan, and the trusted-packaging decision log
record the exception.

The eight malformed staging shapes and nine decoded digest-record shapes were
pinned as characterization tests before the migration and still pass after
it, now asserted through the new decoders. A ninth staging case,
platform-is-empty, pins the tightened diagnosis. Policy suites construct
records directly, so they test the decision rather than the decoding.

Explicitly out of scope

Unchanged, as required by the issue: .github/scripts/_release_admission/,
tests/workflow_contracts/release_publish_path_*.py, the fresh
freshness/evidence inconsistency, and the trusted-publishing Stage C and Stage D
work (Stage C remains partial, Stage D remains open).

Roadmap item 9.1.1 stays open, because its success criterion also covers
thresholds, freshness rules, waivers, and retention; its policy-boundary
sub-item is checked against ADR-0068.

Review follow-up

Three CodeRabbit review passes and the CodeScene code-health check ran
against the branch; every finding is fixed.

  • The record invariants were only enforced in the factories. Both records now
    validate in __post_init__ as well, and the tests exercise direct
    construction alongside the factory path.
  • A directly constructed DigestRecord could repeat an asset name, hiding one
    digest from every comparison. Repeated names are now refused.
  • Four refusal tests differing only in input were folded into one parameterized
    table, per the repository's testing guidance.
  • The ExecPlan's variance note claimed Mapping is invariant in its key type;
    the real reason for Mapping[str, object] is that it admits unvalidated
    decoded JSON.
  • The sort-ordering guard used a one-key fixture, where sorted never compares
    anything. It now pairs a string key with the integer key.
  • A directly constructed StagingTarget accepted a non-string or empty
    platform, which reached sorted as a mixed-type key set and raised
    TypeError instead of a configuration diagnosis. It now refuses the value
    with the existing MalformedTargetKind.MISSING_PLATFORM category; an unknown
    platform name still raises UnknownPlatformError in policy.
  • Review noted that this also changes the diagnosis for platform = "" in a
    configuration file: it is now refused earlier by the record rather than
    later by policy. The malformed-configuration table pins that diagnosis, and
    the documents that claimed every rejection was unchanged now record the
    tightening explicitly.
  • The malformed-configuration suite asserted only the shared exit code, so a
    missing-targets refusal could satisfy an entry-level case. Each case now
    asserts its own diagnosis on stderr.
  • The developers' guide said the record constructor normalizes digests. It now
    states the split: from_mapping lowercases, while __post_init__ refuses a
    digest that is not already lowercase.
  • CodeScene's advisory code-health gate flagged _asset_mapping in
    release_digest_record_files.py at cyclomatic complexity 10 (10.00 → 9.69).
    The shape checks are now split into _require_mapping and _string_keyed,
    with the same messages and rejection order; the file scores 10.00 again.
  • CodeRabbit's review of c1a608460 found the adapter still refused an empty
    assets mapping itself. DigestRecord.__post_init__ already refuses it with
    the same message on every construction path, so the adapter copy duplicated
    an invariant it does not own; it is removed. The same review showed the
    _string_keyed note was wrong: it said the check existed because a mixed-type
    key set would break sorted, but _validated_entry refuses a non-string key
    in the same message before sorting. The docstring now states the real
    situation.

The branch originally carried a separate commit refreshing the generated
typos.toml. Rebasing onto origin/main made it empty: main landed the same
regeneration in 5f0a4d123, and the two files are byte-identical. The commit
was dropped for that reason, so the branch no longer touches typos.toml.

Testing

Deterministic gates, run sequentially on the branch tree carried by the current
head 52f0a4337 (rebased onto origin/main at e5d57c583):

  • make check-fmt (366 Python files, 213 Markdown files unchanged)
  • make typecheck
  • make lint (Clippy, Whitaker, Ruff, Pylint, df12 house lints, ambrleaks,
    Interrogate at 100%)
  • make markdownlint (213 files, no issues; spelling gate leaves the tree
    clean)
  • make nixie
  • make test (4943 passed, 7 skipped; doctests 163 passed, 33 ignored)
  • make test-workflow-contracts (1665 passed, 3 skipped)
  • make test-release-publication (489 passed)
  • make doc-coverage (98.23% aggregate, 6369 of 6484 items, against the 80%
    threshold)

Four replays onto successive origin/main heads each reapplied every branch
commit without a code change. The first three included target commits touching
CLI help metadata, localization keys, and the generated spelling configuration,
none of which is a release-policy seam. The third replay met one conflict, in
docs/contents.md, where main had added ADR-0069 at the anchor this branch uses
for ADR-0068; both entries are kept in serial order. The fourth moved the series
from 9bfb3e117 to e5d57c583, whose tree is identical to 9bfb3e117, so it
re-parented all 14 commits with no content change: the new head's tree hash
equals the old head's.

make check-fmt, make typecheck, make lint, and make test were re-run on
ee431dcaa after that replay and passed on the same content: 4943 tests passed
with 7 skipped, and doctests 163 passed with 33 ignored. The review follow-up in
52f0a4337 was then gated with the full nine-target set above, which is the
evidence recorded here.

The ExecPlan at
docs/execplans/issue-964-evaluate-typed-records-for-release-policy-boundaries.md
records the inventory, migration cost estimate, and progress.

References

Summary by Sourcery

Move external release-record decoding to adapter boundaries and make release policy operate on validated frozen records.

Bug Fixes:

  • Preserve release-policy validation and rejection behavior while preventing malformed staging and digest records from reaching policy comparisons.

Enhancements:

  • Adopt frozen, slotted typed records for external staging-target and trusted-digest boundaries, with adapters owning decoding and domain mappings retained where appropriate.
  • Decode staging targets once before discovery and reclassify release identity decoding as an adapter responsibility.
  • Enforce record invariants for both factory-created and directly constructed staging and digest records, including duplicate digest asset names.
  • Document the boundary decision and conventions in ADR-0068 and the release architecture documentation.

Documentation:

  • Add ADR-0068 and update the developers' guide, design documentation, roadmap, contents index, and trusted-packaging decision log.

Tests:

  • Add characterization and parameterized coverage for malformed staging configurations, digest records, direct record construction, and preserved diagnostic categories.

Chores:

  • Keep the generated spelling configuration in step with main, which landed the
    same regeneration and made the branch's copy redundant.

🤖 Generated with Claude Code

@coderabbitai

coderabbitai Bot commented Oct 31, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • Resume reviews
  • Trigger review

Summary

  • Adopt frozen, slotted StagingTargets and DigestRecord records at external release-policy boundaries. Decode them in the CLI and file adapters.
  • Keep domain-shaped mappings where policy requires them. Reclassify release_identity_record as an adapter without changing its implementation.
  • Preserve release-identity, inventory, coverage and digest checks. Record the decision in ADR-0068.
  • Document the evaluation and migration in the new issue #964 ExecPlan.
  • Record the empty-platform change: reject it earlier as MalformedTargetError(MISSING_PLATFORM) instead of UnknownPlatformError, with the same CLI exit status.
  • Keep trusted-publishing Stage C and Stage D work out of scope. Stage C remains partial, and Stage D remains open.

The author reports that all nine listed gates passed on head 52f0a4337, including make test (4,943 passed, 7 skipped; 163 doctests passed, 33 ignored), workflow-contract tests (1,665 passed, 3 skipped), release-publication tests (489 passed), and 98.23% documentation coverage. Roadmap item 9.1.1 remains open.

Walkthrough

Release policy boundaries now use frozen, slotted StagingTargets and DigestRecord records for external staging and digest data. Adapters decode those shapes before policy use. An empty platform now raises MalformedTargetError(MISSING_PLATFORM) earlier, with the same CLI exit status.

Changes

Release policy boundary migration

Layer / File(s) Summary
Decode staging configuration into records
scripts/release_declaration.py, scripts/release_declaration_cli.py, scripts/tests/test_release_declaration*.py, scripts/tests/test_release_declaration_cli.py, scripts/tests/test_release_declaration_invalid_config.py
The CLI decodes staging configuration into StagingTarget and StagingTargets. Declaration logic accepts those records. Tests cover construction, invalid platforms, and configuration diagnostics.
Validate and adapt trusted digest records
scripts/release_digest_record.py, scripts/release_digest_record_files.py, scripts/tests/release_staging_support.py, scripts/tests/test_release_digest_record*.py
DigestRecord validates and normalises digest entries. The file adapter owns JSON parsing, the assets member, and serialisation. Tests cover record invariants and file-format errors.
Use digest records across release commands
scripts/finalize_github_release_cli.py, scripts/verify_release_inventory_cli.py, scripts/tests/test_verify_release_inventory_*, scripts/release_identity_record.py, tests/workflow_contracts/release_publication_modules.py
Release finalisation and inventory verification use DigestRecord at their policy boundary. Test callers construct records, and the workflow contract classifies release_identity_record as an adapter.
Document the boundary decision
docs/adrs/*, docs/execplans/*, docs/developers-guide.md, docs/contents.md, docs/netsuke-design.md, docs/roadmap.md
The ADR and related documentation describe typed records at external-shape boundaries, retained domain-shaped mappings, and the empty-platform error-category change.

Sequence Diagram(s)

sequenceDiagram
  participant ReleaseDeclarationCLI
  participant StagingTargets
  participant DeclarationLogic
  ReleaseDeclarationCLI->>StagingTargets: decode and validate staging configuration
  StagingTargets-->>ReleaseDeclarationCLI: typed staging targets
  ReleaseDeclarationCLI->>DeclarationLogic: derive declared asset names
Loading

Possibly related issues

Possibly related PRs

  • leynos/netsuke#903: It established mapping-based release-publication policy boundaries that this change migrates to typed records.
  • leynos/netsuke#1077: It consumes the release-publication foundation changed here and identifies typed records as a follow-up.

Suggested labels: Roadmap, Issue

Suggested reviewers: wafflecat-df12

Change: Refactor

Merge Risk: Minimal · up to 52f0a

The change is mergeable with normal checks; the remaining documentation comment is editorial rather than a verified defect.

Pre-merge checks: Passed 14 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Testing (Property / Proof) Warning The prior property-test failure remains unresolved. The PR introduces DigestRecord invariants over arbitrary entry counts and orderings: from_mapping validates every entry, normalizes digest case,… Add a Hypothesis test for non-empty mappings with generated asset names, valid SHA-256 digests, arbitrary sizes, digest case variants, and insertion orders. Assert sorted unique entries, lowercase digests, preserved name sets, and `as_mappi…
Passed Passed checks: 14 passed
Check name Status Explanation
Title check Passed The title "Evaluate typed records for release policy boundaries (#964)" matches the main change: the PR adopts frozen, slotted records at release-policy boundaries, records the decision in ADR-0068, a…
Description check Passed The description states the purpose (evaluate typed records versus validated mappings), the adopted decision (frozen slotted dataclasses decoded at the adapter boundary), the migrated seams, the one do…
Linked Issues check Passed Issue #964 is the only active directly linked issue. It has no contextual or parent links. Its three follow-up items are covered.

[#964] Evaluation: ADR-0068 compares three options: retained mappings…
Out of Scope Changes check Passed No unrelated change was found. The changed files fall into three groups.

Release-policy seams that the migration covers: scripts/release_declaration.py, scripts/release_declaration_cli.py, `scrip…
Docstring Coverage Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Testing (Overall) Passed The changed behaviour has substantive tests. scripts/tests/test_release_declaration.py:49-167 covers complete declarations, sorting, package under/over-counts, duplicate names, unknown platforms, bo…
User-Facing Documentation Passed PASS — The pull request changes internal release-publication policy scripts and their adapter contracts. It does not change Netsuke’s end-user functionality or the reusable workflow interface; no `.gi…
Developer Documentation Passed PASS — The pull request documents the new typed-record boundary in docs/developers-guide.md:2651-2698, including StagingTargets, DigestRecord, adapter ownership, constructor invariants, normaliz…
Module-Level Documentation Passed Pass the module-level documentation check. Every changed Python module has a module docstring. The docstrings state each module's purpose and, where needed, its relationship to adapters, CLIs, policy …
Testing (Unit And Behavioural) Passed Accept the testing coverage. Unit tests exercise StagingTarget and StagingTargets rejection paths, DigestRecord construction invariants including duplicate names and malformed entries, exact dec…
Testing (Compile-Time / Ui) Passed Pass this check. The pull request changes only Python and Markdown files, so the Rust/TypeScript compile-time rule and trybuild requirement do not apply. The changed structured outputs have focused be…
Unit Architecture Passed Preserve the separation. The pull request keeps release_declaration and release_digest_record focused on pure policy and record validation (scripts/release_declaration.py:62-190; `scripts/releas…
Domain Architecture Passed Pass the Domain Architecture check. Keep the domain modules infrastructure-free: release_declaration.py contains frozen domain records and package-count policy only (lines 19-36, 62-93, 172-219), an…
Observability Passed Pass the observability check. The change adds no new service, queue, retry, throughput, latency, or resource path, so it does not require new metrics, tracing, or alerts. The changed failure boundary …
Full details: Title check

Explanation

The title "Evaluate typed records for release policy boundaries (#964)" matches the main change: the PR adopts frozen, slotted records at release-policy boundaries, records the decision in ADR-0068, and migrates the staging-target and digest-record seams. It cites issue #964, as the title requirements require for issue-fixing changes. The title does not name the decision outcome, but it describes the real subject.

Full details: Description check

Explanation

The description states the purpose (evaluate typed records versus validated mappings), the adopted decision (frozen slotted dataclasses decoded at the adapter boundary), the migrated seams, the one documented behaviour change for an empty platform, the out-of-scope items, and the testing evidence. It is directly related to the changeset.

Full details: Linked Issues check

Explanation

Issue #964 is the only active directly linked issue. It has no contextual or parent links. Its three follow-up items are covered.

[#964] Evaluation: ADR-0068 compares three options: retained mappings, adapter-decoded frozen records, and TypedDict. It rejects TypedDict because it does not validate values and is erased at runtime. The ExecPlan records the inventory and migration cost estimate.

[#964] Decision record: ADR-0068 is linked from the contents index, the developers' guide, the design document, the roadmap, and the trusted-packaging decision log.

[#964] Migration: release_declaration_cli.staging_targets decodes the staging configuration into StagingTargets before archive discovery in _compute_declaration. release_digest_record_files.read_digest_record decodes the trusted record into DigestRecord. release_identity_record moves from POLICY_MODULES to ADAPTER_MODULES in tests/workflow_contracts/release_publication_modules.py, with no code change.

[#964] Behaviour preservation: the rejection categories MissingTargetsError, MalformedTargetError with MalformedTargetKind, DigestRecordError, and RecordedDigestMismatchError remain. The exact-coverage and missing-download checks are unchanged. Commit identity remains an exact string comparison. The one behaviour change is an empty platform. It is now refused earlier as MalformedTargetError(MISSING_PLATFORM) rather than as UnknownPlatformError. The CLI exit status is unchanged. The change is recorded in ADR-0068, the ExecPlan, and the decision log, and it is pinned by the platform-is-empty case.

[#964] Changes since the previous reviewed head c1a608460: StagingTarget and DigestRecord now validate in __post_init__. DigestRecord refuses repeated asset names. The adapter's duplicate empty-assets refusal is removed, and DigestRecord refuses that case on every construction path. These changes are described in the pull request and the ExecPlan. This assessment did not re-read those source hunks, so it relies on the described behaviour for them. The previous PASS conclusion still holds for the migration itself.

[#964] Trusted-publishing exclusion: Stage C remains partial and Stage D remains open. The issue does not require their completion.

The gate results (make check-fmt, make typecheck, make lint, make test, make test-workflow-contracts, make test-release-publication, and related targets) are reported by the pull request. This assessment did not re-run them.

Full details: Out of Scope Changes check

Explanation

No unrelated change was found. The changed files fall into three groups.

Release-policy seams that the migration covers: scripts/release_declaration.py, scripts/release_declaration_cli.py, scripts/release_digest_record.py, scripts/release_digest_record_files.py, scripts/verify_release_inventory_cli.py, scripts/finalize_github_release_cli.py (the finalizer consumes DigestRecord), and the docstring-only change in scripts/release_identity_record.py.

Tests that construct the new record types or assert the migrated diagnostics: the scripts/tests/ files listed in the pull request. The workflow contract change in tests/workflow_contracts/release_publication_modules.py is the reclassification that the issue requires.

Documentation that records the decision: ADR-0068, the ExecPlan, docs/contents.md, the developers' guide, the design document, the roadmap, and the trusted-packaging decision log.

Scope checks:

  • A diff of tests/workflow_contracts between the merge base and the reviewed head changes only tests/workflow_contracts/release_publication_modules.py. No release_publish_path_* file changed.
  • A diff of .github/scripts/_release_admission between the merge base and the reviewed head is empty.
  • The pull request reports that typos.toml no longer changes, because the rebase made that commit empty.

The tightening of the empty-platform diagnosis is a documented behaviour change tied to the staging-record seam. It is not an unrelated change.

Full details: Docstring Coverage

Explanation

Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 20 functions across 16 files. (1 skipped: 1 unsupported.)

Full details: Testing (Overall)

Explanation

The changed behaviour has substantive tests. scripts/tests/test_release_declaration.py:49-167 covers complete declarations, sorting, package under/over-counts, duplicate names, unknown platforms, both StagingTargets construction paths, and invalid platform values. scripts/tests/test_release_declaration_invalid_config.py:20-129 covers nine malformed configuration shapes, including the tightened empty-platform diagnosis, exit status, diagnostics, and no output files. scripts/tests/test_release_digest_record.py:26-177 tests factory normalisation, direct-construction invariants, duplicate names, malformed values, exact declaration coverage, missing or differing downloads, mismatch reporting, and exception hierarchy. scripts/tests/test_release_digest_record_files.py:22-109 tests adapter round trips, deterministic output, unreadable paths, JSON shape errors, and malformed entries. The inventory tests independently hash staged files and exercise missing assets, substituted packages, replaced archive/sidecar pairs, and incomplete trusted records (scripts/tests/test_verify_release_inventory_cli.py:30-163). The declaration and finalizer CLI tests exercise the migrated record through producer, verifier, and finalizer boundaries (scripts/tests/test_release_declaration_cli.py:99-129, 326-391; scripts/tests/test_finalize_github_release_cli.py:86-163). These assertions would fail for the plausible regressions introduced by the migration, and they do not rely on mocks as the behavioural oracle.

Full details: User-Facing Documentation

Explanation

PASS — The pull request changes internal release-publication policy scripts and their adapter contracts. It does not change Netsuke’s end-user functionality or the reusable workflow interface; no .github workflow files or user-facing command documentation changed. The exact boundary and diagnostic changes are documented for maintainers in docs/developers-guide.md (lines 2684-2698), while docs/users-guide.md continues to document the public release-workflow input at lines 79-92. The user’s guide therefore remains accurate and no new user-facing documentation is required.

Full details: Developer Documentation

Explanation

PASS — The pull request documents the new typed-record boundary in docs/developers-guide.md:2651-2698, including StagingTargets, DigestRecord, adapter ownership, constructor invariants, normalization, and retained mappings. ADR-0068 records the accepted decision, alternatives, scope, consequences, and references (docs/adrs/adr-0068-typed-records-for-release-policy-boundaries.md:1-175). The design document, trusted-packaging decision log, and docs/contents.md are updated. The new ExecPlan is COMPLETE and records progress and outcomes, while roadmap item 9.1.1 correctly remains open with its completed policy-boundary sub-item checked. No additional language documentation or tooling requirement is introduced.

Full details: Module-Level Documentation

Explanation

Pass the module-level documentation check. Every changed Python module has a module docstring. The docstrings state each module's purpose and, where needed, its relationship to adapters, CLIs, policy modules, and test suites; for example, scripts/release_declaration.py:1-17, scripts/release_digest_record_files.py:1-10, and scripts/tests/test_release_declaration_cli.py:1-9. AST inspection confirmed module docstrings in all 17 changed Python files, and each file also had a docstring in the base revision.

Full details: Testing (Unit And Behavioural)

Explanation

Accept the testing coverage. Unit tests exercise StagingTarget and StagingTargets rejection paths, DigestRecord construction invariants including duplicate names and malformed entries, exact declaration coverage, missing downloads, and digest mismatches (scripts/tests/test_release_declaration.py:102-153; scripts/tests/test_release_digest_record.py:26-176). Adapter tests cover JSON shape errors, normalisation, deterministic writing, unreadable paths, and round-tripping (scripts/tests/test_release_digest_record_files.py:22-109). Behavioural tests invoke the declaration CLI with real temporary configuration and files, verify inventory through the verifier, and exercise the finalizer CLI with persisted records (scripts/tests/test_release_declaration_invalid_config.py:20-129; scripts/tests/test_release_declaration_cli.py:99-392; scripts/tests/test_verify_release_inventory_command.py:33-235; scripts/tests/test_finalize_github_release_cli.py:86-349). The coverage uses public workflow boundaries rather than only private helpers or mocks, and no explicit testing failure condition is introduced.

Full details: Testing (Compile-Time / Ui)

Explanation

Pass this check. The pull request changes only Python and Markdown files, so the Rust/TypeScript compile-time rule and trybuild requirement do not apply. The changed structured outputs have focused behavioural tests: malformed staging cases assert exit codes, stable diagnostic fragments, and no output files in scripts/tests/test_release_declaration_invalid_config.py:20-129; digest JSON tests assert exact sorted, newline-terminated serialization and round-tripping in scripts/tests/test_release_digest_record_files.py:22-44. These semantic assertions avoid brittle snapshots for diagnostics that can contain paths or platform errors.

Full details: Unit Architecture

Explanation

Preserve the separation. The pull request keeps release_declaration and release_digest_record focused on pure policy and record validation (scripts/release_declaration.py:62-190; scripts/release_digest_record.py:39-138). It moves TOML and JSON shape decoding to explicit adapters (scripts/release_declaration_cli.py:58-115; scripts/release_digest_record_files.py:29-180). File reads expose OSError and parsing errors, and the CLI boundaries handle them (scripts/release_digest_record_files.py:58-66; scripts/verify_release_inventory_cli.py:311-339; scripts/finalize_github_release_cli.py:182-185). Writes remain in clearly named command paths such as _write_outputs, write_digest_record, and _write_verified_record; the query and comparison paths do not write, call the network, use clocks, or access ambient mutable state. The typed records give policy functions explicit inputs without adding hidden dependencies or unrelated responsibilities. No explicit Unit Architecture failure condition is introduced.

Full details: Domain Architecture

Explanation

Pass the Domain Architecture check. Keep the domain modules infrastructure-free: release_declaration.py contains frozen domain records and package-count policy only (lines 19-36, 62-93, 172-219), and release_digest_record.py contains the validated DigestRecord and comparison policy without JSON or file access (lines 16-26, 39-138, 246-316). Keep external-shape work in adapters: release_declaration_cli.py decodes TOML field names before discovery (lines 51-119, 369-385), while release_digest_record_files.py owns JSON, assets, paths, and serialization (lines 1-6, 12-22, 29-66, 183-211). Preserve the explicit boundary classification: release_identity_record.py is in ADAPTER_MODULES, and the policy modules remain separate in POLICY_MODULES (tests/workflow_contracts/release_publication_modules.py:66-95). The changed domain modules have no forbidden filesystem, serialization, transport, or provider imports or calls. No explicit architecture failure condition is introduced.

Full details: Observability

Explanation

Pass the observability check. The change adds no new service, queue, retry, throughput, latency, or resource path, so it does not require new metrics, tracing, or alerts. The changed failure boundary remains diagnosable: release declaration reports the stable configuration error and exit status on stderr (scripts/release_declaration_cli.py:215-223), inventory verification distinguishes unreadable and rejected digest records (scripts/verify_release_inventory_cli.py:318-338), and the tests assert the tightened empty-platform diagnosis and exit code (scripts/tests/test_release_declaration_invalid_config.py).

Full details: Testing (Property / Proof)

Explanation

The prior property-test failure remains unresolved. The PR introduces DigestRecord invariants over arbitrary entry counts and orderings: from_mapping validates every entry, normalizes digest case, and sorts entries, while __post_init__ enforces non-empty, lowercase, and unique entries (scripts/release_digest_record.py:39-108). The focused tests use only fixed examples and parameterized cases (scripts/tests/test_release_digest_record.py:26-97; scripts/tests/test_release_digest_record_files.py:73-109). No Hypothesis test targets DigestRecord. The current table does not cover many-entry mappings, permutations, arbitrary valid names, or the normalization invariant over a range of inputs.

Resolution

Add a Hypothesis test for non-empty mappings with generated asset names, valid SHA-256 digests, arbitrary sizes, digest case variants, and insertion orders. Assert sorted unique entries, lowercase digests, preserved name sets, and as_mapping() equality with the normalized mapping. Add generated direct-construction coverage for valid canonical entries and duplicate-name rejection, because mappings cannot represent duplicate keys.

Staging shapes resolve to types,
Digest entries sort in rows.
Adapters read the outer forms,
Policy checks what each record shows.
Empty platforms meet their error,
While the same exit status flows.

CodeRabbit Verify

  • Run CodeRabbit Verify to test the changes in this pull request.

@sourcery-ai

sourcery-ai Bot commented Oct 31, 2026

Copy link
Copy Markdown
Contributor

Reviewer's Guide

This PR records ADR-0068 and migrates the staging-target and trusted-digest external seams from validated mappings to frozen slotted records decoded by adapters, preserving policy checks and error diagnoses while retaining domain mappings where appropriate; it also reclassifies the identity decoder, updates documentation and tests, and refreshes generated spelling configuration.

Sequence diagram for staging configuration decoding before discovery

sequenceDiagram
    participant CLI as release_declaration_cli
    participant Decoder as staging_targets
    participant Policy as release_declaration
    participant Discovery as hoist_binstall_discovery
    CLI->>Decoder: staging_targets(config)
    Decoder-->>CLI: StagingTargets
    CLI->>Discovery: expected_archive_names(staging_config, manifest, version)
    Discovery-->>CLI: archive names
    CLI->>Policy: declared_asset_names(archive_names, targets, staged_package_names)
    Policy->>Policy: platform_target_counts(targets)
    Policy-->>CLI: declared asset names
Loading

Sequence diagram for trusted digest record verification

sequenceDiagram
    participant Files as release_digest_record_files
    participant Record as DigestRecord
    participant Policy as release_digest_record
    participant Verify as verify_release_inventory_cli
    Files->>Files: read_digest_record(source)
    Files->>Record: from_mapping(assets)
    Record-->>Files: validated DigestRecord
    Files-->>Verify: DigestRecord
    Verify->>Policy: require_record_matches_declaration(record, expected)
    Policy->>Record: names()
    Verify->>Policy: require_downloads_match_record(computed, trusted)
    Policy->>Record: as_mapping()
    Policy-->>Verify: acceptance or RecordedDigestMismatchError
Loading

File-Level Changes

Change Details Files
Adopt and document frozen, slotted typed records at external release-policy boundaries while retaining mappings for domain-shaped policy values.
  • Record the decision, trade-offs, scope, and consequences in ADR-0068.
  • Update architecture, developer, roadmap, and trusted-packaging documentation.
  • Add the completed evaluation and migration evidence to the ExecPlan.
docs/adrs/adr-0068-typed-records-for-release-policy-boundaries.md
docs/contents.md
docs/developers-guide.md
docs/execplans/issue-964-evaluate-typed-records-for-release-policy-boundaries.md
docs/execplans/use-trusted-packaging.md
docs/netsuke-design.md
docs/roadmap.md
Move staging-target decoding to the CLI and make declaration policy consume validated staging records.
  • Introduce frozen slotted StagingTarget and StagingTargets records with constructor invariants.
  • Decode targets and platform fields once before discovery in the CLI.
  • Change declaration policy signatures to consume records and preserve malformed-target diagnoses.
scripts/release_declaration.py
scripts/release_declaration_cli.py
scripts/tests/test_release_declaration.py
scripts/tests/test_release_declaration_cli.py
scripts/tests/test_release_declaration_invalid_config.py
Make trusted digest comparisons consume a validated DigestRecord while keeping JSON shape ownership in the file adapter.
  • Add immutable digest entries with normalization in from_mapping and invariant checks in post_init.
  • Reject empty, malformed, duplicate, mixed-case direct-construction, and non-string-key records consistently.
  • Move assets decoding to release_digest_record_files and update verification/finalization callers and fixtures.
scripts/release_digest_record.py
scripts/release_digest_record_files.py
scripts/verify_release_inventory_cli.py
scripts/finalize_github_release_cli.py
scripts/tests/release_staging_support.py
scripts/tests/test_release_digest_record.py
scripts/tests/test_release_digest_record_files.py
scripts/tests/test_verify_release_inventory_cli.py
scripts/tests/test_verify_release_inventory_command.py
Reclassify the identity-record decoder as an adapter and strengthen the characterization coverage for preserved refusal behavior.
  • Move release_identity_record from POLICY_MODULES to ADAPTER_MODULES without production logic changes.
  • Pin malformed staging and digest shapes at decoder seams.
  • Assert category-specific diagnostics and exercise both factory and direct record construction paths.
scripts/release_identity_record.py
tests/workflow_contracts/release_publication_modules.py
scripts/tests/test_release_identity_record.py
Refresh the generated spelling configuration required by repository gates.
  • Regenerate typos.toml from the shared spelling configuration.
  • Commit the generated ignore-pattern changes rather than hand-editing them.
typos.toml

Possibly linked issues


Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

codescene-access[bot]

This comment was marked as outdated.

leynos pushed a commit that referenced this pull request Oct 31, 2026
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 <noreply@anthropic.com>
codescene-access[bot]

This comment was marked as outdated.

codescene-access[bot]

This comment was marked as outdated.

leynos pushed a commit that referenced this pull request Oct 31, 2026
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 <noreply@anthropic.com>
@leynos
leynos force-pushed the issue-964-evaluate-typed-records-for-release-policy-boundaries branch from 61fd0de to 171809c Compare October 31, 2026 07:34
codescene-access[bot]

This comment was marked as outdated.

@leynos

leynos commented Nov 1, 2026

Copy link
Copy Markdown
Owner Author

@coderabbitai please assess the implementation in this PR for completeness and correctness against the original issue:

## Context

PR #903 implements the release policy using the mapping boundary documented in the [ExecPlan decision log](https://github.com/leynos/netsuke/blob/use-trusted-packaging/docs/execplans/use-trusted-packaging.md#L3900). CodeRabbit proposed typed domain records with adapters responsible for decoding provider mappings. The maintainer has chosen to retain the existing boundary for PR #903 and consider that proposal separately.

## Follow-up

- Evaluate whether typed records would improve the release policy boundary compared with the existing validated in-memory mappings, including migration cost and testability.
- Record the architectural decision and rationale in the relevant documentation.
- If adopted, migrate the affected policy interfaces and adapters with behaviour-preserving tests, without weakening release identity or inventory checks.

This issue does not replace the remaining trusted-publishing implementation work: Stage C is partial and Stage D remains open in the ExecPlan.

## References

- https://github.com/leynos/netsuke/pull/903
- https://github.com/leynos/netsuke/pull/903#issuecomment-6448190491

@coderabbitai

coderabbitai Bot commented Nov 1, 2026

Copy link
Copy Markdown
Contributor

Tip

For best results, initiate chat on the files or code changes.

Analysis chain

Script executed:

#!/bin/bash
set -euo pipefail
pwd
git status --short
gh pr view 1052 --repo leynos/netsuke --json headRefOid,baseRefOid,files,statusCheckRollup
gh issue view 964 --repo leynos/netsuke --json title,body,state
fd -H '^AGENTS\.md$' .
git diff --stat origin/main...HEAD
cat AGENTS.md

Length of output: 11746


Script executed:

#!/bin/bash
set -euo pipefail
cat AGENTS.md
cat docs/adrs/adr-0068-typed-records-for-release-policy-boundaries.md
cat docs/execplans/issue-964-evaluate-typed-records-for-release-policy-boundaries.md
gh pr diff 1052 --repo leynos/netsuke -- scripts 2>/dev/null || gh api repos/leynos/netsuke/pulls/1052/files --paginate --jq '.[] | select(.filename | startswith("scripts/")) | "\nFILE: \(.filename)\n\(.patch)"'
ast-grep outline scripts/release_declaration.py scripts/release_declaration_cli.py scripts/release_digest_record.py scripts/release_digest_record_files.py scripts/release_identity_record.py

Length of output: 143118


Script executed:

#!/bin/bash
set -euo pipefail
gh api repos/leynos/netsuke/pulls/1052/files --paginate --jq '.[] | select((.filename | startswith("scripts/") | not) and (.filename | contains("adr-0068") or contains("issue-964") | not)) | "\nFILE: \(.filename)\n\(.patch)"'
rg -n -C2 --type py '\b(platform_target_counts|declared_asset_names|validate_digest_record|read_digest_record|require_record_matches_declaration|require_downloads_match_record)\s*\(' scripts tests/workflow_contracts
ast-grep outline scripts/release_candidate.py scripts/release_inventory.py scripts/verify_release_inventory_cli.py scripts/release_package_formats.py tests/workflow_contracts/release_publication_modules.py
sed -n '222,360p' scripts/release_digest_record.py
sed -n '50,135p' scripts/verify_release_inventory_cli.py
sed -n '103,190p' scripts/release_identity_record.py
rg -n -C4 --type py 'commit.*!=|!=.*commit|def .*identity|def validate_inventory|class MalformedTargetKind|class UnknownPlatformError' scripts/release_candidate.py scripts/release_inventory.py scripts/release_package_formats.py
cat tests/workflow_contracts/release_publication_modules.py
rg -n -C3 'Stage C|Stage D|9\.1\.1|ADR-0068|typed record' docs/roadmap.md docs/execplans/use-trusted-packaging.md docs/developers-guide.md docs/netsuke-design.md
rg -n -C2 --type py 'uppercase|upper\(|non.mapping|INVALID_MAPPING|MISSING_PLATFORM|staging_targets\(' scripts/tests/test_release_declaration* scripts/tests/test_release_digest_record*
git rev-parse HEAD

Length of output: 119307


Script executed:

#!/bin/bash
set -euo pipefail
sed -n '190,223p' scripts/release_declaration.py
sed -n '177,237p' scripts/release_declaration_cli.py
rg -n -C3 --type py '\.commit\s*(!=|==)|(!=|==)\s*[^#\n]*\.commit|recorded_commit|expected_commit' scripts
sed -n '269,330p' scripts/release_inventory.py
sed -n '1,100p' docs/contents.md
sed -n '1,100p' docs/repository-layout.md

Length of output: 4280


@leynos Treat #964 as functionally complete at 171809c19. The implementation covers all three issue requirements. I found no blocking correctness defect in the migrated production paths. Qualify the claim of unchanged diagnostics for one edge case before describing the migration as fully behaviour-preserving.

Assessment against the issue

Requirement Assessment Evidence
Evaluate records against validated mappings, including migration cost and testability Complete The ExecPlan inventories policy interfaces, callers, validation sites, and tests. It estimates signature, adapter, and fixture changes. ADR-0068 compares retained mappings, frozen records, and TypedDict.
Record the decision and rationale Complete ADR-0068 defines the adopted boundary and its exceptions. The design document, developers’ guide, documentation index, roadmap, and trusted-packaging decision log reference that decision.
Migrate adopted interfaces without weakening identity or inventory checks Complete, with the diagnostic qualification below Staging and trusted-digest interfaces now accept records. Adapters own external field names and decoding. The verifier retains inventory, coverage, and digest checks.

Correctness checks

  • Staging decoding: release_declaration_cli decodes targets before discovery runs. It passes the resulting StagingTargets to policy instead of decoding the configuration again.
  • Record invariants: Constructors reject empty records. StagingTarget rejects missing or unusable platform values. DigestRecord rejects malformed digests and duplicate asset names. Its factory preserves uppercase-digest acceptance by normalizing case.
  • Inventory protection: verify_release_inventory_cli.verify still checks the downloaded inventory, exact trusted-record coverage, and downloaded digests in that order. Digest comparison still checks every trusted asset, including missing downloads, and reports all mismatches.
  • Identity protection: The identity decoder’s executable code is unchanged. Reclassification as an adapter does not remove its required-field, identity-validation, or prerelease-flag checks.
  • Boundary scope: Retain the domain-shaped mappings used for GitHub reconciliation and computed download digests. Those mappings do not represent undecoded external records. Introducing records there is not required by #964.
  • Regression coverage: The changed suites cover eight malformed staging configurations and nine malformed digest payloads. They also cover direct construction, duplicate names, exact coverage, missing downloads, and multiple mismatches.

Non-blocking qualification

Document and test the changed diagnosis for platform = "".

The former _platform_of accepted any string, including an empty string. The policy’s platform lookup would reject that value as UnknownPlatformError. The new StagingTarget.__post_init__ rejects it earlier as MalformedTargetError(MISSING_PLATFORM).

The configuration remains rejected, and the CLI still returns the configuration-error exit code. No release protection is weakened. However, the exception category, diagnosis, and validation timing are not identical.

Add an empty-platform case to the malformed-configuration CLI table. Amend the “every rejection category survives with its original type and diagnosis” claim to identify this intentional tightening.

Scope and verification

Keep Stage C partial and Stage D open. Keep roadmap item 9.1.1 open. The changed documentation correctly separates those obligations from completion of #964. The PR does not modify .github/scripts/_release_admission/ or release_publish_path_*.py.

I inspected the source, tests, documentation, production call sites, and provider diff. GitHub reports successful CI, release dry-run, CodeScene, and security checks for this head. Several conditional checks are skipped. I did not execute repository code or independently reproduce the reported test counts.

Proceed with #964 closure through this PR. Do not interpret that closure as completion of trusted publishing.

You are interacting with an AI system.

leynos pushed a commit that referenced this pull request Nov 1, 2026
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 <noreply@anthropic.com>
@leynos
leynos force-pushed the issue-964-evaluate-typed-records-for-release-policy-boundaries branch from 171809c to b39e4e2 Compare November 1, 2026 07:45
codescene-access[bot]

This comment was marked as outdated.

leynos and others added 14 commits November 1, 2026 09:00
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 716f9b9.

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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
The decision is recorded as ADR-0068, the migration is landed, and the
final CodeRabbit review on 7a68b38 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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
Replay the branch onto 9bfb3e1, 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 08d0a91 and
f09b853 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 <noreply@anthropic.com>
@leynos
leynos force-pushed the issue-964-evaluate-typed-records-for-release-policy-boundaries branch from b39e4e2 to ee431dc Compare November 1, 2026 09:01
codescene-access[bot]

This comment was marked as outdated.

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 <noreply@anthropic.com>
codescene-access[bot]

This comment was marked as outdated.

@buzzybee-df12

Copy link
Copy Markdown
Collaborator

@coderabbitai Both qualification actions from your assessment are done at
c1a608460:

  • scripts/tests/test_release_declaration_invalid_config.py now carries a
    platform-is-empty case ([targets]\nlinux = { platform = "" }) that
    asserts the target without a platform diagnosis on stderr, alongside the
    eight existing malformed staging shapes.
  • ADR-0068 (decision driver and decision outcome), the issue-964 ExecPlan, and
    the trusted-packaging decision log now state that an empty platform string
    is refused earlier as MalformedTargetError(MISSING_PLATFORM) rather than
    later as UnknownPlatformError, with the same CLI exit status and no
    weakened identity or inventory check.

The full deterministic gate set passed on this head: check-fmt, typecheck,
lint, markdownlint, nixie, test (4943 passed, 7 skipped; doctests 164
passed, 33 ignored), test-workflow-contracts (1665 passed, 3 skipped),
test-release-publication (489 passed), and doc-coverage (98.23%).

Please confirm the qualification is satisfied, or identify anything else that
blocks describing the migration as behaviour-preserving apart from that
recorded tightening.

@coderabbitai

coderabbitai Bot commented Nov 3, 2026

Copy link
Copy Markdown
Contributor

Tip

For best results, initiate chat on the files or code changes.

Analysis chain

Script executed:

#!/bin/bash
set -euo pipefail
git rev-parse HEAD
fd -H '^AGENTS\.md$' .
cat AGENTS.md
ast-grep outline scripts/tests/test_release_declaration_invalid_config.py
rg -n -C 8 'platform-is-empty|target without a platform|stderr|parametrize|exit_code|returncode' scripts/tests/test_release_declaration_invalid_config.py
rg -n -C 5 'empty|MISSING_PLATFORM|UnknownPlatformError|tighten|diagnos' docs/adrs/adr-0068-typed-records-for-release-policy-boundaries.md docs/execplans/issue-964-evaluate-typed-records-for-release-policy-boundaries.md
rg -n -C 5 'MISSING_PLATFORM|UnknownPlatformError|empty.*platform|platform.*empty' docs/execplans/use-trusted-packaging.md
git show --stat --oneline c1a608460

Length of output: 181828


@buzzybee-df12 Treat the qualification as satisfied at c1a608460. I verified both changes in that checkout:

  • scripts/tests/test_release_declaration_invalid_config.py includes platform-is-empty. The test asserts target without a platform on stderr, EXIT_INPUT_UNUSABLE, and no declaration or digest-record output.
  • ADR-0068, the issue-964 ExecPlan, and the trusted-packaging decision log explicitly record the earlier rejection and changed exception category.

Describe the migration as “behaviour-preserving apart from the documented tightening for an empty platform string.” Retain that exception; do not claim identical diagnostics or validation timing for that input.

I identify no additional blocker to that description from this follow-up and the earlier assessment. This inspection confirms the test and documentation changes. It does not independently reproduce the reported gate results.

Keep completion of #964 separate from the remaining trusted-publishing work: Stage C remains partial, and Stage D remains open.

You are interacting with an AI system.

@leynos
leynos marked this pull request as ready for review November 3, 2026 03:22
@sourcery-ai

sourcery-ai Bot commented Nov 3, 2026

Copy link
Copy Markdown
Contributor

Sorry @leynos, you've used your own review budget of 250,000 diff characters for the last 7 days.

You can request another review in 2 days and 21 hours by commenting @sourcery-ai review. Upgrade to get a review now.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 3, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-03T03:33:06.591618Z c1a6084 Draft marked ready
🔒 Security Review ✅ Completed 2026-11-03T03:23:45.330420Z c1a6084 Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@coderabbitai coderabbitai Bot added Issue A pull request originating from an issue Roadmap A pull request originating from a roadmap item labels Nov 3, 2026
Comment thread scripts/release_digest_record_files.py Resolved
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 c1a6084.

Co-Authored-By: Claude Code <noreply@anthropic.com>
@wafflecat-df12

Copy link
Copy Markdown
Collaborator

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor
Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Additional comments: 1
docs/execplans/issue-964-evaluate-typed-records-for-release-policy-boundaries.md-297-297: 1

297-297: Minor · Maintainability & Code Quality · Quick win

Unverified finding
Verification ran but could not confirm this finding. It is shown for review, not as a verified issue.

Remove the comma before the essential because clause.

The clause explains why the note was wrong. Do not separate it with a comma.

Triage: [type:grammar]

Proposed correction
-      for `sorted` was wrong, because `_validated_entry` refuses a non-string
+      for `sorted` was wrong because `_validated_entry` refuses a non-string

Sources: Path instructions, Linters/SAST tools

Review info
Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: ASSERTIVE
  • Plan: Team
  • Run ID: 48b5d9ba-2e62-4b1d-9765-c21c1a964135
Commits

Reviewing files that changed from the base of the PR and between c1a6084 and 52f0a43.

Files selected for processing: 2
  • docs/execplans/issue-964-evaluate-typed-records-for-release-policy-boundaries.md
  • scripts/release_digest_record_files.py
Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

Included review availability: This review used your included allowance. 0 included reviews remain after this review. Your included PR review attempts over the past 7 days set your current allowance at 1 review per hour.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Issue A pull request originating from an issue Roadmap A pull request originating from a roadmap item

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Evaluate typed records for release policy boundaries

3 participants