Run compiler-produced Edict operations through durable Ticks - #695
Conversation
|
@coderabbitai review |
|
@codex please review |
✅ Action performedReview finished.
|
|
Warning Review limit reached
Next review available in: 30 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: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (6)
📒 Files selected for processing (12)
📝 WalkthroughWalkthroughAdds ChangesEdict runtime witness
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related issues
Possibly related PRs
Sequence Diagram(s)sequenceDiagram
participant CLI
participant RunEdictOperation
participant TrustedRuntimeHost
participant WAL
participant Scheduler
CLI->>RunEdictOperation: pass package, report, closure, input, and WAL paths
RunEdictOperation->>TrustedRuntimeHost: validate and install admitted package
RunEdictOperation->>TrustedRuntimeHost: submit canonical Action
TrustedRuntimeHost->>WAL: commit accepted submission
TrustedRuntimeHost->>Scheduler: evaluate Action in singleton Tick
Scheduler->>WAL: commit decided Tick
RunEdictOperation->>TrustedRuntimeHost: boot fresh host and recover witnesses
TrustedRuntimeHost-->>RunEdictOperation: state, outcome, Receipt, and obstruction evidence
🚥 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: 6
🤖 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 `@xtask/src/run_edict_operation.rs`:
- Around line 434-443: Replace the boolean .is_err() check in the
mutated_initial_state_refused block with validation that the returned error
specifically identifies the typed state/recovery-certificate mismatch, rather
than accepting any failure. In xtask/src/run_edict_operation.rs lines 434-443,
preserve the refusal assertion only for that reason; in
xtask/tests/run_edict_operation.rs lines 144-224, update assert_rejected to
accept an expected reason and verify captured stderr names each case’s refusal:
canonical-decode failure, resource-binding mismatch, replacement bound, host
byte bound, or nonempty WAL.
- Around line 918-928: Update resource_ref_matches to accept a separate fixed
digest-domain parameter, mirroring validate_resource_ref, and use that domain
when recomputing the digest instead of expected_coordinate. Change both call
sites to pass ADAPTER_DOMAIN for adapter validation and CONFIGURATION_DOMAIN for
target configuration validation, while obtaining expected_coordinate from the
package or adapter closure rather than resource_coordinate(resource).
- Around line 445-478: Update the RunEdictOperationReport construction so
verdict, verificationOutcome, closureBound, singletonIntegration,
hiddenMutation, and recovery fields reflect observed execution state rather than
constants or unreachable values. Propagate those postures through the relevant
run_edict_operation control flow, and let the caller determine verdict from
them; alternatively remove fields that cannot vary and adjust the report schema
and assertions accordingly.
In `@xtask/tests/fixtures/edict-operation/lawpack-manifest.cbor`:
- Around line 1-2: Replace the filler digests for verifier.ruleset,
dependencies.compatibility, and conformanceFixtureCorpus by regenerating the
manifest from a real compiler run, or revise the WAL documentation to avoid
claiming these are compiler-produced bytes. Also update the digest-domain
handling around digest_canonical_value_bytes_v1 to use ADAPTER_DOMAIN rather
than targetAdapters[0].adapter.id.
In `@xtask/tests/fixtures/edict-operation/verification-report.cbor`:
- Line 1: Update validate_closure to cross-check the report’s targetIr digest
against the lawpack manifest’s acceptedTargetIr digest, not merely their
resource coordinates; only emit closureBound: true when the byte digests match,
otherwise reject the closure or report it as unbound.
In `@xtask/tests/run_edict_operation.rs`:
- Line 19: Update the test path setup around root and fixture path construction
to anchor both fixture locations and the target/xtask-test-tmp scratch directory
to env!("CARGO_MANIFEST_DIR") instead of the process working directory,
preserving the existing relative layout so tests run consistently from any CWD.
🪄 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 Plus
Run ID: 14fd5c70-106d-4065-aa25-82073945ef6a
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (16)
CHANGELOG.mdREADME.mddocs/topics/GeneratedRules.mddocs/topics/WAL.mdxtask/Cargo.tomlxtask/src/main.rsxtask/src/run_edict_operation.rsxtask/src/runtime_counter_diagnostic.rsxtask/tests/fixtures/edict-operation/ORIGIN.tomlxtask/tests/fixtures/edict-operation/executable-operation-package.cborxtask/tests/fixtures/edict-operation/input.jsonxtask/tests/fixtures/edict-operation/lawpack-adapter.cborxtask/tests/fixtures/edict-operation/lawpack-manifest.cborxtask/tests/fixtures/edict-operation/target-configuration.cborxtask/tests/fixtures/edict-operation/verification-report.cborxtask/tests/run_edict_operation.rs
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 6
🤖 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 `@xtask/src/run_edict_operation.rs`:
- Around line 434-443: Replace the boolean .is_err() check in the
mutated_initial_state_refused block with validation that the returned error
specifically identifies the typed state/recovery-certificate mismatch, rather
than accepting any failure. In xtask/src/run_edict_operation.rs lines 434-443,
preserve the refusal assertion only for that reason; in
xtask/tests/run_edict_operation.rs lines 144-224, update assert_rejected to
accept an expected reason and verify captured stderr names each case’s refusal:
canonical-decode failure, resource-binding mismatch, replacement bound, host
byte bound, or nonempty WAL.
- Around line 918-928: Update resource_ref_matches to accept a separate fixed
digest-domain parameter, mirroring validate_resource_ref, and use that domain
when recomputing the digest instead of expected_coordinate. Change both call
sites to pass ADAPTER_DOMAIN for adapter validation and CONFIGURATION_DOMAIN for
target configuration validation, while obtaining expected_coordinate from the
package or adapter closure rather than resource_coordinate(resource).
- Around line 445-478: Update the RunEdictOperationReport construction so
verdict, verificationOutcome, closureBound, singletonIntegration,
hiddenMutation, and recovery fields reflect observed execution state rather than
constants or unreachable values. Propagate those postures through the relevant
run_edict_operation control flow, and let the caller determine verdict from
them; alternatively remove fields that cannot vary and adjust the report schema
and assertions accordingly.
In `@xtask/tests/fixtures/edict-operation/lawpack-manifest.cbor`:
- Around line 1-2: Replace the filler digests for verifier.ruleset,
dependencies.compatibility, and conformanceFixtureCorpus by regenerating the
manifest from a real compiler run, or revise the WAL documentation to avoid
claiming these are compiler-produced bytes. Also update the digest-domain
handling around digest_canonical_value_bytes_v1 to use ADAPTER_DOMAIN rather
than targetAdapters[0].adapter.id.
In `@xtask/tests/fixtures/edict-operation/verification-report.cbor`:
- Line 1: Update validate_closure to cross-check the report’s targetIr digest
against the lawpack manifest’s acceptedTargetIr digest, not merely their
resource coordinates; only emit closureBound: true when the byte digests match,
otherwise reject the closure or report it as unbound.
In `@xtask/tests/run_edict_operation.rs`:
- Line 19: Update the test path setup around root and fixture path construction
to anchor both fixture locations and the target/xtask-test-tmp scratch directory
to env!("CARGO_MANIFEST_DIR") instead of the process working directory,
preserving the existing relative layout so tests run consistently from any CWD.
🪄 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 Plus
Run ID: 14fd5c70-106d-4065-aa25-82073945ef6a
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (16)
CHANGELOG.mdREADME.mddocs/topics/GeneratedRules.mddocs/topics/WAL.mdxtask/Cargo.tomlxtask/src/main.rsxtask/src/run_edict_operation.rsxtask/src/runtime_counter_diagnostic.rsxtask/tests/fixtures/edict-operation/ORIGIN.tomlxtask/tests/fixtures/edict-operation/executable-operation-package.cborxtask/tests/fixtures/edict-operation/input.jsonxtask/tests/fixtures/edict-operation/lawpack-adapter.cborxtask/tests/fixtures/edict-operation/lawpack-manifest.cborxtask/tests/fixtures/edict-operation/target-configuration.cborxtask/tests/fixtures/edict-operation/verification-report.cborxtask/tests/run_edict_operation.rs
🛑 Comments failed to post (2)
xtask/tests/fixtures/edict-operation/lawpack-manifest.cbor (1)
1-2: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Fixture embeds hand-written placeholder digests while the docs call these exact compiler bytes.
The
verifier.ruleset,dependencies.compatibility, andconformanceFixtureCorpusreferences carry filler digests (32 repeatedt,u, andvbytes). The runner never digest-checks those three, so they pass — butdocs/topics/WAL.mdLine 316 describes this fixture as the artifacts "that closed the compiler build". Either regenerate the manifest from a real compiler run or soften the documentation claim.Separately, this file's
targetAdapters[0].adapter.id(causal.cell.echo-adapter/v1) is what the runner feeds todigest_canonical_value_bytes_v1as the digest domain — concrete evidence for the domain-confusion issue flagged atxtask/src/run_edict_operation.rsLines 918-928, since that string has nothing to do withADAPTER_DOMAIN.🤖 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 `@xtask/tests/fixtures/edict-operation/lawpack-manifest.cbor` around lines 1 - 2, Replace the filler digests for verifier.ruleset, dependencies.compatibility, and conformanceFixtureCorpus by regenerating the manifest from a real compiler run, or revise the WAL documentation to avoid claiming these are compiler-produced bytes. Also update the digest-domain handling around digest_canonical_value_bytes_v1 to use ADAPTER_DOMAIN rather than targetAdapters[0].adapter.id.xtask/tests/fixtures/edict-operation/verification-report.cbor (1)
1-1: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
The report's
targetIrdigest is never cross-checked against the manifest'sacceptedTargetIr.This fixture's
targetIrdigest andlawpack-manifest.cbor'sacceptedTargetIrdigest are different byte strings, and the runner only validates the coordinate for both (require_resource_identity,xtask/src/run_edict_operation.rsLines 565 and 930-937). So nothing proves the Target IR the verifier accepted is the Target IR the lawpack admits, yet the report emitsclosureBound: true. Either bind the two digests invalidate_closureor stop claiming the closure is bound.🤖 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 `@xtask/tests/fixtures/edict-operation/verification-report.cbor` at line 1, Update validate_closure to cross-check the report’s targetIr digest against the lawpack manifest’s acceptedTargetIr digest, not merely their resource coordinates; only emit closureBound: true when the byte digests match, otherwise reject the closure or report it as unbound.
|
@coderabbitai review |
|
@codex please review |
✅ Action performedReview finished.
|
![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: 5acff98cd9
ℹ️ 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".
|
Second-pass disposition for the two CodeRabbit findings that could not be posted inline:
Local gate: 137 xtask tests, 38 executable-operation pipeline tests, clippy, rustfmt, Prettier, Markdownlint, and dead-reference checks pass at |
|
Merge gate record for
The operator explicitly authorized administrative merge. That bypass is used only for the impossible solo-maintainer approval criterion; no check or review finding is bypassed. |
1 participant
Consumes exact compiler and verifier artifacts through the generic Echo Action path. Proves WAL-before-ACK, pending restart, scheduler-owned singleton execution, decided restart, typed duplicate obstruction, and altered-basis refusal.
Closes #690.
Summary by CodeRabbit
New Features
Documentation
Tests