Skip to content

Define the protocol-neutral responsibility contract - #136

Merged
George-RD merged 26 commits into
mainfrom
agent/responsibility-contract
Aug 3, 2026
Merged

Define the protocol-neutral responsibility contract#136
George-RD merged 26 commits into
mainfrom
agent/responsibility-contract

Conversation

@George-RD

@George-RD George-RD commented Aug 1, 2026

Copy link
Copy Markdown
Owner

Summary

Defines the protocol-neutral responsibility contract required by #126.

  • separates canonical action semantics from concrete resolver/executor readiness;
  • adds sealed kernel-resolved context and generic reviewed-scope derivation/comparison;
  • adds evidence integrity, channel-neutral owner review, responsibility reference-view, and deterministic drift contracts;
  • registers reviewed semantics for email.create_draft while deliberately failing closed until Unify approved and delegated effect execution; fail closed on missing executors #127 provides the shared reusable implementation;
  • archives the OpenSpec change and updates canonical specs, D-146, and the dependency-edged change sequence;
  • leaves the current selected-thread Gmail and approved-draft behavior unchanged.

TDD

Red

  • extended contract tests for missing scope, unresolved counterparties, weak or duplicate evidence, tamper detection, channel neutrality, and compatibility drift.

Green

  • implemented pure schemas, validators, catalog wiring, reviewed-scope comparison, evidence integrity, owner-review binding, and responsibility assessment until the new tests passed.

Verification

  • capability-map tests and generation;
  • cargo fmt --check;
  • cargo clippy --workspace --all-targets -- -D warnings;
  • shell build;
  • schema, shell, authority, and gate suites;
  • kernel workspace suite: 819/822 completed in the long local run before the runner ceiling; the three remaining tests passed individually;
  • file-size, claims, and OMP ceremony checks;
  • openspec validate --all --strict — 44/44;
  • git diff --check.

Closes #126.

@coderabbitai

coderabbitai Bot commented Aug 1, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in: 40 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 402649ad-70b1-4089-82bd-002504674ccd

📥 Commits

Reviewing files that changed from the base of the PR and between 054ba09 and 5e8233e.

📒 Files selected for processing (10)
  • crates/openspine-kernel/src/action_catalog.rs
  • crates/openspine-schemas/src/delegation_contract.rs
  • crates/openspine-schemas/src/owner_review.rs
  • crates/openspine-schemas/src/resolved_context.rs
  • crates/openspine-schemas/src/responsibility.rs
  • crates/openspine-schemas/src/reviewed_scope.rs
  • crates/openspine-schemas/tests/responsibility_contract.rs
  • crates/openspine-schemas/tests/responsibility_review_contract.rs
  • openspec/changes/archive/2026-08-02-define-responsibility-contract/specs/kernel-registries/spec.md
  • openspec/specs/kernel-registries/spec.md
📝 Walkthrough

Summary by CodeRabbit

  • New Features

    • Added reusable responsibility and delegation support with scoped actions, owner review, approval evidence, and lifecycle controls.
    • Added compatibility and drift checks requiring renewed review when action context changes or becomes unavailable.
    • Added support for reusable email draft actions with bounded policies and target-selection requirements.
  • Bug Fixes

    • Invalid, incomplete, tampered, or mismatched delegation requests now fail closed.
    • Communication and connector-write actions cannot default to approval during dark windows.
  • Documentation

    • Documented the responsibility contract, validation rules, and staged rollout sequence.

Walkthrough

Added protocol-neutral contracts for reusable delegation. The change adds fail-closed catalog validation, resolved action context, reviewed scopes, delegation evidence, owner review requests, responsibility manifests, drift assessment, tests, and OpenSpec records.

Changes

Responsibility contract

