feat: freeze closed resolve OpenAPI v0 - #74
Conversation
|
Warning Review limit reached
Next review available in: 28 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughThis PR freezes the public ChangesOpenAPI v0 contract and runtime lineage
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
engine/runtime/release_lineage.py (1)
180-196: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueOptional:
__all__isn't isort-sorted (Ruff RUF022).
CONTENT_PROFILE_DIGEST_V0precedesCONTENT_SCHEMA_REF_V0-ordered peers andCURATION_PROFILE_DIGEST_V0precedesCURATION_PROFILE_REF_V0. Only actionable if RUF022 is enabled in the enforced lint config; otherwise ignore.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@engine/runtime/release_lineage.py` around lines 180 - 196, Sort the names in __all__ alphabetically to satisfy Ruff RUF022, specifically placing CONTENT_SCHEMA_REF_V0 before CONTENT_PROFILE_REF_V0 as appropriate and ordering each CURATION, INDEX, PACKAGE, and RUNTIME group consistently. Keep the exported symbol set unchanged.Source: Linters/SAST tools
.github/workflows/ci.yml (1)
13-15: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winSet
persist-credentials: falseon checkout.With
fetch-depth: 0, the full clone is retained and the checkout token stays persisted in.git/configby default. Since later steps only need local history forgit show/git rev-parse, disabling credential persistence closes the artipacked exposure without affecting the baseline checks.🔒 Proposed change
- uses: actions/checkout@v4 with: fetch-depth: 0 + persist-credentials: false🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/ci.yml around lines 13 - 15, Update the actions/checkout@v4 configuration to set persist-credentials to false alongside fetch-depth: 0, preserving the full local history while preventing the checkout token from being stored in .git/config.Source: Linters/SAST tools
tests/catalog/test_validate_security_catalog.py (1)
133-149: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winBroaden the on-disk drift guard to every canonical activation. This new check only walks
CANONICAL_OPENAPI_V0_ACTIVATION, so the other eight canonical activations can still silently reference a renamed/deleted test node: the frozen tests assert thesurfacestring equals the canonical constant, but nothing verifies the referenceddef <node>still exists on disk. IteratingCANONICAL_ACTIVATIONScloses that gap uniformly.♻️ Suggested generalization
-from scripts.validate_security_catalog import ( - CANONICAL_OPENAPI_V0_ACTIVATION, +from scripts.validate_security_catalog import ( + CANONICAL_ACTIVATIONS,def test_active_evidence_surfaces_reference_existing_test_nodes() -> None: """Canonical active evidence cannot drift to a renamed or deleted test.""" - test_evidence = CANONICAL_OPENAPI_V0_ACTIVATION["testEvidence"] - assert isinstance(test_evidence, list) - for activation in test_evidence: - assert isinstance(activation, dict) - surface = activation["surface"] - assert isinstance(surface, str) - for reference in surface.split(): - file_ref, separator, node_ref = reference.partition("::") - path = Path(__file__).parents[2] / file_ref - assert path.is_file(), reference - if separator: - source = path.read_text(encoding="utf-8") - assert f"def {node_ref}(" in source, reference + for activation in CANONICAL_ACTIVATIONS: + test_evidence = activation["testEvidence"] + assert isinstance(test_evidence, list) + for evidence in test_evidence: + assert isinstance(evidence, dict) + surface = evidence["surface"] + assert isinstance(surface, str) + for reference in surface.split(): + file_ref, separator, node_ref = reference.partition("::") + path = Path(__file__).parents[2] / file_ref + assert path.is_file(), reference + if separator: + source = path.read_text(encoding="utf-8") + assert f"def {node_ref}(" in source, referenceNote: confirm every surface referenced by the other eight activations resolves on disk before merging this broadening.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/catalog/test_validate_security_catalog.py` around lines 133 - 149, Broaden test_active_evidence_surfaces_reference_existing_test_nodes to iterate over every activation in CANONICAL_ACTIVATIONS instead of only CANONICAL_OPENAPI_V0_ACTIVATION. Preserve the existing surface parsing and on-disk path/node validation, and confirm all canonical activation surfaces resolve successfully.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@engine/runtime/construction.py`:
- Around line 360-362: Update the policy-epoch comparison in the egress veto
condition within finalize-for-delivery logic so it rejects any mismatch among
provenance.policy_epoch, invocation.policy_epoch, and
invocation.user_actor.policy_epoch. Replace the chained inequality with explicit
pairwise mismatch checks, preserving the gate’s existing behavior when all three
values agree.
In `@eval/catalogs/security-catalog.schema.json`:
- Around line 772-798: Extend the reusable activationTestEvidence.id enum and
activation.carrier enum in the security catalog schema to include all new `#66`
test-evidence IDs and carrier values already represented by the activation const
entries. Keep existing enum values unchanged and ensure reusable-definition
validation accepts the same `#66-shaped` activations as the catalog instance.
---
Nitpick comments:
In @.github/workflows/ci.yml:
- Around line 13-15: Update the actions/checkout@v4 configuration to set
persist-credentials to false alongside fetch-depth: 0, preserving the full local
history while preventing the checkout token from being stored in .git/config.
In `@engine/runtime/release_lineage.py`:
- Around line 180-196: Sort the names in __all__ alphabetically to satisfy Ruff
RUF022, specifically placing CONTENT_SCHEMA_REF_V0 before CONTENT_PROFILE_REF_V0
as appropriate and ordering each CURATION, INDEX, PACKAGE, and RUNTIME group
consistently. Keep the exported symbol set unchanged.
In `@tests/catalog/test_validate_security_catalog.py`:
- Around line 133-149: Broaden
test_active_evidence_surfaces_reference_existing_test_nodes to iterate over
every activation in CANONICAL_ACTIVATIONS instead of only
CANONICAL_OPENAPI_V0_ACTIVATION. Preserve the existing surface parsing and
on-disk path/node validation, and confirm all canonical activation surfaces
resolve successfully.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 733d0f4d-6115-4d08-88d0-e021c6bd0b62
📒 Files selected for processing (59)
.github/workflows/ci.ymlMakefileadapters/http/app.pyadapters/http/contracts.pyadapters/http/route_policy.pyadapters/http/transport.pydocs/decisions/0026-normalize-no-authorized-evidence.mddocs/decisions/0047-freeze-openapi-v0-through-one-runtime-path.mddocs/decisions/README.mdengine/persistence/membership_context.pyengine/persistence/schema_security_manifest.yamlengine/runtime/actor.pyengine/runtime/construction.pyengine/runtime/context_run.pyengine/runtime/contracts.pyengine/runtime/package_digest.pyengine/runtime/release_lineage.pyeval/catalogs/security-catalog.schema.jsoneval/catalogs/security-invariants.yamlmigrations/versions/20260723_0021_runtime_release_observation.pyopenapi/v0/openapi.jsonopenapi/v0/openapi.sha256scripts/freeze_openapi.pyscripts/validate_security_catalog.pytests/catalog/test_validate_security_catalog.pytests/integration/conftest.pytests/integration/test_file_import_tracer.pytests/integration/test_membership_field_projection_integration.pytests/integration/test_migrations.pytests/integration/test_runtime_authorized_evidence_integration.pytests/integration/test_runtime_empty_package_integration.pytests/integration/test_runtime_non_enumeration_integration.pytests/integration/test_runtime_policy_epoch_integration.pytests/integration/test_z_egress_grant_file.pytests/integration/test_zz_file_content_noop.pytests/integration/test_zz_file_resource_tombstone.pytests/integration/test_zz_file_revision_replacement.pytests/integration/test_zz_file_source_offboarding.pytests/process/conformance_app.pytests/process/test_processes.pytests/support/releases.pytests/unit/test_actor_contracts.pytests/unit/test_context_run.pytests/unit/test_database_harness_contract.pytests/unit/test_effective_scope_runtime.pytests/unit/test_egress_grant.pytests/unit/test_http_authorized_evidence_contract.pytests/unit/test_http_trust_boundary.pytests/unit/test_http_unavailable_capabilities.pytests/unit/test_membership_context.pytests/unit/test_openapi_v0_contract.pytests/unit/test_openapi_v0_snapshot.pytests/unit/test_package_digest.pytests/unit/test_runtime_authorized_evidence.pytests/unit/test_runtime_contracts.pytests/unit/test_runtime_empty_package.pytests/unit/test_runtime_unavailable_capabilities.pytests/unit/test_schema_security_manifest.pytests/unit/test_ticket_audience_separation.py
| "id": { | ||
| "enum": [ | ||
| "PG-REVOCATION-006", | ||
| "RUN-006", | ||
| "CACHE-002", | ||
| "RUN-UNAVAILABLE-016", | ||
| "HTTP-UNAVAILABLE-016", | ||
| "LEASE-SIGNING-017", | ||
| "PG-WORKER-LEASE-NOOP-017", | ||
| "WORKER-LEASE-REPLAY-007", | ||
| "TICKET-AUDIENCE-018", | ||
| "PG-TICKET-EPOCH-018", | ||
| "DIGEST-019", | ||
| "RUN-LINEAGE-019", | ||
| "AUTHORIZED-RUN-019", | ||
| "PG-TRACE-REDACTION-012", | ||
| "PROP-FIELD-PROJECTION-048", | ||
| "PG-FIELD-PROJECTION-048", | ||
| "HTTP-ACCEPT-002-048", | ||
| "PROP-DELIVERY-EVIDENCE-063", | ||
| "PG-DELIVERY-EVIDENCE-063", | ||
| "HTTP-DELIVERY-EVIDENCE-063", | ||
| "FILE-DELIVERY-EVIDENCE-063", | ||
| "PROP-EGRESS-011", | ||
| "PG-EGRESS-011", | ||
| "RUNTIME-EGRESS-011" | ||
| ] |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check whether the reusable activation/activationTestEvidence $defs are used to validate
# any activation instances (vs. only the const prefixItems), and whether `#66` ids/carrier appear in the enums.
rg -nP '066|activationTestEvidence|`#/`\$defs/activation\b' eval/catalogs/security-catalog.schema.json
rg -nP 'activationTestEvidence|\$defs/activation|carrier' scripts/validate_security_catalog.py tests/catalog/test_validate_security_catalog.pyRepository: stone16/context-engine
Length of output: 15935
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== schema activation $defs around activationTestEvidence and carrier =="
sed -n '740,900p' eval/catalogs/security-catalog.schema.json | cat -n
echo
echo "== `#66` activation section =="
sed -n '450,535p' eval/catalogs/security-catalog.schema.json | cat -n
echo
echo "== validation script activation and carrier definitions =="
sed -n '980,1150p' scripts/validate_security_catalog.py | cat -n
sed -n '1740,1835p' scripts/validate_security_catalog.py | cat -n
sed -n '2920,3155p' scripts/validate_security_catalog.py | cat -n
echo
echo "== occurrences in validation script =="
rg -n 'ACTIVATION_TEST_EVIDENCE_FIELDS|CARRIER_FIELDS|activationTestEvidence|carrierStatusAtM0|`#66`|OPENAPI-CONTRACT-066|PG-RUNTIME-RELEASE-066' scripts/validate_security_catalog.pyRepository: stone16/context-engine
Length of output: 197
🏁 Script executed:
#!/bin/bash
# Programmatic check: extract JSON enums from the schema and compare against the new `#66` fields.
# Avoid executing repository code; parse the static schema JSON and use pytest-style structural expectations.
python3 - <<'PY'
import json
from pathlib import Path
schema_path = Path("eval/catalogs/security-catalog.schema.json")
schema = json.loads(schema_path.read_text())
# Locate activationTestEvidence/id enum and activation/carrier enum in static definitions.
defs = schema.get("$defs", {})
activation_test_evidence = defs.get("activationTestEvidence")
activation_def = defs.get("activation")
print("schema has activationTestEvidence", bool(activation_test_evidence))
print("activationTestEvidence item definition:", activation_test_evidence.get("items") if activation_test_evidence else None)
print("schema has activation defi:", bool(activation_def))
def deep_find_id_enum(obj):
if isinstance(obj, dict) and obj.get("enum"):
# Heuristic narrow match for id enum: first entry is a string.
if all(isinstance(e, str) for e in obj["enum"]):
return obj["enum"]
if isinstance(obj, (dict, list)):
for v in obj.values() if isinstance(obj, dict) else obj:
ret = deep_find_id_enum(v)
if ret:
return ret
return None
id_enum = deep_find_id_enum(activation_test_evidence)
carrier_enum = activation_def.get("items") if activation_def else None
print("id enum length", len(id_enum) if id_enum else None)
print("id enum contains `#66` values", id_enum and all(x in id_enum for x in [
"OPENAPI-CONTRACT-066",
"OPENAPI-BREAKING-066",
"HTTP-V0-066",
"PG-RUNTIME-RELEASE-066",
]))
# Extract activation prefixItems enum entries from the schema.
activations = defs.get("activations", {})
prefix_items = activations.get("items", {}).get("prefixItems", [])
print("activations definition prefixItems count", len(prefix_items))
target_id_values = []
carrier_values = []
for item in prefix_items:
if isinstance(item, dict) and item.get("id") in [
"OPENAPI-CONTRACT-066",
"OPENAPI-BREAKING-066",
"HTTP-V0-066",
"PG-RUNTIME-RELEASE-066",
]:
target_id_values.append(item.get("id"))
if isinstance(item, dict) and item.get("carrier"):
carrier_values.append(item.get("carrier"))
print("`#66` target values present in activations.prefixItems:", target_id_values)
print("`#66` carrier present in activations.prefixItems:", any(v == "frozen public POST /v0/resolve OpenAPI contract" for v in carrier_values))
print("carrier enum missing `#66` carrier:", carrier_enum is None or not isinstance(carrier_enum, dict) or carrier_enum.get("const") != "frozen public POST /v0/resolve OpenAPI contract")
# Basic consistency check: in any prefixItems entry, the id must either be const/enum-validated by id field,
# and carrier must either be const/carrier value equal. For `#66` entries, schema enum absence means this instance
# violates the reusable activation.items schema if that schema is applied, not the const instance.
for item in prefix_items:
if isinstance(item, dict) and item.get("id") in target_id_values:
print("conflict found:", item.get("id"), "; carrier:", item.get("carrier"))
print("id field enum includes value:", (activation_test_evidence or {}).get("items") and item["id"] in id_enum)
PYRepository: stone16/context-engine
Length of output: 522
Add the #66 activation values to the reusable schema enums.
#66 introduces new test-evidence ids and a carrier that are only covered by the activation const entries. That keeps this catalog instance valid, but the reusable activationTestEvidence.id enum and activation.carrier enum remain closed to those values, so validation against the reusable definitions would reject a #66-shaped activation. Extend both enums with the new id and carrier values.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@eval/catalogs/security-catalog.schema.json` around lines 772 - 798, Extend
the reusable activationTestEvidence.id enum and activation.carrier enum in the
security catalog schema to include all new `#66` test-evidence IDs and carrier
values already represented by the activation const entries. Keep existing enum
values unchanged and ensure reusable-definition validation accepts the same
`#66-shaped` activations as the catalog instance.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 496a113d60
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| item["citationOpenRef"] = None | ||
| document["continuation"] = None |
There was a problem hiding this comment.
When ContextPackageWire validates a package, it builds digest_document from the incoming model and calls this helper before verify_context_package_digest. These assignments overwrite any non-null citationOpenRef or continuation, so a package can carry an injected continuation/citation capability while reusing the packageDigest for the null version; any consumer using the wire model as the frozen v0 verifier will accept fields that are outside the digested package closure. Reject non-null inactive fields, or include their actual values, before digest verification.
Useful? React with 👍 / 👎.
| connection.execute( | ||
| text( | ||
| """ | ||
| SELECT pg_catalog.pg_advisory_xact_lock_shared( |
There was a problem hiding this comment.
This shared advisory lock does not serialize release observation today: the ContextLearning promotion path (context_learning_promote_release/promote_atomically) only locks the active_release_manifest row with FOR UPDATE and never takes context-engine.release: exclusively. During a concurrent promotion, /v0/resolve can read release A, then promotion B commits before the later package/context_run commit, causing the response asOf to be stamped after A stopped being active; add the same keyed exclusive lock to promotion or use row locks on the read path.
Useful? React with 👍 / 👎.
1 participant
Closes #66
Summary
Verification
Summary by CodeRabbit
POST /v0/resolveAPI with bearer authentication and request validation.