Add three-tier thinking room posture contract - #542
Merged
Conversation
Contributor
|
Warning Review limit reached
More reviews will be available in 17 minutes and 23 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more credits in the billing tab to continue. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (9)
📝 WalkthroughWalkthroughMigrates posture handling from ChangesCausalPosture core and authority modeling
Braid shell posture threading
Settlement and public API
Design and enforcement
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Owner
Author
|
@codex review please |
![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: f40e8047de
ℹ️ 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".
Owner
Author
|
@codex please confirm or challenge this self-review. Self-Code Review Findings
Count
Style witnesses run during review:
All three passed; findings above are semantic/structural. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f40e8047de
ℹ️ 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".
Owner
Author
|
Resolved the seven self-review findings in Resolution map:
Local validation after the fix:
Pre-push also passed exact Rust slices and Prettier on the pushed tree. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
scripts/tests/causal_posture_constructor_lint_test.sh (1)
121-127: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winTest coverage: add false-positive verification.
The tests verify that violations are correctly rejected, but don't verify that valid constructor usage (e.g.,
RetentionPosture::new(...)andSessionContext::new(...)) is NOT flagged. Add positive test cases to prevent false positives.🧪 Suggested additional test fixtures
+echo "" +echo "3. Valid constructor usage is not rejected" +valid_retention_fixture="${tmpdir}/valid-retention.rs" +cat >"${valid_retention_fixture}" <<'RS' +fn valid_fixture() { + let _posture = RetentionPosture::new( + causal_posture, + posture_derivation, + authority, + retention_contract, + admission_scope, + ); +} +RS + +assert "RetentionPosture::new is allowed" \ + env CAUSAL_POSTURE_LINT_PATHS="${valid_retention_fixture}" "${guard}" + +valid_session_fixture="${tmpdir}/valid-session.rs" +cat >"${valid_session_fixture}" <<'RS' +fn valid_fixture() { + let _session = SessionContext::new( + session_id, + origin_id, + actor_id, + author_domain, + authority_binding, + seal_strength, + default_posture, + default_admission_scope, + retention_contract, + ); +} +RS + +assert "SessionContext::new is allowed" \ + env CAUSAL_POSTURE_LINT_PATHS="${valid_session_fixture}" "${guard}"🤖 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 `@scripts/tests/causal_posture_constructor_lint_test.sh` around lines 121 - 127, Current tests only assert that bad constructor usages are flagged; add positive test cases that include valid constructor calls like RetentionPosture::new(...) and SessionContext::new(...) so the linter does NOT report them. Update the test fixtures used by scripts/tests/causal_posture_constructor_lint_test.sh to include files with correct usages of RetentionPosture::new and SessionContext::new, ensure the test runner treats those files as expected-passing cases (incrementing the passed count and not the failed count), and run the existing comparison logic so false positives are caught in future changes.crates/warp-core/src/settlement.rs (1)
273-307:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftPersist plural posture in the retained record, not just the draft/ABI.
PluralAlternativeDraftnow carriespostureandto_abi()exports it, butappend_plural_artifact()still records onlyProvenanceEventKind::PluralArtifact { plural_id }. After settlement, the retained provenance entry no longer contains the posture, so replay/query code cannot reconstruct it from stored data. Persist the posture in the retained artifact schema or bind it into the retained record/digest.🤖 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/warp-core/src/settlement.rs` around lines 273 - 307, Plural posture is exported by PluralAlternativeDraft::to_abi but not persisted: update the retained provenance record so the posture is stored (rather than only recording ProvenanceEventKind::PluralArtifact { plural_id }). Modify the ProvenanceEventKind::PluralArtifact variant (or the retained-artifact schema) to include a posture field, update append_plural_artifact() to write that posture from the PluralAlternativeDraft when creating the retained entry, and adjust any serialization/digest functions (and ABI conversion paths) that construct or validate retained plural records so the persisted entry and its digest include the posture.
🤖 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/warp-core/src/revelation.rs`:
- Around line 773-808: ImportAdmissionReceipt can be replayed across different
imports because it lacks an import-specific identifier; add a field (e.g.,
import_id: ImportId or ImportedArtifactId) to the ImportAdmissionReceipt struct,
update ImportAdmissionReceipt::new to accept and store that identifier (and keep
existing authority_proof.authorizes_new_admission call), change
import_posture_disposition() to require receipt.import_id == current_import_id
(rather than merely Some(receipt)) before returning LocallyAdmittedShared, and
add a regression test asserting a receipt minted for import A does not admit
import B; apply the same field/check where ImportAdmissionReceipt is
constructed/consumed in the other affected sites.
- Around line 1390-1413: The test currently checks only
AuthorityResolutionProof::LocalAuthorityDomain but misses the nested case where
AuthorityResolutionProof::LocalCapability(CapabilityProof::LocalAuthorityDomain(...))
names a different authority; add a test (or extend
promotion_rejects_mismatched_authority_proof) that calls
PromotionIntent::admit_shared with
AuthorityResolutionProof::LocalCapability(CapabilityProof::LocalAuthorityDomain(...))
using an AuthorityDomainRef/AuthorityDomainId that differs from
fixture_authority_ref(), and assert it returns
Err(PostureObstruction::AuthorityProofMismatch { authorized_by:
fixture_authority_ref() }) so the nested mismatch path in the admit_shared
authority check is covered (use the same IntentId/ActorId/strand/posture/etc. as
the existing test to mirror the scenario).
In `@docs/design/0027-three-tier-thinking-room/design.md`:
- Around line 75-102: Current Truth in design 0027 has a stale anchor for Design
0026: update the anchor reference so the 0027 "Current Truth" paragraph points
to the actual section in Design 0026 that contains the "echo#538 / E0-lite lands
as E1’s first commit" text (rather than the blank line currently linked); leave
the other snapshots and claims unchanged (RevelationPosture,
promote_posture/WitnessDigest, Strand, ForkBasisRef, and Session v1 observations
are correct).
In `@scripts/check-causal-posture-constructors.sh`:
- Around line 59-62: Update the regex passed to check_rg so it also treats '[' ,
'=>' and 'return' as valid delimiters before struct-literal occurrences of
RetentionPosture and SessionContext; in other words, extend the leading-token
group (currently (^|[=(:,]\s*)) to include an optional '\[' token, a '=>' token,
and 'return' followed by whitespace so the pattern still matches
'([A-Za-z0-9_]+::)*(RetentionPosture|SessionContext)\s*\{' but will also catch
cases like '[ ... {', '... => {', and 'return ... {' as an extra hardening; keep
the rest of the pattern and flags unchanged.
---
Outside diff comments:
In `@crates/warp-core/src/settlement.rs`:
- Around line 273-307: Plural posture is exported by
PluralAlternativeDraft::to_abi but not persisted: update the retained provenance
record so the posture is stored (rather than only recording
ProvenanceEventKind::PluralArtifact { plural_id }). Modify the
ProvenanceEventKind::PluralArtifact variant (or the retained-artifact schema) to
include a posture field, update append_plural_artifact() to write that posture
from the PluralAlternativeDraft when creating the retained entry, and adjust any
serialization/digest functions (and ABI conversion paths) that construct or
validate retained plural records so the persisted entry and its digest include
the posture.
In `@scripts/tests/causal_posture_constructor_lint_test.sh`:
- Around line 121-127: Current tests only assert that bad constructor usages are
flagged; add positive test cases that include valid constructor calls like
RetentionPosture::new(...) and SessionContext::new(...) so the linter does NOT
report them. Update the test fixtures used by
scripts/tests/causal_posture_constructor_lint_test.sh to include files with
correct usages of RetentionPosture::new and SessionContext::new, ensure the test
runner treats those files as expected-passing cases (incrementing the passed
count and not the failed count), and run the existing comparison logic so false
positives are caught in future changes.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: e509e18f-3a23-4469-9d22-b4136c407679
📒 Files selected for processing (9)
crates/warp-core/src/braid_shell.rscrates/warp-core/src/lib.rscrates/warp-core/src/provenance_store.rscrates/warp-core/src/revelation.rscrates/warp-core/src/settlement.rsdocs/design/0027-three-tier-thinking-room/design.mdscripts/check-causal-posture-constructors.shscripts/tests/causal_posture_constructor_lint_test.shscripts/verify-local.sh
Owner
Author
|
@codex Activity Summary for the Code Lawyer repair pass.
Resolved review threads via GraphQL after push: 9. Active unresolved review threads after resolution: 0. Validation:
Note: an exploratory |
Summary
Testing / Checks
|
This was referenced Jun 14, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Refs #538.
This PR lands PLATFORM-0027 and the first implementation tranche for Echo's three-tier thinking room model. It makes causal/revelation posture explicit instead of treating retained work as implicitly shared.
Changes:
docs/design/0027-three-tier-thinking-room/design.mdwith the amended posture, authority, admission-scope, promotion, legacy-derivation, import, and materialization requirements.CausalPosturewith a deprecatedRevelationPosturecompatibility alias.RetentionPostureandSessionContextconstructors soSharedrequires anAdmissionScopeIdand non-shared posture cannot carry one.verify-localpattern guards.Validation
Passed locally:
cargo fmt --all -- --checkgit diff --checkscripts/check-no-app-nouns-in-core.shcargo check -p warp-corecargo test -p warp-core --lib revelation::testscargo test -p warp-core --libcargo clippy -p warp-core --lib -- -D warningsscripts/check-causal-posture-constructors.shbash scripts/tests/causal_posture_constructor_lint_test.shbash -n scripts/check-causal-posture-constructors.sh scripts/tests/causal_posture_constructor_lint_test.sh scripts/verify-local.shscripts/check_spdx.sh scripts/check-causal-posture-constructors.sh scripts/tests/causal_posture_constructor_lint_test.sh scripts/verify-local.shscripts/verify-local.sh ultra-fastscripts/verify-local.sh fullKnown unrelated local baseline:
cargo clippy -p warp-core --all-targets -- -D warningsis blocked by existing test-target lints, tracked as warp-core all-targets clippy is blocked by existing test-target lints #541.VERIFY_LOCAL_HOOK_TESTS=1 scripts/verify-local.sh fullis blocked locally by unrelated hook-test/tooling baselines and missing local Node packages under Node v26; normalscripts/verify-local.sh fullpasses.Summary by CodeRabbit
Release Notes
New Features
Refactor