Skip to content

Repo polish v2 - #161

Closed
kenwalger wants to merge 2 commits into
mainfrom
repo-polish-v2
Closed

Repo polish v2#161
kenwalger wants to merge 2 commits into
mainfrom
repo-polish-v2

Conversation

@kenwalger

Copy link
Copy Markdown
Owner

No description provided.

kenwalger added 2 commits July 5, 2026 22:23
…ne, warning preservation, SPDX migration

Round 8 — exception de-duplication (policy.py): AirlockConfigurationError extends
ValueError, so clean boot-time validation errors raised inside _parse_rule were being
caught by the broad except (KeyError, TypeError, ValueError) handler in
PolicyEngine.__init__ and silently re-wrapped with a redundant "Invalid rule definition"
prefix. An explicit except AirlockConfigurationError: raise guard inserted before the
broad handler lets those errors surface natively.

Round 8 — pre-sieve warning preservation (exception.py, boundary.py):
AirlockPolicyViolation.__init__ now accepts optional warnings: list[str] | None (stored
as self.warnings). When AirlockBoundary.process() raises AirlockPolicyViolation on a
post-sieve deny verdict, it passes verdict.warnings so callers can inspect accumulated
pre-sieve warn-action diagnostic context via exc.warnings without losing it at the
exception boundary. RuntimeError message contract is unchanged; all existing callers
remain compatible.

Release prep — SPDX license metadata (pyproject.toml): project.license migrated from
the deprecated TOML table form ({ text = "MIT" }) to the SPDX string literal ("MIT").
Deprecated License :: OSI Approved :: MIT License classifier removed. Eliminates two
SetuptoolsDeprecationWarning emissions that become hard build errors after 2027-02-18.

PEP 517 build validated: uv build --package sovereign-sdk-airlock produces clean sdist
and pure-Python wheel with no editable-path leakage and zero deprecation warnings.
Wheel METADATA confirmed: Name, Version, Requires-Python, Description-Content-Type,
all Requires-Dist entries, and full README.md long-description (3,401 bytes, UTF-8).

Documentation: CHANGELOG.md [Unreleased] promoted to [1.4.0] — 2026-07-05. ROADMAP.md
Phase 9.6 delivered list updated. Package and workspace README.md updated with
AirlockPolicyViolation.warnings inspection pattern. 84 tests pass, 0 regressions.
…tion

Pre-sieve deny path now passes verdict.warnings to AirlockPolicyViolation, closing the
diagnostic asymmetry with the post-sieve deny path (which already forwarded warnings in
Round 8). Both deny paths now carry accumulated warn-rule messages on exc.warnings for
caller inspection, completing the uniform diagnostic contract across the full
AirlockBoundary.process() lifecycle.

New test test_pre_sieve_deny_preserves_accumulated_warnings (TestAirlockBoundaryPolicyDenial):
a payload containing both the guard_internal_namespaces warn pattern and the
block_private_keys deny pattern triggers both rules pre-sieve; asserts that
exc.warnings is non-empty and contains the guard_internal_namespaces message.

Test delta: 84 → 85 cases. 85 passed, 0 failed, 0 regressions.
@greptile-apps

greptile-apps Bot commented Jul 6, 2026

Copy link
Copy Markdown

Greptile Summary

This PR adds a warnings diagnostic attribute to AirlockPolicyViolation so that pre-sieve warn-action messages are not lost when a deny exception propagates, fixes AirlockConfigurationError double-wrapping in PolicyEngine.__init__, and migrates pyproject.toml to SPDX license metadata.

  • exception.py / boundary.py: AirlockPolicyViolation gains an optional warnings parameter; both the pre-sieve and post-sieve deny raise sites now forward verdict.warnings — though the post-sieve path omits post_verdict.warnings, silently dropping any prose-tax or telemetry warn-action messages that fired in the same post-sieve pass.
  • policy.py: An explicit except AirlockConfigurationError: raise guard is inserted before the broad except (KeyError, TypeError, ValueError) handler so clean validation errors from _parse_rule surface with their original message rather than a redundant "Invalid rule definition" prefix.
  • pyproject.toml: license migrated from deprecated TOML table form to SPDX string literal; deprecated OSI classifier removed.

Confidence Score: 4/5

Safe to merge with the understanding that post-sieve deny exceptions will carry incomplete warning context until the one-line fix is applied.

The post-sieve deny path in boundary.py passes only verdict.warnings (pre-sieve) to the exception, discarding any post_verdict.warnings accumulated during the same evaluate_post_sieve call — including prose-tax threshold warnings and any concurrent warn-action telemetry rules. This is a gap in the very feature this PR introduces, and the new test suite covers only the pre-sieve case, leaving the post-sieve scenario untested.

packages/sovereign-airlock/src/sovereign_airlock/boundary.py — the post-sieve raise path at line 151.

Important Files Changed