Layer / File(s) Summary
Delegation descriptors and catalog validation
crates/openspine-schemas/src/delegation_contract.rs, crates/openspine-schemas/src/action.rs, crates/openspine-kernel/src/action_catalog*.rs
Defines separate action and implementation descriptors. Catalog validation rejects missing, mismatched, or invalid delegation data.
Resolved context, reviewed scope, and responsibility assessment
crates/openspine-schemas/src/resolved_context.rs, crates/openspine-schemas/src/reviewed_scope.rs, crates/openspine-schemas/src/responsibility.rs
Constructs sealed context, derives scope dimensions, compares persisted scope, and reports compatibility drift.
Evidence and channel-neutral owner review
crates/openspine-schemas/src/delegation_evidence.rs, crates/openspine-schemas/src/owner_review.rs, crates/openspine-schemas/tests/*
Adds repeated-approval evidence, digest integrity, owner-review validation, lifecycle controls, and contract tests.
Architecture and specification records
.raw/openspine-decision-log.md, openspec/changes/archive/2026-08-02-define-responsibility-contract/*, openspec/specs/*, openspec/openspine-change-sequence.md
Documents the responsibility contract, fail-closed rules, runtime schemas, catalog requirements, completed tasks, and future execution sequence.

Estimated code review effort: 4 (Complex) | ~60 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Kernel
  participant ActionCatalog
  participant OwnerReviewRequest
  participant ResponsibilityManifest
  Kernel->>ActionCatalog: resolve and validate delegation descriptors
  ActionCatalog-->>Kernel: return validated action and implementation data
  Kernel->>OwnerReviewRequest: bind resolved context, scope, and evidence
  OwnerReviewRequest->>ResponsibilityManifest: create digest-bound review object
  ResponsibilityManifest-->>Kernel: report Compatible or NeedsReview
Loading

Possibly related issues

Possibly related PRs

Poem

A rabbit checks scopes by moonlight,
Digests keep each boundary tight.
Missing parts fail, approvals bind,
Drift sends old grants back to mind.
“Hop safely,” says Bunny, “review first.”


Caution

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

  • Ignore

❌ Failed checks (2 errors)

Check name Status Explanation Resolution
Docstring Coverage ❌ Error Docstring coverage is 25.68% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
All Comments Resolved ❌ Error Comments remain unresolved: tests call ReviewedActionScope::derive with one argument, call ResolvedActionContext::try_new with four, and expect nonexistent MissingCatalogEgressDeclaration. Update all affected tests to use the current derive and try_new signatures, and assert only declared ResolvedActionContextError variants.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the primary change: defining the protocol-neutral responsibility contract.
Description check ✅ Passed The description directly explains the responsibility contract, implementation scope, tests, verification, and preserved Gmail behavior.
Linked Issues check ✅ Passed The changes satisfy the linked issue's contract, fail-closed, evidence, scope, review, drift, documentation, testing, and compatibility objectives [#126].
Out of Scope Changes check ✅ Passed The code, tests, specifications, architecture records, and change-sequence updates all support the linked issue objectives.
📋 Issue Planner

Built with CodeRabbit's Coding Plans for faster development and fewer bugs.

View plan used: #126

✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch agent/responsibility-contract

Comment @coderabbitai help to get the list of available commands.

@George-RD
George-RD marked this pull request as ready for review August 2, 2026 23:46

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a259a03931

ℹ️ 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".

Comment on lines +163 to +168
if input.limits.quota.max == 0
|| input.limits.quota.window_secs <= 0
|| input.limits.rate.max == 0
|| input.limits.rate.window_secs <= 0
|| input.limits.expires_after_secs <= 0
{

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 Badge Enforce catalog bounds on reviewed limits

When a proposal supplies explicit or narrowed limits, this validation accepts every positive quota, rate, window, and expiry. validate_policy only checks catalog defaults, so a review can be successfully constructed and digest-bound with values above the action's DelegationPolicyBounds—for example, quota 21 for the email.create_draft maximum of 20 or an expiry beyond 90 days. Validate ReviewLimits against the selected action policy before returning a review request.

Useful? React with 👍 / 👎.

Comment on lines +32 to +40
impl ProposalProvenance {
pub fn from_evidence(evidence: &DelegationEvidence, summary: String) -> Self {
Self {
schema_version: 1,
kind: evidence.kind(),
summary,
evidence_digest: evidence.provenance_digest().clone(),
evidence_count: evidence.evidence_count(),
}

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 Badge Reject invalid evidence when creating provenance

When evidence has been rehydrated from storage, this constructor copies its claimed kind, count, and digest without calling integrity_is_valid(). A tampered repeated-approval record—for example, one whose approval_count was changed while retaining the old evidence-set digest—therefore produces provenance that OwnerReviewRequest::try_new accepts and binds as if it were valid. Make provenance construction fail for evidence that does not pass its integrity check.

Useful? React with 👍 / 👎.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 8

🧹 Nitpick comments (7)
openspec/openspine-change-sequence.md (1)

504-517: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win

Keep effect-executor failures on the post-approval path.

State that effect-executor is separate from ActionHandlerRegistry. Apply typed missing-executor failures to approved or delegated effect execution and readiness checks. Preserve the honest stub for known allowed actions without direct handlers.

🤖 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 `@openspec/openspine-change-sequence.md` around lines 504 - 517, Update
unify-approved-and-delegated-effect-execution to explicitly separate the
kernel-owned effect-executor registry from ActionHandlerRegistry. Apply typed
missing-executor failures only to post-approval approved/delegated effect
execution and its readiness checks, while preserving the honest successful stub
for known allowed actions that lack direct handlers.
crates/openspine-schemas/tests/responsibility_contract.rs (2)

180-187: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Assert the exact mismatch set.

The loop checks membership only. An implementation that reported every dimension as mismatched would still pass. The four changed inputs are connector_instance_id, account_identity_digest, target_refs[0].id, and workflow_id, so the expected set is exact.

🧪 Proposed assertion
-    for expected in [
-        ReviewedScopeDimension::ConnectorInstance,
-        ReviewedScopeDimension::AccountIdentity,
-        ReviewedScopeDimension::Target,
-        ReviewedScopeDimension::Workflow,
-    ] {
-        assert!(dimensions.contains(&expected));
-    }
+    assert_eq!(
+        dimensions,
+        BTreeSet::from([
+            ReviewedScopeDimension::ConnectorInstance,
+            ReviewedScopeDimension::AccountIdentity,
+            ReviewedScopeDimension::Target,
+            ReviewedScopeDimension::Workflow,
+        ])
+    );
🤖 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 `@crates/openspine-schemas/tests/responsibility_contract.rs` around lines 180 -
187, Update the assertions around the ReviewedScopeDimension loop to verify that
dimensions contains exactly the four expected values—ConnectorInstance,
AccountIdentity, Target, and Workflow—with no additional mismatches. Use an
exact set comparison or equivalent length-and-membership checks rather than
membership assertions alone.

190-207: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add coverage for validate_policy rejection branches.

The tests cover descriptor completeness, scope breadth, and dark-window allow. They do not reach validate_policy. DelegationEligibilityError::InvalidPolicyBounds and DelegationEligibilityError::DefaultOutOfBounds have no test, and validate_policy is the check that keeps catalog-authored quota, rate, and lapse values sane.

Useful cases: an inverted quota range, a zero policy_version, a default quota above maximum_max, a default expires_after_secs above maximum_lapse_secs, and ExplicitLimitsRequired combined with Some(defaults).

I can generate these tests if you want.

🤖 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 `@crates/openspine-schemas/tests/responsibility_contract.rs` around lines 190 -
207, Add tests exercising validate_policy rejection branches in the existing
responsibility-contract test module: cover inverted quota bounds, zero
policy_version, defaults exceeding maximum_max or maximum_lapse_secs, and
ExplicitLimitsRequired with Some(defaults). Assert each case returns the
appropriate DelegationEligibilityError variant, reusing the existing
policy/catalog builders and validation entry points.
crates/openspine-schemas/tests/responsibility_review_contract.rs (2)

251-263: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add coverage for the required-decision and required-control rejections.

The test supplies all four decisions and all four lifecycle controls, so try_new always takes the success path. OwnerReviewRequestError::MissingRequiredDecisions and OwnerReviewRequestError::MissingRequiredControls are never exercised.

These two checks guarantee that the owner always keeps Reject and Revoke available. Removing OwnerReviewDecision::Reject from available_decisions, and removing ResponsibilityLifecycleControl::Revoke from lifecycle_controls, would pin that property.

🤖 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 `@crates/openspine-schemas/tests/responsibility_review_contract.rs` around
lines 251 - 263, Extend the responsibility review contract tests around try_new
to cover missing required entries: create cases that omit
OwnerReviewDecision::Reject and assert
OwnerReviewRequestError::MissingRequiredDecisions, and omit
ResponsibilityLifecycleControl::Revoke and assert
OwnerReviewRequestError::MissingRequiredControls. Keep the existing complete-set
success case unchanged.

32-133: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Consider sharing the test fixtures.

digest, descriptor, implementation, and context_input are near-duplicates of crates/openspine-schemas/tests/responsibility_contract.rs lines 18-115. Only the version literals differ, and that divergence is deliberate so that assess can distinguish which version drifted.

A shared tests/common/mod.rs with version parameters would remove the duplication and keep the intentional differences explicit.

🤖 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 `@crates/openspine-schemas/tests/responsibility_review_contract.rs` around
lines 32 - 133, Share the duplicated test fixtures digest, descriptor,
implementation, and context_input through tests/common/mod.rs, parameterizing
the version literals needed by each contract test. Update
responsibility_review_contract.rs and responsibility_contract.rs to reuse these
helpers while keeping their deliberate version differences explicit so assess
can still detect which version drifted.
crates/openspine-schemas/src/owner_review.rs (1)

225-244: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Consider deriving the digest input from serialization.

The digest map lists all 16 public fields explicitly. The list is correct today. A future field added to OwnerReviewRequest but not to this map would silently drop out of the binding, and no compiler error or test would report it.

Building the digest input from serde_json::to_value(self) and removing the binding_digest key keeps the coverage automatic. The same pattern applies to ReviewedActionScope::calculate_context_class_digest, which already digests its whole field set.

🤖 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 `@crates/openspine-schemas/src/owner_review.rs` around lines 225 - 244, Update
OwnerReviewRequest::calculate_binding_digest to derive its digest input from
serde_json::to_value(self), remove the binding_digest field from the resulting
object, and digest the remaining serialized fields instead of maintaining an
explicit field list. Apply the same serialization-based approach to
ReviewedActionScope::calculate_context_class_digest, preserving its existing
digest behavior.
crates/openspine-kernel/src/action_catalog.rs (1)

97-97: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Consider a completeness test for delegation descriptor ids.

The comment at lines 80-83 states the catalog convention: an id added without its row must be a review-visible failure. Delegation descriptors have no equivalent assertion. If a future descriptor names an id that is absent from ids, validated_delegation_contract returns UnknownAction and delegation silently never becomes available for that action.

The behavior fails closed, so this is coverage, not a defect.

🧪 Proposed test
#[test]
fn every_delegation_descriptor_names_a_catalogued_action() {
    let catalog = canonical_catalog();
    for descriptor in action_catalog_data::delegation_descriptors() {
        assert!(
            catalog.contains(&descriptor.action_id),
            "delegation descriptor {} is not a catalogued action",
            descriptor.action_id
        );
    }
}
🤖 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 `@crates/openspine-kernel/src/action_catalog.rs` at line 97, Add a completeness
test alongside the existing catalog tests for `canonical_catalog` that iterates
over `action_catalog_data::delegation_descriptors()` and asserts each
descriptor’s `action_id` is present in the catalog, including the action id in
any failure message.
🤖 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 @.raw/openspine-decision-log.md:
- Line 3514: Update the date in the D-146 changelog entry to 2026-08-02 so it
matches the current date and archive directory convention; leave the rest of the
entry unchanged.

In `@crates/openspine-schemas/src/delegation_contract.rs`:
- Around line 84-94: Update DataDestination::is_communication_or_connector_write
to include Self::ExternalService in the matched destinations, ensuring
validate_delegation_contract applies the conservative reviewed-scope
requirements and rejects DarkWindowPolicy::BoundedAllow for external-service
destinations.
- Around line 295-311: The communication scope floor in the descriptor
eligibility check must require ReviewedScopeDimension::Counterparty. Add it to
the minimum dimensions used by is_communication_or_connector_write(), and add a
regression test covering a CounterpartyCommunication descriptor that omits
Counterparty and is rejected.

In `@crates/openspine-schemas/src/owner_review.rs`:
- Around line 163-170: Update OwnerReviewRequest::try_new to accept the
applicable DelegationPolicyBounds and validate quota and rate limits with
BudgetWindowBounds::contains, while rejecting expiry values above the policy
ceiling as well as existing non-positive values before constructing the review
request. Make BudgetWindowBounds::contains public so owner_review.rs can perform
this validation, and update the constructor signature for future callers.

In `@crates/openspine-schemas/src/resolved_context.rs`:
- Around line 108-113: Update try_new_with_classifications so egress_class,
disclosure_class, and output_channels are sourced from the ActionCatalog
declaration rather than trusted caller-provided ResolvedActionClassifications;
alternatively, explicitly restrict this constructor to kernel-only use and
enforce that callers provide catalog-derived values. Preserve
reviewed_scope::value_for behavior and the catalog-ownership rule for
enforcement.
- Around line 48-51: Remove the `Deserialize` derive from
`ResolvedActionContext` to preserve its sealed construction path through
`try_new`, keeping the existing `Serialize` support and validation guarantees
intact.

In `@crates/openspine-schemas/src/responsibility.rs`:
- Around line 79-89: Update ResponsibilityManifest::assess to evaluate the
manifest’s lifecycle status before returning compatibility; ensure Revoked,
Expired, and Paused statuses cannot produce
ResponsibilityAssessment::Compatible, while preserving the existing context,
scope, and version checks for active responsibilities.

In `@openspec/specs/kernel-registries/spec.md`:
- Around line 101-111: Scope the fail-closed catalog behavior to requests for
reusable-delegation readiness, preserving existing composition for
known-but-unimplemented actions and stub responses for missing direct handlers.
Apply this clarification in the requirement and scenario at
openspec/specs/kernel-registries/spec.md lines 101-111 and the corresponding
archived requirement at
openspec/changes/archive/2026-08-02-define-responsibility-contract/specs/kernel-registries/spec.md
lines 5-15.

---

Nitpick comments:
In `@crates/openspine-kernel/src/action_catalog.rs`:
- Line 97: Add a completeness test alongside the existing catalog tests for
`canonical_catalog` that iterates over
`action_catalog_data::delegation_descriptors()` and asserts each descriptor’s
`action_id` is present in the catalog, including the action id in any failure
message.

In `@crates/openspine-schemas/src/owner_review.rs`:
- Around line 225-244: Update OwnerReviewRequest::calculate_binding_digest to
derive its digest input from serde_json::to_value(self), remove the
binding_digest field from the resulting object, and digest the remaining
serialized fields instead of maintaining an explicit field list. Apply the same
serialization-based approach to
ReviewedActionScope::calculate_context_class_digest, preserving its existing
digest behavior.

In `@crates/openspine-schemas/tests/responsibility_contract.rs`:
- Around line 180-187: Update the assertions around the ReviewedScopeDimension
loop to verify that dimensions contains exactly the four expected
values—ConnectorInstance, AccountIdentity, Target, and Workflow—with no
additional mismatches. Use an exact set comparison or equivalent
length-and-membership checks rather than membership assertions alone.
- Around line 190-207: Add tests exercising validate_policy rejection branches
in the existing responsibility-contract test module: cover inverted quota
bounds, zero policy_version, defaults exceeding maximum_max or
maximum_lapse_secs, and ExplicitLimitsRequired with Some(defaults). Assert each
case returns the appropriate DelegationEligibilityError variant, reusing the
existing policy/catalog builders and validation entry points.

In `@crates/openspine-schemas/tests/responsibility_review_contract.rs`:
- Around line 251-263: Extend the responsibility review contract tests around
try_new to cover missing required entries: create cases that omit
OwnerReviewDecision::Reject and assert
OwnerReviewRequestError::MissingRequiredDecisions, and omit
ResponsibilityLifecycleControl::Revoke and assert
OwnerReviewRequestError::MissingRequiredControls. Keep the existing complete-set
success case unchanged.
- Around line 32-133: Share the duplicated test fixtures digest, descriptor,
implementation, and context_input through tests/common/mod.rs, parameterizing
the version literals needed by each contract test. Update
responsibility_review_contract.rs and responsibility_contract.rs to reuse these
helpers while keeping their deliberate version differences explicit so assess
can still detect which version drifted.

In `@openspec/openspine-change-sequence.md`:
- Around line 504-517: Update unify-approved-and-delegated-effect-execution to
explicitly separate the kernel-owned effect-executor registry from
ActionHandlerRegistry. Apply typed missing-executor failures only to
post-approval approved/delegated effect execution and its readiness checks,
while preserving the honest successful stub for known allowed actions that lack
direct handlers.
🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 61ef53e5-8e87-4037-8e5c-ce94d950f5ed

📥 Commits

Reviewing files that changed from the base of the PR and between 8a6ef59 and a259a03.

📒 Files selected for processing (23)
  • .raw/openspine-decision-log.md
  • crates/openspine-kernel/src/action_catalog.rs
  • crates/openspine-kernel/src/action_catalog_data.rs
  • crates/openspine-schemas/src/action.rs
  • crates/openspine-schemas/src/delegation_contract.rs
  • crates/openspine-schemas/src/delegation_evidence.rs
  • crates/openspine-schemas/src/lib.rs
  • crates/openspine-schemas/src/owner_review.rs
  • crates/openspine-schemas/src/resolved_context.rs
  • crates/openspine-schemas/src/responsibility.rs
  • crates/openspine-schemas/src/reviewed_scope.rs
  • crates/openspine-schemas/tests/responsibility_contract.rs
  • crates/openspine-schemas/tests/responsibility_review_contract.rs
  • openspec/changes/archive/2026-08-02-define-responsibility-contract/design.md
  • openspec/changes/archive/2026-08-02-define-responsibility-contract/proposal.md
  • openspec/changes/archive/2026-08-02-define-responsibility-contract/specs/core-runtime-schemas/spec.md
  • openspec/changes/archive/2026-08-02-define-responsibility-contract/specs/kernel-registries/spec.md
  • openspec/changes/archive/2026-08-02-define-responsibility-contract/specs/responsibility-contract/spec.md
  • openspec/changes/archive/2026-08-02-define-responsibility-contract/tasks.md
  • openspec/openspine-change-sequence.md
  • openspec/specs/core-runtime-schemas/spec.md
  • openspec/specs/kernel-registries/spec.md
  • openspec/specs/responsibility-contract/spec.md

| 2026-07-24 | Added D-123 (production adoption at ambiguous route resolution), D-124 (canonical tied candidate ids), D-125 (lexicographic within-class selection), D-126 (invalid/failed competitors escalate), D-127 (all-non-applicable ties are silent non-matches), D-128 (rated-egress production guard), and D-129 (persist the selected composition snapshot), settled while implementing `wire-authority-equivalence-selection`. |
| 2026-07-24 | Added D-130–D-141 (pure proposed-only miner boundary, authenticated bounded grants, verified encrypted-reference evidence, derived exact-match repetition, correction/probe separation, fail-closed provenance, independent durable budgets, scoped consolidation, declarative scheduled grant composition, and owner-bound cross-grant evidence), settled while implementing `implement-reflection-miner`. |
| 2026-07-24 | Added D-142 (native OAuth 2.0 PKCE authentication), D-143 (encrypted vault storage in SecretStore), D-144 (preemptive single-flight background token refresher), and D-145 (gateway bearer token resolution with automatic single-retry on 401), settled while implementing `implement-model-provider-oauth-onboarding`. |
| 2026-08-03 | Added D-146 (protocol-neutral two-axis responsibility contract; responsibility remains a reference view over workflow/standing-rule inputs; communication dark-window Allow forbidden; drift fails to `needs_review`), settled while implementing `define-responsibility-contract`. |

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Use a consistent change-log date.

Line 3514 records 2026-08-03, but the current date is August 2, 2026 and the archive directory is dated 2026-08-02. Change the entry to 2026-08-02.

Proposed fix
-| 2026-08-03 | Added D-146 ...
+| 2026-08-02 | Added D-146 ...
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
| 2026-08-03 | Added D-146 (protocol-neutral two-axis responsibility contract; responsibility remains a reference view over workflow/standing-rule inputs; communication dark-window Allow forbidden; drift fails to `needs_review`), settled while implementing `define-responsibility-contract`. |
| 2026-08-02 | Added D-146 (protocol-neutral two-axis responsibility contract; responsibility remains a reference view over workflow/standing-rule inputs; communication dark-window Allow forbidden; drift fails to `needs_review`), settled while implementing `define-responsibility-contract`. |
🤖 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 @.raw/openspine-decision-log.md at line 3514, Update the date in the D-146
changelog entry to 2026-08-02 so it matches the current date and archive
directory convention; leave the rest of the entry unchanged.

Comment thread crates/openspine-schemas/src/delegation_contract.rs
Comment thread crates/openspine-schemas/src/delegation_contract.rs
Comment thread crates/openspine-schemas/src/owner_review.rs
Comment thread crates/openspine-schemas/src/resolved_context.rs Outdated
Comment thread crates/openspine-schemas/src/resolved_context.rs Outdated
Comment thread crates/openspine-schemas/src/responsibility.rs
Comment thread openspec/specs/kernel-registries/spec.md

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 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 `@crates/openspine-schemas/tests/responsibility_contract.rs`:
- Line 275: Update each ReviewedActionScope::derive call in the affected test
cases to pass both required arguments, descriptor and context, in the order
required by the method signature. Apply this at the calls around lines 275, 310,
and 368 while preserving the existing test setup and assertions.
- Around line 319-338: Update
resolved_context_fails_closed_without_catalog_classifications to match the
current ResolvedActionContext::try_new signature by passing only its three
accepted arguments, and assert the corresponding existing
ResolvedActionContextError variant for missing catalog classifications instead
of the undefined MissingCatalogEgressDeclaration variant.
🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 6c4cff41-34b7-47dd-9fbf-2548b61e6500

📥 Commits

Reviewing files that changed from the base of the PR and between a259a03 and 054ba09.

📒 Files selected for processing (1)
  • crates/openspine-schemas/tests/responsibility_contract.rs

Comment thread crates/openspine-schemas/tests/responsibility_contract.rs
Comment thread crates/openspine-schemas/tests/responsibility_contract.rs

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5e8233e8b2

ℹ️ 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".

Comment on lines +189 to +191
let provenance =
ProposalProvenance::try_from_evidence(&input.evidence, input.provenance_summary)
.map_err(|_| OwnerReviewRequestError::InvalidEvidence)?;

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 Badge Bind repeated evidence to the reviewed scope

When valid repeated-approval evidence is supplied with a context_class_digest different from input.reviewed_scope.context_class_digest(), this constructor still accepts and digest-binds the review. This lets approvals observed for one context be presented as provenance for a different connector, account, target, or workflow scope; explicitly compare these digests for RepeatedApprovals before constructing the review.

Useful? React with 👍 / 👎.

Comment on lines +47 to +52
Ok(Self {
schema_version: 1,
kind: evidence.kind(),
summary,
evidence_digest: evidence.provenance_digest().clone(),
evidence_count: evidence.evidence_count(),

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 Badge Prevent non-pattern evidence from claiming a pattern

When the evidence is an explicit owner request, correction proposal, or manual artifact, callers can still set summary to text such as “Lyra noticed a pattern,” and this semantic owner-review object accepts and binds that misleading claim. Although supports_pattern_claim() exists, it is never enforced here, so pattern provenance needs a structured flag or validation that only permits it for repeated approvals.

Useful? React with 👍 / 👎.

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.

Define the protocol-neutral responsibility contract above standing rules

1 participant