Skip to content

Airlock v2 - #154

Closed
kenwalger wants to merge 2 commits into
mainfrom
airlock-v2
Closed

Airlock v2#154
kenwalger wants to merge 2 commits into
mainfrom
airlock-v2

Conversation

@kenwalger

Copy link
Copy Markdown
Owner

No description provided.

kenwalger added 2 commits July 5, 2026 19:54
…bling convention

Renames `packages/sovereign-sdk-airlock/` to `packages/sovereign-airlock/` to match
the established directory layout of sibling packages (`sovereign-core`, `sovereign-sieve`,
`sovereign-ledger`, `sovereign-edge`, `sovereign-sensor`).

Distribution name (`sovereign-sdk-airlock`), import namespace (`sovereign_airlock`), and
all runtime dependency declarations are unchanged.

Changes:
- packages/sovereign-sdk-airlock/ → packages/sovereign-airlock/ (directory rename)
- Updated file-path header comments in all 7 source modules under src/sovereign_airlock/
- uv.lock: editable path references updated from packages/sovereign-sdk-airlock to
  packages/sovereign-airlock; re-resolved cleanly (uv lock)
- README.md: workspace topography tree entry corrected
- CHANGELOG.md: workspace member path reference corrected
- ROADMAP.md: Phase 9.6 pyproject.toml path reference corrected

Verification: 469 passed, 1 skipped. Zero regressions.
…validation, prose tax threshold, payload immutability

Round 1 PR remediation pass for sovereign-sdk-airlock (v1.4.0).

Defect 1 — PolicyEngine regex boot-time validation (policy.py):
- PolicyRule.pattern type changed from str | None to re.Pattern[str] | None.
- _parse_rule() compiles all patterns via re.compile() at PolicyEngine.__init__
  time; re.error is caught and re-raised as AirlockConfigurationError immediately,
  guaranteeing bad configurations fail at startup rather than at evaluation time.
- _evaluate_raw() and _evaluate_fields() updated to call rule.pattern.search()
  on the pre-compiled Pattern object.
- New test: TestPolicyLoading.test_raises_on_malformed_regex_pattern.

Defect 2 — Prose Tax warning threshold lifecycle (policy.py, boundary.py):
- PolicyEngine.check_prose_tax_threshold(telemetry) added: returns a non-fatal
  warning when tax_savings_percentage falls below prose_tax_warning_threshold * 100.
  Threshold of 0.0 (default) disables the check.
- AirlockBoundary.process() calls check_prose_tax_threshold() after sieve
  convergence; warnings are extended onto verdict.warnings before evidence
  generation so they are sealed in receipt metadata.
- conftest.py: standard test policy prose_tax_warning_threshold reduced to 0.0
  to decouple baseline fixture from threshold behaviour tests.
- New tests: TestProseTaxThreshold (2 cases).

Defect 3 — Deep immutability on NormalizedPayload (payload.py):
- from __future__ import annotations added; import types introduced with no
  eager evaluation namespace leaks.
- __post_init__ converts: content → tuple[str, ...]; metadata →
  types.MappingProxyType[str, Any]; each tools entry →
  types.MappingProxyType[str, Any] stored in an outer tuple. object.__setattr__
  used to bypass frozen=True. Constructor still accepts plain list/dict equivalents.
- default_factory updated to MappingProxyType({}) for metadata and tuple for tools.
- New tests: TestNormalizedPayloadImmutability (4 cases).

Test delta: 59 → 66 cases (+7). 476 passed, 1 skipped workspace-wide. Zero regressions.
Updates: CHANGELOG.md, ROADMAP.md.
@greptile-apps

greptile-apps Bot commented Jul 6, 2026

Copy link
Copy Markdown

Greptile Summary

