Harden strand construction and typed braid failures

[flyingrobots]

Summary

• implement Goalpost 1 of the braids/strands hardening roadmap
• move Strand<P> public struct-literal construction behind Strand::new(...) plus read-only accessors
• validate retained posture and typestate/runtime posture coherence before constructing typed strands
• replace proof validation strings with structured ProofError
• replace braid invalid-transition action strings with BraidTransitionKind
• thread typed proof errors through BraidShellError::ProofShapeValidationFailed
• update public API tests, strand fixture builders, changelog, and the roadmap checklist

Why

PR #545 made strands and braids law-bearing surfaces, but Strand<P> still looked forgeable through public fields and proof/braid failures still exposed behavioral facts through strings. This PR closes the first hardening goalpost: law-bearing strand values are admitted through named construction, and callers can branch on typed failure variants without parsing display text.

Runtime and registry checks remain authoritative. Typestate is only the guardrail.

Red / Green witnesses

RED before implementation:

• cargo test -p warp-core --test braid_public_api_tests
• cargo test -p warp-core --test strand_contract_tests strand_constructor_rejects_static_runtime_posture_mismatch

GREEN after implementation:

• cargo test -p warp-core --lib — 560/560
• cargo test -p warp-core --test braid_public_api_tests — 3/3
• cargo test -p warp-core --test strand_contract_tests — 29/29
• cargo test -p warp-core --doc — includes the new public Strand compile-fail witness
• cargo clippy -p warp-core --lib -- -D warnings
• cargo fmt --all -- --check
• git diff --check
• scripts/check_spdx.sh
• pnpm exec markdownlint-cli2 CHANGELOG.md docs/design/braids-and-strands-roadmap.md docs/design/braids-and-strands-hardening/goalpost-01-lawful-construction-and-typed-failures.md
• pre-commit and pre-push hooks

Summary by CodeRabbit

• New Features

  • Structured error reporting for invalid state transitions with specific transition types and current status details.
  • Typed proof validation errors replacing generic error messages with specific failure categories.

• Improvements

  • Strengthened data structure construction with enhanced invariant validation and better error reporting for violations.
  • Single-writer-head invariant enforcement across data structure initialization and registration.

• Documentation

  • Completed roadmap milestone for lawful construction and typed failures.

[coderabbitai]

Warning

Review limit reached

@flyingrobots, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 22 minutes and 31 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 @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: be009c85-fff8-484d-b522-f3263771edac

📥 Commits

Reviewing files that changed from the base of the PR and between 26ef1b7 and 33f8894.

📒 Files selected for processing (3)

• crates/echo-wesley-gen/tests/generation.rs
• crates/warp-core/src/strand.rs
• crates/warp-core/tests/strand_contract_tests.rs

📝 Walkthrough

Walkthrough

Implements GP1 of the braids-and-strands hardening roadmap: Strand<P> fields are restricted to pub(crate), a posture-aware validate_local_strand_invariants function is introduced, and a Strand::new constructor enforces all local invariants. ProofEnvelope::validate_shape switches from String errors to a structured ProofError enum; BraidError::InvalidTransition replaces action: String with a typed BraidTransitionKind. All call sites, tests, and design docs are updated accordingly.

Changes

GP1: Lawful Construction and Typed Failures

Layer / File(s)	Summary
Typed proof error contracts crates/warp-core/src/proof.rs, crates/warp-core/src/lib.rs, crates/warp-core/src/braid_shell.rs	Adds VerificationFailureCode and ProofError enums via thiserror; ProofEnvelope::validate_shape returns Result<(), ProofError>; BraidShellError::ProofShapeValidationFailed carries ProofError instead of String; both types re-exported from lib.rs.
Typed BraidTransitionKind and InvalidTransition update crates/warp-core/src/braid.rs	Adds BraidTransitionKind enum with Display; BraidError::InvalidTransition switches from action: String to transition: BraidTransitionKind; all three Braid::apply error sites updated.
Strand field encapsulation and invariant-enforcing constructor crates/warp-core/src/strand.rs	Strand<P> fields changed to pub(crate); validate_local_strand_invariants::<P>() added to enforce posture coherence, worldline distinctness, v1 writer-head cardinality/ownership, and support-pin constraints; pub fn new(...) constructor and getter accessors exposed; StrandRegistry::insert delegates to shared validator.
Coordinator wired to Strand::new crates/warp-core/src/coordinator.rs	WorldlineRuntime::fork_strand replaces struct literal construction with Strand::new(...); new regression test asserts empty writer_heads is rejected and all partial state is rolled back.
Braid internal + public API tests crates/warp-core/src/braid.rs, crates/warp-core/tests/braid_public_api_tests.rs	Three internal unit tests updated to match BraidTransitionKind fields; new public API tests assert typed InvalidTransition errors, human-readable display strings, and typed ProofError variants from validate_shape.
BraidShell proof validation tests crates/warp-core/src/braid_shell.rs	Wildcard proof-error matches replaced with structured ProofError::PublicInputsMismatch, ProofError::EmptyPayload, and ProofError::UnsupportedKind assertions.
Strand contract tests refactored to constructor/accessor API crates/warp-core/tests/strand_contract_tests.rs	Removes make_test_strand_raw, introduces make_test_strand_with_parts using Strand::new; all field accesses replaced with accessor calls; new constructor rejection tests added for INV-S7, INV-S8, and Shared posture without admission scope.
Design docs and changelog CHANGELOG.md, docs/design/braids-and-strands-roadmap.md, docs/design/braids-and-strands-hardening/goalpost-01-lawful-construction-and-typed-failures.md	Goalpost 1 status set to "implemented"; GP1-S1 through GP1-S5 checked off in the roadmap tracker; two CHANGELOG entries added.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

