Prove bounded workspace observation - #22
Conversation
|
@coderabbitai review |
|
@codex please review |
✅ Action performedReview finished.
|
|
Warning Review limit reached
Next review available in: 50 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: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
Summary by CodeRabbit
WalkthroughAdds a compiler-authored bounded workspace observation application, a Rust host for durable request/claim/settlement handling, build and execution scripts, and an end-to-end witness covering recovery, retries, replay, refusals, ambiguity, budgets, artifact validation, and bounded stress. ChangesWorkspace observation
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant build as effect-build.sh
participant compiler as edict-cli
participant host as hello-effect-host
participant wal as WAL coordinator
participant workspace as workspace observation adapter
build->>compiler: build application from compiler settings
compiler-->>build: emit core.cbor and target-ir.cbor
build->>host: execute request, claim, and settle phases
host->>wal: record request and claim
host->>workspace: observe bounded workspace root
workspace-->>host: return files, basis evidence, or refusal
host->>wal: admit settlement
wal-->>host: return durable posture and commit ordering
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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 |
![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: eec0fb96f5
ℹ️ 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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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 `@effect-host/src/main.rs`:
- Around line 80-97: Update the admission-error match around
admit_edict_external_action_request_v1 so every invocation phase, not only
"request", emits the structured compilerArtifactRejected obstruction with
wal.commitCount set to 0 and exits with the existing rejection status. Remove
the phase-specific guard while preserving successful admission handling and the
existing formatted error path only for errors that are not compiler-artifact
admission rejections.
In `@README.md`:
- Around line 130-134: Update the refusal bullet in README.md to include the
stale-basis refusal alongside unauthorized, parent-escaped, and symlink paths,
aligning it with the documented four-refusal count and runtime witness.
In `@tests/effect-build.sh`:
- Around line 57-65: Update the path preparation in the effect build script
before the sed substitutions for ECHO_REPO and project_root: serialize each path
as a valid TOML string so quotes and backslashes are preserved, then escape the
serialized values for safe sed replacement. Keep the existing `@ECHO_REPO`@ and
`@PROJECT_ROOT`@ substitutions intact.
In `@tests/effect-run.sh`:
- Around line 24-25: Normalize EFFECT_CORE_FILE and EFFECT_TARGET_IR_FILE
against project_root when their environment-provided values are relative,
matching the existing request_file and wal_dir handling; preserve absolute paths
and the current defaults.
In `@tests/effect-runtime.sh`:
- Around line 441-446: Update the checkout-path leak check’s artifact glob in
the grep block to include both report JSON and recovery JSON files, covering the
request-recovery.json and claim-recovery.json artifacts asserted earlier while
preserving the existing path-content validation.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: ca289ab3-50ce-4e0c-b44c-2db4f81098ec
📒 Files selected for processing (15)
CHANGELOG.mdREADME.mddocs/roadmap.mdedict.effect.application.jsoneffect-host/Cargo.toml.templateeffect-host/src/main.rseffect/src/observe-workspace.edicteffect/vendor/workspace-snapshot/adapter.cboreffect/vendor/workspace-snapshot/exports.cboreffect/vendor/workspace-snapshot/manifest.cboreffect/vendor/workspace-snapshot/request-profile-configuration.cbortests/effect-build-request.jsonltests/effect-build.shtests/effect-run.shtests/effect-runtime.sh
📜 Review details
🔇 Additional comments (12)
edict.effect.application.json (1)
1-21: LGTM!effect/src/observe-workspace.edict (1)
6-12: 🔒 Security & PrivacyProve the symlink rule is part of the admitted contract.
The request binds paths, scope, basis, and budgets, but no link-following policy. Confirm that
workspace.snapshot.input@1’s verified schema fixesno-followand that settlement evidence retains that commitment; otherwise provider behavior can drift from the compiler-authorized policy.Also applies to: 20-32
effect/vendor/workspace-snapshot/adapter.cbor (1)
1-1: LGTM!effect/vendor/workspace-snapshot/exports.cbor (1)
1-1: LGTM!effect/vendor/workspace-snapshot/manifest.cbor (1)
1-2: LGTM!effect/vendor/workspace-snapshot/request-profile-configuration.cbor (1)
1-1: LGTM!tests/effect-build-request.jsonl (1)
1-1: LGTM!effect-host/Cargo.toml.template (1)
1-17: LGTM!effect-host/src/main.rs (2)
326-336: 🎯 Functional Correctness | ⚡ Quick winPositional constructor with adjacent same-typed
Hashfields is a silent-swap risk.
ExternalActionSettlementCandidateV1::newtakes 9 positional args, several of which are back-to-backHashvalues (settlement_schema_digest/basis_digest, then laterschema_admission_evidence_digest/external_evidence_digest). A transposition here compiles cleanly and would corrupt retry reconciliation silently. Worth confirming the order against the declared signature inwarp-core, and, if the crate exposes a builder or named-field constructor, preferring that here.
556-571: 🗄️ Data Integrity & IntegrationConfirm the
warp-coreepoch fencing contract before relying on this host setup.The local code always hashes the same epoch literals and passes
previous_epoch_id: None/previous_epoch_final_commit_digest: None; the checkpoint requires thewarp-corecontract to determine whether this counts as idempotent recovery or bypasses real epoch-lineage fencing for stale writers.CHANGELOG.md (1)
23-26: LGTM!docs/roadmap.md (1)
111-119: LGTM!Also applies to: 133-136
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@codex please review |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Merge gate: OPEN at 97be4f1. Full local pure-runtime and Hello Effect witness suites, Rust formatting/clippy, ShellCheck, Markdown lint, JSON validation, and diff checks pass. CodeRabbit completed rate-limited with its check passing; Codex completed usage-limited. GraphQL reports zero unresolved review threads, APPROVED, MERGEABLE, and CLEAN. Solo-maintainer technical-gate substitution applies; no independent human approval is required by branch protection. |
1 participant
Admit compiler-authored observation requests, durably settle bounded workspace reads, and prove recovery, reconciliation, and effect-free replay. Closes #10.