Bind observation settlements to their basis evidence - #27
Conversation
The observation report projected five settlement fields; patch-host projected eleven. The three that mattered were absent: basisDigest, externalEvidenceDigest, and schemaAdmissionEvidenceDigest. A regression in BoundedWorkspaceObservationAdapterV1 that retained a basis over the wrong bytes was invisible to the witness, which checked the returned file contents and nothing about the durable evidence beside them. effect-host now projects attemptId, basisDigest, externalEvidenceDigest, and schemaAdmissionEvidenceDigest, as patch-host does. Making them checkable took more than asserting they are present. A succeeded observation cannot establish the binding at all: the adapter derives a basis over what it read and refuses with stale-basis unless that equals the requested basis, so on the success path the two agree by construction and comparing them proves nothing. That is the vacuous assertion class review of #24 spent several rounds removing, and it is the shape this change had to avoid. The stale-basis refusal is the one settlement family where the retained evidence is independently informative. It carries a basis over what was actually found, which by construction is not what the request declared. The witness reaches that same value by a second route: a separate successful observation declaring those exact bytes derives a basis over the same (path, bytes) pair. Recomputing Echo's domain-separated hash in the consumer would have rebuilt producer logic across the boundary this witness exists to hold. Two guards keep the equality from being satisfied by a constant: the refusal evidence must not equal the requested basis, and the basis must differ for different bytes on a fixed path. Verified by mutation: projecting basis_digest in place of external_evidence_digest, which is exactly the single-arbitrary-value producer the two-route derivation exists to catch, fails the witness. Full CI gate passes locally against the pinned producers. Closes #26
|
Warning Review limit reached
Next review available in: 39 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. 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 (2)
Summary by CodeRabbit
WalkthroughObservation settlement reports now expose attempt and evidence identities. Runtime tests validate these identities for successful and rejected settlements, including stale-basis handling and independent derivation from observed bytes. Roadmap and changelog entries document the verification behavior. ChangesObservation Evidence Binding
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant RuntimeTests as tests/effect-runtime.sh
participant EffectHost as effect-host
participant ObservedBytes as observed bytes
RuntimeTests->>EffectHost: request settlement report
EffectHost->>ObservedBytes: read settlement observation
EffectHost-->>RuntimeTests: return settlement identities
RuntimeTests->>ObservedBytes: independently derive basis
RuntimeTests-->>RuntimeTests: compare retained and derived basis
Possibly related PRs
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@tests/effect-runtime.sh`:
- Line 184: Extend the unknown-report assertions alongside
assert_settlement_evidence to validate all four settlement digest fields, not
only settlement/posture/commit-kind and empty-file conditions. Derive the
observed external evidence independently and require externalEvidenceDigest to
match that value, preventing claimed or stale bytes from satisfying the witness.
🪄 Autofix
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: 2fbbfa36-99b1-41b5-89a3-90d5c84a2e0c
📒 Files selected for processing (4)
CHANGELOG.mddocs/roadmap.mdeffect-host/src/main.rstests/effect-runtime.sh
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: Witness gate
🔇 Additional comments (4)
effect-host/src/main.rs (1)
519-529: LGTM!tests/effect-runtime.sh (1)
129-141: LGTM!Also applies to: 523-567, 730-730
docs/roadmap.md (1)
122-126: LGTM!CHANGELOG.md (1)
83-95: LGTM!
| and (.settlement.commitDigest | test("^[0-9a-f]{64}$")) | ||
| and (.settlement.resultDigest | test("^[0-9a-f]{64}$"))' \ | ||
| "$case_root/settlement-report.json" >/dev/null | ||
| assert_settlement_evidence "$case_root/settlement-report.json" |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 6 \
'uncert|outcomeUnknown|assert_settlement_evidence|externalEvidenceDigest|basisDigest' \
tests/effect-runtime.shRepository: flyingrobots/hello-echo
Length of output: 5105
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
git ls-files | sed -n '1,200p'
echo "== effect-runtime outline =="
wc -l tests/effect-runtime.sh
sed -n '1,80p;180,470p;500,575p' tests/effect-runtime.sh
echo "== repository references to uncertainty/assert settlement evidence/evidence ids =="
rg -n --hidden --iglob '!*.lock' --iglob '!node_modules/**' \
'uncertainty|outcomeUnknown|assert_settlement_evidence|externalEvidenceDigest|basisDigest|schemaAdmissionEvidenceDigest|retained.observed|settlement-report.json|unknown-report.json' .Repository: flyingrobots/hello-echo
Length of output: 29247
Bind uncertainty assertions to retained observed evidence.
unknown-report.json only checks settlement/posture/commit-kind fields and empty files. Apply the same evidence contract there: check the four settlement digest fields and link externalEvidenceDigest to an independently derived observed value so a stale/ambiguous actual observation does not satisfy the witness by replaying claimed bytes.
🤖 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 `@tests/effect-runtime.sh` at line 184, Extend the unknown-report assertions
alongside assert_settlement_evidence to validate all four settlement digest
fields, not only settlement/posture/commit-kind and empty-file conditions.
Derive the observed external evidence independently and require
externalEvidenceDigest to match that value, preventing claimed or stale bytes
from satisfying the witness.
…ishes The roadmap listed four projected identities and said they were bound to the bytes that were actually read. Only the external-evidence digest is, and only in the stale-basis refusal family. The attempt identity is not a digest of observed bytes, the requested basis is by construction the value the request declared, and no case exercises the schema-admission digest at all. The second route is also named more precisely. It is a second settlement path through Echo's own basis derivation, not a second implementation of it: a defect inside bounded_workspace_observation_basis_v1 would appear identically on both routes. The projection and its witness are unchanged. Only the claims move.
1 participant
Closes #26.
What was wrong
The observation report projected five settlement fields.
patch-hostprojected eleven. The three that mattered were absent:basisDigest,externalEvidenceDigest, andschemaAdmissionEvidenceDigest.A regression in
BoundedWorkspaceObservationAdapterV1that retained a basis over the wrong bytes was invisible to the witness, which checked the returned file contents and nothing about the durable evidence recorded beside them.The part that was not obvious
Projecting the fields is three lines. Making them checkable was the actual work, and the first approach I tried was vacuous.
A succeeded observation cannot establish the binding. The adapter derives a basis over what it read and refuses with
stale-basisunless that equals the requested basis:So on the success path
basisDigestandexternalEvidenceDigestare equal by construction, and asserting it proves nothing — the same class of assertion review of #24 spent several rounds removing.What is actually proven
The stale-basis refusal is the one settlement family where the retained evidence is independently informative: it carries a basis over what was actually found, which by construction is not what the request declared.
The witness reaches that same value by a second route — a separate successful observation declaring those exact bytes derives a basis over the same
(path, bytes)pair. Recomputing Echo's domain-separated hash in the consumer would rebuild producer logic across the boundary this witness exists to hold.Observed values:
basisDigest(declared, over'secret')3536d993…externalEvidenceDigest(over what was found)a4e83804…a4e83804…Two guards keep that equality from being satisfied by a constant: the refusal evidence must not equal the requested basis, and the basis must differ for different bytes on a fixed path.
Verification
Mutation-tested. Projecting
basis_digestin place ofexternal_evidence_digest— exactly the single-arbitrary-value producer the two-route derivation exists to catch — fails the witness. Restoring it passes.Full CI gate run locally against the pinned producers (
df80f92a/c354d531):producer-lockruntimeeffect-runtimepatch-runtimebuild-cleans-outputwriter-epoch-assertions