• flyingrobots/echo#315: Introduced the original Strand module and StrandRegistry/Strand APIs that this PR directly tightens by encapsulating fields and enforcing construction invariants.
• flyingrobots/echo#545: Overlaps directly with the typed braid lifecycle/proof error refactoring and posture-validated strand construction that this PR completes.

Poem

🔒 No more forging strands by hand,
the fields are locked, the law has land.
BraidTransitionKind says what went wrong,
ProofError typed, the contract's strong.
GP1: checked — the invariants hold,
Strand::new guards every fold. 🧵

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title directly and accurately describes the main objective of the PR: hardening strand construction and implementing typed braid failures, which aligns with the core changes across strand.rs, braid.rs, and proof.rs.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch hardening/lawful-construction-typed-failures

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[flyingrobots]

Self-Code Review: Code Lawyer Pass

@codex Please confirm or challenge these findings. I reviewed PR #548 against origin/main after a clean worktree gate and git fetch origin.

Severity	File / lines	Infraction	Evidence	Recommended mitigation prompt
P1	crates/warp-core/src/strand.rs:167, crates/warp-core/src/strand.rs:192, crates/warp-core/src/strand.rs:214, crates/warp-core/src/strand.rs:234, crates/warp-core/src/strand.rs:807, crates/warp-core/src/strand.rs:821, crates/warp-core/src/coordinator.rs:1610	The new law-bearing constructor boundary still admits strands with zero or multiple writer heads, and the production fork path still builds a Strand through a crate-internal struct literal instead of the constructor.	The field docs state writer_heads has cardinality 1 in v1, but Strand::new(...) only verifies each head belongs to child_worldline_id; it never checks writer_heads.len() == 1. StrandRegistry::insert(...) repeats the ownership check but also does not check cardinality. WorldlineRuntime::fork_strand(...) constructs Strand { ... } directly and relies on registry insertion, so any constructor-only invariant added later can be bypassed by production code.	Fix the strand construction boundary so v1 writer-head cardinality is enforced through one shared local-invariant validator. Add failing tests showing Strand::new(...)rejects zero writer heads and two writer heads. HaveStrand::new(...)andStrandRegistry::insert(...)both call the same validator, and replace the productionfork_strandstruct literal withStrand::new(...) so production construction goes through the named door.
P4	crates/warp-core/src/braid.rs:57	BraidError display text regressed from human action phrases to Rust debug variant names.	The roadmap says typed transition failures should preserve stable display text for humans while exposing typed variants to callers. The old display produced phrases like cannot weave member; the new #[error("cannot transition braid state: cannot {transition:?} ...")] renders cannot WeaveMember, which is typed but not human-facing.	Implement DisplayforBraidTransitionKindusing human action phrases such ascreate braid, weave member, finalize settlement, and collapse braid, then change the error format to {transition}. Add a regression asserting the display string remains human-readable while behavior tests continue matching BraidTransitionKind variants.

Summary: the PR is directionally correct and all local/remote checks are green, but Goalpost 1 should not be considered complete until the strand constructor boundary closes the v1 writer-head cardinality gap and production strand creation uses the named constructor path.

[flyingrobots]

Follow-up on the self-review findings. @codex please sanity-check the closure.

Finding	Severity	Resolution	Commit
Strand<P> constructor/registry did not enforce the v1 single-writer-head invariant, and runtime fork bypassed Strand::new(...).	P1	Added shared local strand invariant validation, enforced exactly one writer head in both Strand::new(...) and StrandRegistry::insert(...), routed WorldlineRuntime::fork_strand(...) through Strand::new(...), and added constructor plus runtime rollback regressions.	14226725
BraidTransitionKind leaked enum debug text in BraidError::InvalidTransition display output.	P4	Added human-facing Display for BraidTransitionKind, switched the error text to {transition}, and added public API display regression coverage.	26ef1b7f