Filename Overview
packages/sovereign-airlock/src/sovereign_airlock/boundary.py Post-sieve deny now carries pre-sieve warnings on the exception; however, the post-sieve pass's own warnings (prose-tax, warn-action telemetry rules) are silently dropped in the same raise path.
packages/sovereign-airlock/src/sovereign_airlock/exception.py Adds warnings: list[str] attribute to AirlockPolicyViolation via a correctly defensive list(warnings) copy in __init__; no issues.
packages/sovereign-airlock/src/sovereign_airlock/policy.py Adds except AirlockConfigurationError: raise guard before the broad ValueError/KeyError/TypeError handler to prevent clean validation errors from being re-wrapped with a redundant prefix; correct Python exception-ordering idiom.
packages/sovereign-airlock/tests/test_boundary.py New test covers pre-sieve deny preserving warn-rule messages; no test for the post-sieve deny + post-sieve warnings scenario, leaving the dropped-warnings bug undetected.
packages/sovereign-airlock/pyproject.toml Migrates license from deprecated TOML table form to SPDX string literal and removes the deprecated OSI classifier; correct PEP 639 / Setuptools guidance.
CHANGELOG.md Version bumped to 1.4.0 with accurate entries for warnings attribute, de-duplication fix, and SPDX migration.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant Caller
    participant AirlockBoundary
    participant PolicyEngine
    participant Sieve

    Caller->>AirlockBoundary: process(payload)
    AirlockBoundary->>PolicyEngine: evaluate(payload)
    PolicyEngine-->>AirlockBoundary: verdict (allowed/denied, warnings[])

    alt pre-sieve deny
        AirlockBoundary-->>Caller: "raise AirlockPolicyViolation(violations, warnings=verdict.warnings)"
    else allowed
        AirlockBoundary->>Sieve: sieve_with_metrics(content)
        Sieve-->>AirlockBoundary: sieve_output + telemetry
        AirlockBoundary->>PolicyEngine: evaluate_post_sieve(telemetry)
        PolicyEngine-->>AirlockBoundary: post_verdict (allowed/denied, post_warnings[])

        alt post-sieve deny
            Note over AirlockBoundary: post_verdict.warnings dropped
            AirlockBoundary-->>Caller: "raise AirlockPolicyViolation(violations, warnings=verdict.warnings only)"
        else allowed
            AirlockBoundary->>AirlockBoundary: verdict.warnings.extend(post_verdict.warnings)
            AirlockBoundary-->>Caller: AirlockResult(sieved_content, telemetry, receipt, policy_warnings)
        end
    end
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant Caller
    participant AirlockBoundary
    participant PolicyEngine
    participant Sieve

    Caller->>AirlockBoundary: process(payload)
    AirlockBoundary->>PolicyEngine: evaluate(payload)
    PolicyEngine-->>AirlockBoundary: verdict (allowed/denied, warnings[])

    alt pre-sieve deny
        AirlockBoundary-->>Caller: "raise AirlockPolicyViolation(violations, warnings=verdict.warnings)"
    else allowed
        AirlockBoundary->>Sieve: sieve_with_metrics(content)
        Sieve-->>AirlockBoundary: sieve_output + telemetry
        AirlockBoundary->>PolicyEngine: evaluate_post_sieve(telemetry)
        PolicyEngine-->>AirlockBoundary: post_verdict (allowed/denied, post_warnings[])

        alt post-sieve deny
            Note over AirlockBoundary: post_verdict.warnings dropped
            AirlockBoundary-->>Caller: "raise AirlockPolicyViolation(violations, warnings=verdict.warnings only)"
        else allowed
            AirlockBoundary->>AirlockBoundary: verdict.warnings.extend(post_verdict.warnings)
            AirlockBoundary-->>Caller: AirlockResult(sieved_content, telemetry, receipt, policy_warnings)
        end
    end
Loading

Reviews (1): Last reviewed commit: "fix(airlock): close pre-sieve warning as..." | Re-trigger Greptile

Comment on lines 147 to +152
post_verdict = self._policy.evaluate_post_sieve(telemetry)
if not post_verdict.allowed:
raise AirlockPolicyViolation("; ".join(post_verdict.violations))
raise AirlockPolicyViolation(
"; ".join(post_verdict.violations),
warnings=verdict.warnings,
)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Post-sieve deny drops concurrent post-sieve warnings. When a post-sieve deny fires, the code passes only verdict.warnings (pre-sieve) to the exception, silently discarding any post_verdict.warnings accumulated during the same evaluate_post_sieve call. This includes prose-tax threshold warnings and any warn-action telemetry rules that fired alongside the deny. A caller inspecting exc.warnings for diagnostics will be missing half the context in this scenario.

Suggested change
post_verdict = self._policy.evaluate_post_sieve(telemetry)
if not post_verdict.allowed:
raise AirlockPolicyViolation("; ".join(post_verdict.violations))
raise AirlockPolicyViolation(
"; ".join(post_verdict.violations),
warnings=verdict.warnings,
)
post_verdict = self._policy.evaluate_post_sieve(telemetry)
if not post_verdict.allowed:
raise AirlockPolicyViolation(
"; ".join(post_verdict.violations),
warnings=verdict.warnings + post_verdict.warnings,
)

@kenwalger kenwalger closed this Jul 6, 2026
@kenwalger
kenwalger deleted the repo-polish-v2 branch July 6, 2026 05:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant