Conversation
samovers
left a comment
There was a problem hiding this comment.
Review disposition: Request changes — one blocking finding
Reviewed PR #28 at ae080693c2ea0c017ae8cc7a7080841ccbb306c0, against base 71ca724a8b6ec23f1655b086a6f549496d10a47f. The reviewed change adds exactly one file: the 417-line sharing-revocation Phase A candidate.
Blocking finding — §5.1 changes the approved delegation ceiling
Location: sharing_access_revocation_protected_effect_contract_rfc_candidate_v0_1.md, line 97.
The candidate labels its action-binding table “Fixed approved value consumed here,” but specifies:
EXACT_ONLY / EXPLICIT_SOURCE_PERMISSION_REQUIRED
The pinned PR #11 candidate at 03a21f669ee04f96d444e14f00ae7212cab04803 instead selects EXACT_ONLY / DELEGATION_ALLOWED for SHARE_REVOKE_ACCESS. Its §7.2 matrix uses X / DA and explicitly defines DA as DELEGATION_ALLOWED.
These are not interchangeable labels. PR #11 §10 distinguishes them:
| Ceiling | Permitted source delegation permission |
|---|---|
DELEGATION_ALLOWED |
Permission covering all granted actions, or permission listing the requested action |
EXPLICIT_SOURCE_PERMISSION_REQUIRED |
Permission explicitly listing the requested action; blanket permission is insufficient |
That difference is an authorization rule, not a domain-result mapping choice.
Concrete failure scenario: a source AuthorityGrant covers SHARE_REVOKE_ACCESS and carries DELEGABLE_GRANTED_ACTIONS, without a narrowing action list. A valid delegation and independently eligible human approval satisfy every other requirement. PR #11 permits that delegated path to be evaluated successfully; PR #28’s substituted ceiling excludes the blanket permission. A later implementation could therefore reject an otherwise valid revocation or encounter a contradictory rule/contract binding. This would narrow authorization inside a contract that explicitly says it does not change who may revoke.
Smallest required patch:
- Replace the ceiling in §5.1 with
DELEGATION_ALLOWED. - Add a design-level regression case covering the valid blanket-source-permission scenario, and require the consumed action-binding values to match the pinned PR #11 row.
- Do not add a second delegation evaluator to this contract.
This is a correction to the candidate’s dependency transcription—not a reason to amend PR #11, change baseline law, or reopen delegation design.
Remaining substantive checks
No additional blocking issue was found in the following areas:
Result mapping and accountable human. The twelve-field table closes every result property, distinguishes the eligible human approver’s authority-subject Party from the requesting principal, preserves representation evidence, copies the reason exactly, and requires the action/replacement arrays to be absent. Those are explicit proposed domain choices rather than hidden result-constructor discretion.
Temporal meaning and commit proof. The candidate preserves trusted human-act time, requested effective time, authorization time, and actual commit time separately. It explicitly distinguishes a precommit domain PASS from a successful transaction receipt. The earlier concern about presenting validation as proof that revocation already committed is addressed.
Unchanged carrier and evidence association. The candidate does not pretend that bare RevocationDecision v0.1 JSON proves the complete effect. It binds the omitted source, artifact, human, and transaction facts through immutable evidence, requires complete mapping/postcondition dispositions, and avoids hashing a future receipt back into its own precommit trace. Missing executable bindings remain explicit materialization gates.
Classification and transaction scope. The proposed GovernanceEvent / governance decision classification is accompanied by an explicit semantic justification. It creates no automatic companion event, grant mutation, replacement, current-state update, or dispatch. The fresh-approval handoff keeps domain failure separate from authorization outcomes and consumes the shared transaction protocol rather than defining another one.
Verification and disposition
GitHub’s repository-validation and generated-currentness checks passed at the reviewed head. The thirty hostile/positive cases are specifications, not implemented tests; the candidate states that limitation explicitly. This review did not execute a runtime validator or local conformance suite.
Required authority impact: amend only this non-authoritative Phase A candidate. No active-baseline, accepted-RFC, schema, currentness, or OFARM2 runtime change is justified by this finding.
Keep semantic approval pending until the delegation mismatch is corrected and the amended head is re-reviewed.
|
The requested delegation correction is published at Correction diff from the reviewed head — one Phase A file, 7 additions and 4 deletions.
Verification passed: all nine consumed table rows match the pinned source definitions; a static negative control detects the former wrong ceiling; 12 result-field mappings, nine postconditions and 31 unique consecutive case specifications remain complete. Repository hygiene, generated currentness, cross-reference and steward checks passed locally. GitHub repository-validation and generated-currentness checks passed at this exact head for both push and PR runs. The remote file blob matches the clean local worktree. The 31 cases remain design specifications, not implemented runtime tests. Scope stayed within sharing-revocation record semantics and classification. The previous review is preserved; the amended candidate requires re-review and remains unapproved. What is next: re-review section 5.1, SR-T31 and their traceability updates at this exact head before considering semantic approval. |
samovers
left a comment
There was a problem hiding this comment.
Re-review disposition: No blocking findings
Reviewed PR #28 at 4e186aab5c238215906fcf9b24ce74443abb6f72, against the previously reviewed head ae080693c2ea0c017ae8cc7a7080841ccbb306c0.
The correction is one commit, with 7 additions and 4 deletions, confined to the existing Phase A candidate. At re-review, the PR remained draft and unmerged.
Previous blocker: closed
§5.1 now preserves the approved delegation ceiling. It specifies EXACT_ONLY / DELEGATION_ALLOWED, matching the pinned PR #11 candidate at 03a21f669ee04f96d444e14f00ae7212cab04803's X / DA row. The incorrect substitution of EXPLICIT_SOURCE_PERMISSION_REQUIRED is removed.
New case SR-T31 covers the actual failure scenario. It specifies a source AuthorityGrant with DELEGABLE_GRANTED_ACTIONS, no narrowing action list, a valid delegation, independently eligible human approval, and all other conditions satisfied. The domain contract must not reject that path by imposing an action-specific source-permission list. This matches PR #11 §10 without treating blanket delegation permission as sufficient authority on its own.
The correction preserves ownership of authorization. The added paragraph requires every consumed action-binding value to match the pinned row, legends, and governing definitions. A mismatch blocks binding review rather than becoming a local policy choice. Delegation evaluation remains with authorization; no second evaluator is introduced. All nine table rows were checked against the applicable pinned definitions, with no remaining mismatch found.
Regression and scope check
The traceability table and verification checklist now include SR-T31. The correction leaves the previously reviewed result mappings, human/Party attribution, temporal semantics, immutable evidence association, event classification, and transaction handoff unchanged. It introduces no baseline, schema, currentness, or runtime changes. No new blocking finding emerged from the revision.
Verification and remaining gates
The GitHub workflow results were independently checked for 4e186aa: Validate repository and Validate generated currentness both completed successfully. The candidate still correctly distinguishes its 31 case specifications from executable tests; those cases have not been implemented or exercised by these checks. This review did not run a local conformance suite or runtime validator.
The candidate is ready for exact-head steward semantic approval at 4e186aab5c238215906fcf9b24ce74443abb6f72. This review does not grant that approval or authorize merge, accepted-law promotion, machine-contract materialization, current/default activation, or OFARM2 runtime implementation. The separately owned binding and delivery gates remain in place.
Task-user Phase A semantic approval — recorded by the AIThe task user replied “i approve” immediately after being asked for exact-head Phase A semantic approval of PR #28 following its no-blocker re-review. This records that explicit instruction in context; it does not claim the user typed the decision identifier or commit hash verbatim, and it is not a separate approval granted by the AI. Approved scope:
Exact-head re-review closed the previous blocker and found no new blocking issues. The reviewed head, one-file scope, candidate blob and pinned dependencies were rechecked before recording approval. This is Phase A semantic approval only. It does not authorize a merge, accepted-law promotion, machine-contract/schema materialization, current/default activation, partial action-rule activation, production-readiness claims, or OFARM2 runtime implementation. The 31 positive/hostile cases remain specifications, not executed effect tests. PR #11 remains pinned at The reviewed document and review history remain unchanged. Its in-file “approval pending” text is the reviewed snapshot; this later approval record and live tracking metadata record the subsequent decision without changing the approved bytes. PR #28 remains draft and unmerged. No other trust boundary was changed. What is next: reconcile the remaining canonical prerequisites under #12 and #14 before selecting the next bounded task; executable binding review and OFARM2 implementation remain gated. |
Phase A only — exact-head semantic approval recorded
Refs #27; parent #12. Downstream: OFARM2#353 / draft PR #359.
Candidate head:
4e186aab5c238215906fcf9b24ce74443abb6f72.Base:
71ca724a8b6ec23f1655b086a6f549496d10a47f.Approval record — 2026-09-07
The task user's Phase A semantic approval is recorded here for decision
OFARM-ISSUE27-SHARING-REVOCATION-PROTECTED-EFFECT-001, version1, at this exact head. The exact-head re-review closed the previous blocker and found no new blocking issues.Approval is limited to the sharing-revocation domain effect semantics and commit classification. It does not authorize merging, accepted-law promotion, schema or executable-contract materialization, current/default activation, partial rule-set activation, or OFARM2 runtime implementation. This PR remains draft and unmerged.
The reviewed document is unchanged: 420 lines, Git blob
944e3d3c14bea3996a80e7ff03675f1b52babd59. Its in-file pending-approval wording is the reviewed snapshot; this live metadata and the linked approval record record the subsequent decision without changing approved bytes.Review correction — exact-head re-review completed
Addresses the single blocker in the review of head ae08069:
DELEGATION_ALLOWED, matching pinned PR RFC candidate: executable authorization evidence v0.2 #11'sX / DArow.SR-T31: a valid sourceAuthorityGrantwithDELEGABLE_GRANTED_ACTIONSand no narrowing action list must not be rejected by a stricter domain-local ceiling.The correction changes only the existing Phase A file: 7 additions and 4 deletions from
ae080693c2ea0c017ae8cc7a7080841ccbb306c0. The other proposed domain choices and approval-card version are unchanged. The exact amended head has now passed the linked no-blocker re-review and received the limited Phase A semantic approval recorded above.Outcome
Define how one authorized sharing-permission cancellation is recorded without changing the original grant or its history. The candidate proves the exact saved result; it does not change who may revoke access or claim that every independent access path has ended.
Primary trust boundary and exact PR scope
Sharing-revocation domain effect semantics and commit classification.
Exactly one new 420-line historical Phase A document:
package_meta/history/clean_baseline_migration/phase_reports/sharing_access_revocation_protected_effect_contract_rfc_candidate_v0_1.md.No accepted law, Event Grammar, companion policy, schema, currentness index, existing candidate PR, authorization evaluator, transaction manager, database code or OFARM2 runtime changed. Scope stayed within the named boundary.
Approved Phase A semantic choices
SHARE_REVOKE_ACCESS, one exactSharingGrant, and one newRevocationDecision v0.1usingTERMINATE.decidedAt.notes, preserve exacteffectiveFromand scope, and require both action/replacement arrays absent.GovernanceEventand its record asgovernance decision, with no automatic companion event, current-state update or external dispatch.The decision card is in section 15:
OFARM-ISSUE27-SHARING-REVOCATION-PROTECTED-EFFECT-001, version1. The card's reviewed bytes remain unchanged; the linked task-user approval record supplies the later exact-head Phase A decision.Exact dependencies and remaining gates
03a21f669ee04f96d444e14f00ae7212cab04803.98f8c4fafbae42c8f7fd931f43f53adcb4733713.Any review finding requiring another trust boundary must stop at a separate prerequisite, not be appended here.
Verification
Passed on the final committed content:
git diff --check 71ca724a8b6ec23f1655b086a6f549496d10a47f HEADpython3 -B package_meta/tools/validate_repo_hygiene.pypython3 -B package_meta/tools/check_generated_currentness.pypython3 -B package_meta/tools/check_repository_cross_references.pypython3 -B package_meta/tools/check_repository_steward_guardrails.pyThese are author checks, not independent semantic review. The 31 cases are specifications, not executed fixtures. Local checks were limited to the commands and static checks above; no full repository conformance suite was run locally. No production command, PostgreSQL test or runtime effect validation was run.
GitHub's automatic
repository-validationandgenerated-currentnesschecks passed at this exact head for both push and pull-request runs. Those checks exercise the existing repository, not the 31 unimplemented sharing-revocation case specifications.Completed focused re-review
The completed re-review covered the bounded correction from
ae080693c2ea0c017ae8cc7a7080841ccbb306c0to4e186aab5c238215906fcf9b24ce74443abb6f72: section 5.1's exact delegation ceiling/dependency fidelity, new caseSR-T31, and the matching traceability/checklist updates. It closed the previous blocker and found no new blocking issues. The result mappings, temporal meaning, evidence association, classification and common transaction handoff have not changed. Phase A semantic approval is recorded above; executable binding review and all later delivery gates remain outstanding.What is next: reconcile the remaining canonical prerequisites under #12 and #14 before selecting the next bounded task; executable binding review and OFARM2 implementation remain gated.