Validation:

• RED witnesses failed before implementation: missing writer head, multiple writer heads, empty-head runtime fork rollback, and braid transition display.
• GREEN focused witnesses passed after implementation.
• cargo fmt --all -- --check
• cargo test -p warp-core --test strand_contract_tests (31/31)
• cargo test -p warp-core --test braid_public_api_tests (4/4)
• cargo test -p warp-core --lib (561/561)
• cargo test -p warp-core --doc (23 passed / 6 ignored)
• cargo clippy -p warp-core --lib -- -D warnings
• git diff --check
• scripts/check_spdx.sh
• pnpm exec markdownlint-cli2 CHANGELOG.md
• pre-push exact Rust slices passed on push.

[flyingrobots]

Additional CI follow-up on head eb1f2c68:

• The first remote rerun exposed a CI-sensitive harness race in echo-wesley-gen generation tests: --no-std --contract-host is expected to fail before reading stdin, so the parent can observe BrokenPipe while writing the fixture IR.
• Fixed the test harness to tolerate ErrorKind::BrokenPipe for early CLI exits and still assert on the child status/stderr. Commit: eb1f2c68.
• Local validation: cargo test -p echo-wesley-gen --test generation test_contract_host_generation_rejects_no_std -- --exact --nocapture and full cargo test -p echo-wesley-gen --test generation both pass; pre-push reran the full generation suite successfully.

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. Hooray!

Reviewed commit: 26ef1b7f7d

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

[coderabbitai]

Actionable comments posted: 3