This PR introduces sovereign-sdk-airlock, a new workspace package implementing the model-neutral outbound governance boundary (SAR-0004). It adds a four-component lifecycle — policy evaluation, sieve convergence, telemetry assembly, and evidence generation — backed by a 66-case test suite.

  • PolicyEngine (policy.py): evaluates YAML-declared rules across raw, fields, and telemetry scopes with allow/warn/deny actions; all regex patterns are pre-compiled at init time.
  • AirlockBoundary (boundary.py): async orchestrator wiring the full lifecycle; deny verdict raises before any sieve or ledger operation; ledger write failure is non-fatal by design.
  • NormalizedPayload (payload.py) and AirlockTelemetry (telemetry.py): deep-immutable dataclasses providing the stable governance surface and sieve metrics respectively.

Confidence Score: 3/5

The payload, receipt, and sieve layers are solid, but the policy engine has a correctness gap that would silently misfire for any telemetry rule targeting sieved_tokens or tax_savings_percentage.

In _evaluate_telemetry, when no post-sieve telemetry is available (the common path in AirlockBoundary.process), every telemetry-scope rule falls back to comparing payload.token_estimate against the rule threshold regardless of which metric is named. For metric: raw_tokens this approximation is fine, but for metric: tax_savings_percentage, threshold: 50.0 the comparison becomes token_estimate > 50.0 — nearly always true for any non-trivial payload — which means the rule would fire constantly on the wrong condition.

policy.py — specifically _evaluate_telemetry and the PolicyRule.fields mutability gap.

Important Files Changed

Filename Overview
packages/sovereign-airlock/src/sovereign_airlock/policy.py Core policy engine with a logic bug: when telemetry is absent, all metric types fall back to token_estimate, causing wrong evaluations for sieved_tokens and tax_savings_percentage metrics. PolicyRule also has a shallow-immutability gap (mutable list in frozen dataclass).
packages/sovereign-airlock/src/sovereign_airlock/boundary.py Async orchestrator for the four-component Airlock lifecycle; clean structure with non-fatal error handling for both ledger write and signing failures.
packages/sovereign-airlock/src/sovereign_airlock/telemetry.py Frozen telemetry dataclass with ZeroDivisionError guard; tax_savings_percentage lacks a lower-bound clamp against negative sieve expansions.
packages/sovereign-airlock/src/sovereign_airlock/payload.py Provider-neutral NormalizedPayload with deep immutability via MappingProxyType and tuple conversion; factory functions for OpenAI, Anthropic, and raw payloads look correct.
packages/sovereign-airlock/src/sovereign_airlock/receipt.py ReceiptBuilder correctly separates signing (fatal) from ledger commit (non-fatal); metadata assembly matches the documented structure.
packages/sovereign-airlock/tests/test_boundary.py 66-case async test suite covering the full boundary lifecycle, denial, warning, transport neutrality, and resiliency paths; asyncio_mode auto configured at workspace level.
packages/sovereign-airlock/tests/test_policy.py Policy tests cover all three scopes, actions, global ceiling, and regex boot-time validation; however no test exercises sieved_tokens or tax_savings_percentage metrics with telemetry=None to catch the fallback bug.
packages/sovereign-airlock/pyproject.toml New workspace member at version 1.4.0 with correct runtime dependencies and Python 3.12 requirement.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant C as Caller
    participant B as AirlockBoundary
    participant P as PolicyEngine
    participant S as sovereign_sieve
    participant R as ReceiptBuilder
    participant L as SovereignLedger

    C->>B: process(NormalizedPayload)
    B->>P: evaluate(payload) [pre-sieve, no telemetry]
    P-->>B: PolicyVerdict
    alt "verdict.allowed == False"
        B-->>C: raise AirlockPolicyViolation
    end
    B->>S: sieve_with_metrics(raw_content)
    S-->>B: SieveOutput
    B->>B: AirlockTelemetry.from_sieve_output()
    B->>P: check_prose_tax_threshold(telemetry)
    P-->>B: prose_tax_warnings (may be empty)
    B->>R: build_and_commit(sieved_content, telemetry, warnings, source)
    R->>R: key_manager.generate_receipt()
    R->>L: append_receipt(receipt, sieved_content)
    alt ledger write fails
        L-->>R: Exception (non-fatal)
        R-->>B: ForensicReceipt (still returned)
    else success
        L-->>R: ok
        R-->>B: ForensicReceipt
    end
    B-->>C: AirlockResult(sieved_content, telemetry, receipt, warnings)
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 C as Caller
    participant B as AirlockBoundary
    participant P as PolicyEngine
    participant S as sovereign_sieve
    participant R as ReceiptBuilder
    participant L as SovereignLedger

    C->>B: process(NormalizedPayload)
    B->>P: evaluate(payload) [pre-sieve, no telemetry]
    P-->>B: PolicyVerdict
    alt "verdict.allowed == False"
        B-->>C: raise AirlockPolicyViolation
    end
    B->>S: sieve_with_metrics(raw_content)
    S-->>B: SieveOutput
    B->>B: AirlockTelemetry.from_sieve_output()
    B->>P: check_prose_tax_threshold(telemetry)
    P-->>B: prose_tax_warnings (may be empty)
    B->>R: build_and_commit(sieved_content, telemetry, warnings, source)
    R->>R: key_manager.generate_receipt()
    R->>L: append_receipt(receipt, sieved_content)
    alt ledger write fails
        L-->>R: Exception (non-fatal)
        R-->>B: ForensicReceipt (still returned)
    else success
        L-->>R: ok
        R-->>B: ForensicReceipt
    end
    B-->>C: AirlockResult(sieved_content, telemetry, receipt, warnings)