🤖 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/strand.rs`:
- Around line 214-225: The self-support and duplicate-target checks are being
performed twice: once in the inline validation code (around lines 214-225) and
again in the validate_support_pins function (lines 1020-1042). Since
StrandRegistry::insert calls both validators in sequence, this creates
unnecessary duplication and maintenance burden. Remove the self-support and
duplicate-target validation checks from the validate_support_pins function,
keeping only the registry-relative checks that require live strand context (such
as target liveness and worldline matching validation). This leaves the simpler
checks to the inline validator that runs first in the insert flow.

In `@crates/warp-core/tests/strand_contract_tests.rs`:
- Around line 681-684: The assertions currently use a broad pattern match that
accepts any StrandError::InvariantViolation without validating which specific
invariant was violated. Replace the generic InvariantViolation pattern match
with explicit checks for the invariant error messages or codes (INV-S7 and
INV-S8 respectively) to ensure that only the expected invariant violations cause
the tests to pass, preventing regression failures from being masked by
violations of different invariants. This applies to both the assertion at lines
681-684 and the related assertion at lines 701-704 in the same test file.

In
`@docs/design/braids-and-strands-hardening/goalpost-01-lawful-construction-and-typed-failures.md`:
- Line 6: The three documentation files prematurely claim GP1 is complete
despite two unresolved blocking issues (P1 incomplete strand constructor
boundary validation, P4 BraidError display regression). In
docs/design/braids-and-strands-hardening/goalpost-01-lawful-construction-and-typed-failures.md
at line 6, change the status from "implemented" to "in progress" or "blocked on
P1/P4" to reflect the incomplete work. In
docs/design/braids-and-strands-roadmap.md at lines 64-71, uncheck GP1-S1 (since
the public forgery prevention acceptance criterion is not satisfied) and add a
note referencing the two outstanding issues from the self-code review. In
CHANGELOG.md at lines 10-19, either move the GP1 completion claims to a
follow-up section scheduled after the P1/P4 fixes land, or reword them to
accurately describe what is currently implemented and explicitly flag what
remains incomplete (e.g., note that cardinality enforcement is incomplete).

🪄 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: eac0377b-7ef2-44c8-af14-0a24690fc662

📥 Commits

Reviewing files that changed from the base of the PR and between 3d6fdba and 26ef1b7.

📒 Files selected for processing (11)

• CHANGELOG.md
• crates/warp-core/src/braid.rs
• crates/warp-core/src/braid_shell.rs
• crates/warp-core/src/coordinator.rs
• crates/warp-core/src/lib.rs
• crates/warp-core/src/proof.rs
• crates/warp-core/src/strand.rs
• crates/warp-core/tests/braid_public_api_tests.rs
• crates/warp-core/tests/strand_contract_tests.rs
• docs/design/braids-and-strands-hardening/goalpost-01-lawful-construction-and-typed-failures.md
• docs/design/braids-and-strands-roadmap.md

[flyingrobots]

@codex self-audit found one new documentation precision issue while reviewing the current PR head.

ID	Severity	Source	File	Lines	Issue	Mitigation
SELF-01	P5	Self	crates/warp-core/src/strand.rs	constructor docs around Strand::new(...) errors	The public constructor now rejects missing/multiple v1 writer heads, but the # Errors docs only mention wrong-worldline heads and omit cardinality.	Update the constructor error docs to state the v1 exactly-one-writer-head invariant explicitly.

[chatgpt-codex-connector]

Summary

• Clarified the public Strand::new(...) # Errors documentation so it explicitly states that v1 strands must contain exactly one writer head, matching the constructor’s enforced invariant. crates/warp-core/src/strand.rsL239-L245
• Committed the change on the current branch with commit a0745ff (docs(warp-core): clarify strand writer-head constructor errors).
• Created a PR titled “Clarify strand constructor writer-head error docs”.

Testing

• ✅ cargo test -p warp-core --doc
• ✅ cargo check -p warp-core
• ✅ git diff --check

Note: pnpm-lock.yaml had pre-existing unstaged local changes and was not touched or included in the commit.

View task →

[flyingrobots]

Activity Summary - Code Lawyer Pass

@codex Current-head closure pass for PR #548.

#	Source	Severity	File	Issue	Commit	Outcome
1	PR	P4	crates/warp-core/tests/strand_contract_tests.rs	Constructor rejection tests matched broad InvariantViolation(_) instead of the exact invariant codes.	77069682	Fixed: tests now assert INV-S7 and INV-S8 message payloads exactly.
2	PR	P5	crates/warp-core/src/strand.rs	Registry support-pin validation duplicated self-target and duplicate-target checks already owned by local strand invariant validation.	30792554	Fixed: registry validation now keeps only registry-relative checks for missing targets and worldline mismatch.
3	Self	P5	crates/warp-core/src/strand.rs	Strand::new docs omitted the v1 single-writer-head cardinality rejection.	33f8894d	Fixed: constructor error docs now name the exact writer-head cardinality precondition.
4	PR	P4	docs/design/braids-and-strands-hardening/goalpost-01-lawful-construction-and-typed-failures.md	Completion-status thread was stale after current-head fixes landed.	14226725, 26ef1b7f	Resolved: roadmap, goalpost status, and changelog now match the implemented/fixed state.

Review-thread state after GraphQL resolution: 0 unresolved, non-outdated threads.

Local verification passed:

• cargo fmt --all -- --check
• cargo test -p warp-core --test strand_contract_tests
• cargo test -p warp-core --test braid_public_api_tests
• cargo test -p warp-core --lib
• cargo test -p warp-core --doc
• cargo test -p echo-wesley-gen --test generation
• cargo test --workspace --exclude warp-core
• cargo clippy -p warp-core --lib -- -D warnings
• cargo clippy -p echo-wesley-gen --lib --tests -- -D warnings
• git diff --check
• scripts/check_spdx.sh

Pre-push verification also passed on the pushed head 33f8894d.

[chatgpt-codex-connector]

Summary

• Confirmed the current head already contains the closure fixes described in the trigger: Strand::new(...) documents the v1 exactly-one-writer-head rejection, including missing or multiple writer heads. crates/warp-core/src/strand.rsL239-L245
• Confirmed local strand validation enforces the v1 writer-head cardinality invariant through the shared validator. crates/warp-core/src/strand.rsL177-L205
• Confirmed StrandRegistry::insert(...) routes through the same local invariant validator before registry-relative support-pin checks. crates/warp-core/src/strand.rsL837-L850
• Confirmed registry support-pin validation now only handles registry-relative checks: missing targets and worldline mismatch. crates/warp-core/src/strand.rsL1021-L1035
• Confirmed constructor rejection tests assert the exact INV-S7 and INV-S8 invariant payloads rather than matching broad InvariantViolation(_). crates/warp-core/tests/strand_contract_tests.rsL667-L708
• Confirmed GP1 docs and changelog currently reflect implemented/fixed status for the completed follow-up work. docs/design/braids-and-strands-hardening/goalpost-01-lawful-construction-and-typed-failures.mdL4-L16 docs/design/braids-and-strands-roadmap.mdL59-L71 CHANGELOG.mdL10-L19
• No repository files were changed by this pass, so I did not create a commit or call the PR tool. git status --short still shows the pre-existing unstaged pnpm-lock.yaml modification.

Testing

• ✅ cargo fmt --all -- --check
• ✅ cargo test -p warp-core --test strand_contract_tests
• ✅ cargo test -p warp-core --test braid_public_api_tests
• ✅ git diff --check
• ✅ git status --short checked worktree state; only pre-existing pnpm-lock.yaml remains modified.

View task →