Loading

Comments Outside Diff (1)

  1. packages/sovereign-airlock/src/sovereign_airlock/policy.py, line 1124-1134 (link)

    P1 Telemetry fallback collapses all metrics to token_estimate

    When telemetry is None, the code sets metric_value = payload.token_estimate regardless of which rule.metric is configured. For a rule with metric: tax_savings_percentage, threshold: 50.0, the comparison becomes token_estimate > 50.0 — a nearly-always-true condition for any payload with more than 50 tokens — instead of evaluating percentage savings. For metric: sieved_tokens the semantics are also wrong since token_estimate is a pre-sieve value.

    In AirlockBoundary.process() the call is always self._policy.evaluate(payload) with no telemetry argument, so every telemetry-scope rule runs through this fallback path. Rules targeting sieved_tokens or tax_savings_percentage will produce incorrect verdicts at runtime. The fallback is only sound for raw_tokens, where token_estimate is an intentional proxy.

Reviews (1): Last reviewed commit: "fix(airlock): resolve three PR remediati..." | Re-trigger Greptile

scope: str
action: str
pattern: re.Pattern[str] | None = None
fields: list[str] = field(default_factory=list)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 PolicyRule is declared frozen=True, but its fields attribute is a plain list[str]. The dataclass freeze only prevents reassignment of the reference — rule.fields.append("injected") still mutates the list in place. Converting to a tuple (which is the pattern already used for NormalizedPayload.content) provides genuine deep immutability consistent with the "immutable descriptor" contract in the docstring.

Suggested change
fields: list[str] = field(default_factory=list)
fields: tuple[str, ...] = field(default_factory=tuple)

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Comment on lines +60 to +63
else:
tax_savings_percentage = round(
(raw_tokens - sieved_tokens) / raw_tokens * 100.0, 4
)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 The docstring declares tax_savings_percentage has a range of [0.0, 100.0], but the formula (raw_tokens - sieved_tokens) / raw_tokens * 100.0 can produce a negative value if the sieve expands the content (sieved_tokens > raw_tokens). A degenerate sieve pass or mocked output could yield negative savings that would always satisfy < threshold_pct in check_prose_tax_threshold, causing spurious warnings. Clamping to zero aligns the actual range with the documented contract.

Suggested change
else:
tax_savings_percentage = round(
(raw_tokens - sieved_tokens) / raw_tokens * 100.0, 4
)
else:
tax_savings_percentage = round(
max(0.0, (raw_tokens - sieved_tokens) / raw_tokens * 100.0), 4
)

@kenwalger kenwalger closed this Jul 6, 2026
@kenwalger
kenwalger deleted the airlock-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