Constrain and authorize legacy correction supersession - #380
Conversation
Document the closed correction relationship and separate retirement action for K-02. Preserve inert queue intent, existing transactions and production closure; implementation awaits semantic approval. Delivery #379.
|
Phase A design review completed at 28a16d3, based on merged ff092c4. Delivery #379; draft PR #380. Primary boundary: semantic promotion and supersession authorization, legacy only. Scope stayed inside it. Three coordinated AI specialist reviews covered canonical authority and accepted intent, implementation paths and code excellence, and reproduction/evidence/verification. The lead inspected each report and independently executed the live HTTP/PostgreSQL reproduction. Consolidated disposition: 0 Blockers, 0 new Follow-ups, 0 Preferences. Existing #180/#184 lifecycle and graph work, K-03 and K-04 remain separate.
Remote draft state/head and documentation-only scope were verified. The only committed change is the proposed 221-line RFC. Mandatory package contract check passed with 0 failures, including architecture constraints; whitespace check passed and worktree is clean. This review establishes Phase A readiness only. It does not establish an implemented fix, semantic approval, runtime readiness or merge authority. No expensive baseline admission or execution has been requested for this Phase A head. Next: present the complete same-task decision card for OFARM2-LEGACY-CORRECTION-AUTHORIZATION-001 version 1, naming PR #380, and obtain exact semantic approval before implementation. |
samovers
left a comment
samovers
left a comment
There was a problem hiding this comment.
Phase A review — Delivery #379 / PR #380
Verdict: no demonstrated in-scope Phase A blockers. One Follow-up concerning existing transaction ownership.
Reviewed head: 28a16d3c3f3cce42de568b71250c6d45af8f6c0a
Base: ff092c414db9fa24dbd6ab86c7722db89e0c95b5
This review concerns the draft PR's single changed file: the 221-line proposed correction-authorization RFC. No implementation exists at this head. This is a content/design review, not semantic approval, baseline admission, implementation acceptance, merge authorization or deployment authority.
F1 — Follow-up: shared-connection HTTP concurrency is not covered by the separate-Store guarantee
Affected existing code: kernel/legacy_m1/api.py, kernel/legacy_m1/runtime.py, and kernel/store.py.
The legacy app shares one runtime, pipeline and Store across its request handlers. The Store retains one database connection, and serialized_tx() acquires a PostgreSQL transaction advisory lock without providing request-level exclusion around that connection. The synchronous HTTP handlers can execute concurrently in FastAPI's thread pool.
Sources: legacy HTTP app, legacy runtime composition, Store connection and transaction ownership, and FastAPI synchronous path-operation execution.
That distinction matters: cursors sharing a Psycopg connection also share its transaction. An overlapping transaction context can become a nested savepoint, and reacquiring an advisory lock already held by the same database session does not block. I checked the transaction-context implementation at Psycopg 3.3.4, not just the general documentation: connection implementation, transaction implementation, shared-connection concurrency, and transaction/savepoint behavior.
A source-level counterexample is therefore possible:
Request A: validate predecessor C; pause before emission.
Request B: enter the same connection's nested transaction;
validate C; emit its correction; release the savepoint.
Request A: resume using its previously checked C; emit another correction.
The database lock does not prevent this interleaving. Releasing B's savepoint also does not independently commit B's work. The existing schema's trace-reachability constraint is not a uniqueness constraint on supersession targets. See the existing schema.
Evidence limit: this is a source-based finding, not a live PostgreSQL reproduction performed during this review.
Classification: Follow-up, not a blocker for this narrow authorization design. It predates the PR, concerns transaction ownership explicitly excluded from this Delivery, and does not demonstrate a bypass of the proposed supersession-permission check. Nevertheless, C08's two-independent-Store test must not be presented as proof of concurrent requests against one legacy app. Record separate transaction-ownership work: exclusive request transaction ownership, tested through concurrent requests to the same app. Do not silently expand this PR into that redesign.
Why the correction design otherwise holds
It addresses two independent defects rather than fixing only the permission check. The existing submission validator accepts a same-farm, non-superseded consequence without establishing a compatible replacement relationship. Queued acceptance separately selects the first stored intent edge. The proposed shared resolver replaces these unchecked interpretations with one validated predecessor consumed by the emitters. See validators, emission, and the proposed RFC.
Checking only consequence type would be insufficient: observation and structure both currently produce STATE_CHANGE_ACCEPTED. Checking the originating assertion, accepting review, event and typed structural payload is therefore necessary to prevent an observation from being treated as structural history. The proposed same-identity rule preserves D18; operation compatibility remains explicitly narrower than a durable operation-identity model. See policy mappings and settled D5/D17/D18 decisions.
Authority is checked at the retirement effect, not at inert capture. Allowing an ordinary assertion author to queue a compatible correction does not change current truth. Requiring the actual accepting reviewer to hold both REVIEW_ACCEPT and REVIEW_SUPERSEDE before emission matches the canonical action distinction. Keeping the standalone supersede request disabled avoids accidentally activating a broader governance capability. See the pinned canonical Authority Action Matrix and RFC C05/C06/C09.
The design handles legitimate historical provenance. An accepted queued assertion remains stored as PENDING_REVIEW; its accepting decision and consequence establish the accepted outcome. Rejecting that stored state would break lawful subsequent corrections. Likewise, an old accepted predecessor may belong to an earlier RuntimeBundle, while the queued assertion being accepted must still pass the existing current-bundle gate. The RFC preserves this distinction. See review/dispute semantics, GovernanceAcceptanceValidator, and the proposed RFC's owned decision path.
Implementation evidence still required
The most important verification discipline is to isolate compatibility failure from authority failure. A cross-class correction must refuse even when the reviewer possesses supersession rights; a compatible correction must refuse when those rights are absent, expired or revoked. Otherwise, one working guard could conceal the other being missing. The planned matrix supports both sets of tests.
The implemented slice must also demonstrate lawful direct and queued corrections, repeated correction chains, poisoned or multiple intents, stale queued predecessors, unchanged predecessor bytes, action-specific authorization receipts, rollback and replay behavior. Those are already acceptance requirements—not additional scope. Observation/compliance correction remains unsupported, and broader lifecycle work remains open.
Authority and verification limits
This is an implementation/conformance RFC, not a baseline-law amendment. No active baseline files need changing. Production governed endpoints remain explicitly blocked in the current production app; this review does not establish production readiness.
I reviewed the proposed RFC and its owning implementation paths. I did not execute repository tests or the package checker, and did not independently reproduce the author's K-02 HTTP demonstration. The PR's reported reproduction concerns the existing defect, not completion of a fix that has not yet been implemented.
Recommendation: proceed to the named semantic-approval decision for the bounded correction-authorization design. Keep the shared-connection issue separate and keep K-02 open until implementation and its own verification are complete.
samovers
left a comment
samovers
left a comment
There was a problem hiding this comment.
PR #380 — Constrain and authorize legacy correction supersession (Phase A)
Independent review at head 28a16d3c3f3cce42de568b71250c6d45af8f6c0a, base
ff092c414db9fa24dbd6ab86c7722db89e0c95b5. Documentation-only; the sole change is
docs/rfcs/OFARM_Legacy_Correction_Authorization_RFC_v0_1.md, 221 lines / 15,677 bytes,
blob bdf514581efc116a0faf7a84b0bad129e3b5dc1f,
sha256:d42b359d12fdb391ebc386ed0768c6e84441dbacb64ce2104daf8aff31b28518. git diff --check
is clean. Classified against the RFC's own EXC-001–EXC-006 and its C01–C09 matrix, and against
issue #379's falsifiable acceptance criteria.
Position relative to the existing pass. One prior review exists, as issue comment
5571568278 by samovers at this same head: "three coordinated AI specialist reviews and lead
consolidation", disposition 0 Blockers / 0 Follow-ups / 0 Preferences. It is authored by the PR
author, so it is a self-review; GitHub refuses APPROVE from the author, which is why it is an
issue comment and /pulls/380/reviews is empty. This pass is also samovers. Neither
satisfies an independent two-reviewer gate — say so in the decision card rather than letting the
"0 Blockers" line stand as external clearance. I measured first and read that comment afterwards.
I independently reproduce its central claim, and I disagree with its disposition: I find two
Blockers it did not reach, both in the space between the RFC's refusal list and behaviour that
exists in the code today.
Disposition: 2 Blockers, 2 Should fix, 2 Follow-ups. Phase A is not ready for the decision
card until the Blockers are closed.
Method and environment
Everything below was executed, not reasoned about. Fresh anonymous clone of
samovers/OFARM2, detached at the base commit, with the repository's own locked dependency
baseline (requirements-review-baseline.lock, requirements-review-tools.lock).
- CPython 3.12.13, built from the
v3.12.13GitHub tag — the exact interpreter the
architecture check's fixed source-snapshot descriptor demands (3.12.3 is refused
UNSUPPORTED_PYTHON_VERSION). - ruff 0.15.5, the repository pin. Without it the architecture check fails
repository-pinned Ruff formatter is unavailable, not on any constraint. - Live PostgreSQL 16.13 on x86_64 Linux, real
Store, realGatePipeline, real legacy
FastAPI app viaTestClient.
Ten probes in one module against the base commit; the module was deleted afterwards and the
clone is clean. Full transcript below. Deviations, stated at the point of the result: this is
PostgreSQL 16.13, not the pinned 17.10, and x86_64 Linux, not the RFC's Darwin ARM64
supplemental run nor the locked baseline. The kernel Store does not enforce the 17.10 pin (only
the tenant-provisioning path does), so every result below is on the code path under review; but a
17.10-specific behaviour would not show here. Two probes drive the pipeline in-process rather
than over HTTP; the two K-02 probes and both CONTEST probes go through the real app.
Base-commit check state, measured
| Check | Result at ff092c4 |
|---|---|
conformance/rewrite_architecture_check.py |
PASS |
conformance/ofarm_pkg_contract_check.py |
PASS (0 failures) (runs the architecture check internally) |
conformance/ofarm_profile_extraction_consistency_check.py |
FAIL (2 failures) |
The two extraction failures are missing review records for seed-scan hit paths
conformance/review_baseline_test_inventory.json and
kernel/tests/test_rewrite_architecture_check.py. This check is not in the RFC's verification
list. Hold that thought — it returns in Blocker 2.
The RFC's central claim: reproduced
K-02 reproduces exactly as described, on both paths, at the base commit.
[A1] structural predecessor consequenceType: STATE_CHANGE_ACCEPTED
[A1] attacker grants: ASSERT_OPERATION_CLAIM, REVIEW_ACCEPT (no ASSERT_STRUCTURE, no REVIEW_SUPERSEDE)
[A1] outcome: PROMOTE_ACCEPTED problems: []
[A1] replacement consequenceType: EXECUTION_CONFIRMED
[A1] structural predecessor superseded: True
[A2] queue outcome: RETAIN_DRAFT | predecessor still in force: True
[A2] reviewer = advisor, grants REVIEW_ACCEPT / REVIEW_REJECT_OR_CONTEST / RECEIVE_READ_DATA (no REVIEW_SUPERSEDE)
[A2] accept outcome: PROMOTE_ACCEPTED problems: []
[A2] replacement consequenceType: EXECUTION_CONFIRMED
[A2] structural predecessor superseded: True
The mechanism is readable in one class. SupersessionValidator
(kernel/validators.py:300-329) checks four things and no more: the target resolves; its
record_kind is ofarm.acceptedeventconsequence.v0.1; it is anchored on this farm; it is not
already superseded. No class compatibility, no identity, no authority.
COMMIT_CLASS_TO_AUTHORITY_ACTION_CLASS (kernel/policy.py:40-41) then asks only for
ASSERT_OPERATION_CLAIM, and the retirement effect is never separately authorised. "Queuing alone
retired nothing; later acceptance did" also reproduces — [A2] shows the predecessor in force
after the queue and retired after the accept. The problem statement is accurate and the diagnosis
is right.
I also found a variant the audit does not name: an operation claim retires an observation
consequence just as readily.
[B3] OPERATION_CLAIM superseding an OBSERVATION consequence: PROMOTE_ACCEPTED | retired: True
C02/C03 would close it. Worth naming in the RFC's problem statement, because it shows K-02 is a
class of defect, not one pair.
Blockers
Blocker 1 — the closed matrix is narrower than CONTEST's target set, so this slice makes disputes against observation and compliance truth permanently unresolvable
The RFC states, twice and without qualification:
No correction relationship is supported here for observation, compliance, advisory, hypothesis,
note, evidence or arbitrary governance submissions. An explicit supersession pointer on those
classes refuses.
And in the same document:
D20/D21 preserve rejection, contest and the deferred standalone supersede verb.
Those two sentences are in conflict. D21 (DECISIONS.md:37) says the dispute
"resolves only by supersession — a governed CORRECTION that supersedes the disputed consequence
(→ CORRECTED) … never an edit, never a silent clear." And _validate_contest
(kernel/validators.py:466-507) gates a contest on exactly four things: the target is an
ofarm.acceptedeventconsequence.v0.1, farm-anchored, IN_FORCE, and not already disputed. There
is no class restriction. So every consequence class is contestable, while after this slice only
two classes remain correctable.
Both halves measured:
[B1] OBSERVATION consequenceType: STATE_CHANGE_ACCEPTED
[B1] CONTEST http: 200 RETAIN_DRAFT problems: []
[B1] DISPUTE edge attached: True
[B1] same-class CORRECTION today: PROMOTE_ACCEPTED problems: []
[B1] disputed predecessor retired: True
[B2] COMPLIANCE consequenceType: COMPLIANCE_STATUS_ACCEPTED
[B2] CONTEST http: 200 RETAIN_DRAFT problems: []
[B2] DISPUTE edge attached: True
[B2] same-class CORRECTION today: PROMOTE_ACCEPTED problems: []
[B2] disputed predecessor retired: True
An observation consequence and a compliance fact are each contestable and correctable today.
The RFC's matrix removes the second half and keeps the first. The result is a dispute that can be
opened and never closed: no edit, no clear, no correction — a permanent OPEN_DISPUTE on any
observation or compliance consequence anyone contests, for the lifetime of the record.
This is not a documentation nit. Both sides of it live inside this slice's own blast radius:
kernel/validators.py is the RFC's first named expected area and holds both
SupersessionValidator and _validate_contest. The refusal is a change this PR proposes; the
deadlock is its direct consequence.
Why the prior pass missed it. EXC-006 does consider dispute resolution — "Disabling every
correction would remove the accepted D18 and dispute-resolution consumers" — so the authors checked
that some correction survives. Nobody crossed the refused-class list against the set of classes
CONTEST admits as targets. The prior review's line "Exact consumed provenance distinguishes
observation from structure" is true about the discriminator and silent about the consequence.
Counter-argument, stated fairly. One could argue the deadlock is acceptable because a contest
against an observation is rare, and because the RFC scopes general graph repair to #184. Two
reasons that does not hold. First, the RFC does not claim the deadlock as an accepted cost — it
claims the opposite, that D20/D21 are preserved. An unrecorded regression in a governed
append-only ledger is worse than a recorded one. Second, the freeze path already treats an open
dispute as blocking (DISPUTE_OPEN refuses freeze_document_assembly when the disputed
consequence is in the document's basis), so the cost is not purely cosmetic. In fairness: in my
fixture the freeze did not refuse for either target, because neither consequence entered the
passport's basis — I could not demonstrate a live freeze block, and I am not claiming one. The
finding stands on the unresolvable dispute itself.
What closes it. Any one of these, decided explicitly:
- Narrow
_validate_contestin this same slice to exactly the classes correction still supports,
and add the pairing to C09 as an invariant ("the contestable set equals the correctable set")
with a test that fails if the two lists ever diverge again. This is the version I would take:
it makes the invariant mechanical instead of prose. - Keep same-class correction supported for every contestable class, and restrict only the
cross-class relationship — which is what K-02 actually is. - Accept the residual explicitly: record it as a named open decision, say which classes become
permanently disputable-but-uncorrectable, and confirm with the D21 owner that a dispute with no
resolution path is lawful. If this is the choice, it needs to be in the RFC text and in the
decision card, not discovered in Phase B.
Blocker 2 — Phase B cannot add a single C01–C09 test without editing a nodeid-pinned artifact the RFC's expected-areas list omits
The RFC's "Complete vertical slice and simplicity" section is written as a closed list:
Expected areas:
kernel/validators.py,kernel/stages.py,kernel/emission.py,
kernel/policy.py; focused existing semantic tests; fictional profile fixtures; this note and
review semantics documentation; the owned generated capability manifest if its action inventory
drifts. No frozen snapshot, contract schema, production route, storage role, migration,
credential, signing, audit or runtime-activation change belongs here.
conformance/review_baseline_test_inventory.json is not on that list. It must be.
Measured at the base commit: the inventory is schemaVersion
ofarm.review-baseline-test-inventory.v1, testRoot kernel/tests, holding 4,269 entries
with entryCount: 4269 and an entriesSha256 over the canonical entry bytes. Each entry is a
pytest nodeid — one row per test function, not per module:
kernel/tests/test_m2_review.py: 25 nodeids
kernel/tests/test_m2_identities.py: 19 nodeids
kernel/tests/test_conformance.py: 23 nodeids
kernel/tests/test_review_fixes.py: 16 nodeids
kernel/tests/test_stages.py: 10 nodeids
And it is enforced exactly, not advisorily. conformance/run_review_baseline.py:570-586 builds
expected_keys and actual_keys on (nodeid, sourceModule, sourcePath) and returns
"matches": not missing and not unexpected. One new test function is an unexpected entry and
the baseline comparison fails. That baseline is the RFC's own downstream gate — its verification
section requires "existing admitted locked two-run baseline/native evidence and publication
receipt".
So the RFC's EXC-003 ("C01–C09 trace to the resolver, retirement gate and focused tests") is
unreachable as written: the focused tests cannot pass the baseline the same section mandates
without regenerating a 4,269-row digest-pinned artifact the slice claims not to touch. The
capability manifest is correctly called out as a regenerated artifact — this one is the same kind
of thing and was missed.
It compounds. That same file is one of the two paths already failing
ofarm_profile_extraction_consistency_check.py at the base commit (measured above), and that
check is absent from the RFC's verification list. So Phase B will regenerate an artifact that is
already red in a check nobody runs, with no stated pass criterion — the implementer cannot tell a
pre-existing failure from a regression they caused. Generalising: cross the Phase B file
boundary against every already-red check, not only the ones the RFC chose to list.
What closes it. Add conformance/review_baseline_test_inventory.json to the expected areas
with the reason (nodeid-level, count- and digest-pinned, regenerated by
conformance/run_review_baseline.py); add the extraction consistency check to the verification
list with its base-commit state recorded as FAIL (2 failures) and the explicit criterion "no new
failures beyond these two"; and say which command regenerates the inventory so Phase B does not
hand-edit it.
Should fix
S1 — EXC-006 overclaims what the pair closes; a permitted reviewer can still retire an unrelated operation
EXC-006 argues:
adding only
REVIEW_SUPERSEDEis simpler but still lets a permitted reviewer retire an unrelated
class or identity
— and presents the relationship check as what closes that. For structure it does. For operations
it does not, because C03's discriminator is (subjectType, subjectRef), and distinct operations
share one subject:
[C1] A subject tuple: ('FIELD', 'field:demo.kmetija.a.gerk-1000001')
[C1] B subject tuple: ('FIELD', 'field:demo.kmetija.a.gerk-1000001')
[C1] identical: True
[C1] correcting an UNRELATED same-subject operation: PROMOTE_ACCEPTED problems: []
[C1] unrelated operation retired: True | the other untouched: True
A third spray naming an unrelated sibling's consequence as its predecessor is accepted today, and
C03 does not separate the two — same family, same result type, same subject. After this slice, a
reviewer holding REVIEW_SUPERSEDE can still retire an unrelated operation on the same field.
The RFC does concede this, once: "same subject is deliberately bounded compatibility, not proof of
a unique operation identity", with D5 deferring operation identity. That concession is correct and
I am not asking to widen the scope. I am asking that it stop being contradicted three sections
later. EXC-006 and the outcome statement read as if the class of defect is closed. State plainly,
in EXC-006 and in the problem/outcome section, that the residual "retire an unrelated operation on
the same subject" survives this slice and is bounded by D5 until operation identity lands. A reader
of the decision card should not have to reconcile the two passages themselves.
S2 — C06 names three of the four authority dimensions and leaves the call form unspecified, where the two existing accepting paths disagree
C06 requires the retirement grant be evaluated "using existing scope/time/revocation evaluation".
The Authority Action Matrix row this RFC itself cites has a fourth column:
| REVIEW_SUPERSEDE | govern/decide | assertion, consequence, state scope | NO_INHERIT by default | no by default | human-only by default | Changes what remains in force |
NO_INHERIT is covered ("fictional positive test actors receive explicit NO_INHERIT review
grants"). Human-only is never mentioned. And in this codebase human-only is not a property of
the action class — it is derived from the caller's arguments at
kernel/authority.py:298: non_human and action_stage in ("PROMOTION","PUBLICATION","ATTESTATION"),
where non_human = bool(acting_agent_ref) or party["partyClass"] == "SOFTWARE_AGENT".
The two existing accepting paths pass different arguments:
kernel/stages.py:325-334(AuthorityGate) passesaction_stage="PROMOTION"and
acting_agent_ref/ai_assistancefrom the submission.kernel/stages.py:675-678(direct self-reviewREVIEW_ACCEPT) passesaction_stage="PROMOTION"
and neither agent input.
The RFC says the shared retirement check "runs after ordinary review admission and immediately
before emission" and "serves both accepting paths" — so it will be written once, against one of
these two call forms, with no guidance on which. Written against line 675 it silently drops the
human-only dimension of the one action the matrix marks as changing what remains in force.
In fairness, there is no live bypass today, and I checked:
[D1] AI-assisted direct correction: REQUIRE_REVIEW ['HUMAN_APPROVAL_REQUIRED'] | retired: False
[D1] SOFTWARE_AGENT queued acceptance: REQUIRE_REVIEW ['HUMAN_APPROVAL_REQUIRED'] | retired: False
Both are stopped — by the earlier AuthorityGate, which the new check runs after. That is why
this is Should fix and not a Blocker: the protection exists, it just is not the protection C06
describes, and C06's evidence column has no non-human row. Name the call form in C06:
action_stage="PROMOTION", agent and AI-assistance inputs taken from the submission, and treat
non-ALLOW as refusal. On that last point the codebase is already on the RFC's side —
AuthorityDecision.allowed is outcome == "ALLOW" (kernel/authority.py:31-32), so an
implementer using .allowed fails closed on REQUIRE_HUMAN_APPROVAL and REQUIRE_REVIEW
automatically; one comparing against "DENY" does not. Worth one sentence, since it is the
difference between a correct check and a check that looks correct.
Follow-ups
F1 — the architecture check imposes no size or function-length discipline on any file this slice touches
The RFC says "No arbitrary line-count or new architecture-checker exception is needed." That is
true. It is worth recording why, because it is not restraint.
Parsed out of conformance/rewrite_architecture_check.py at the base commit:
MAX_FUNCTION_LINES: 80 | MAX_TEST_LINES: 800
kernel/validators.py: in a budget table = False
kernel/stages.py: in a budget table = False
kernel/emission.py: in a budget table = False
kernel/policy.py: in a budget table = False
kernel/gates.py: in a budget table = False
kernel/tests/test_m2_review.py: matched by a TEST_GLOB = False
kernel/tests/test_m2_identities.py: matched by a TEST_GLOB = False
kernel/tests/test_conformance.py: matched by a TEST_GLOB = False
kernel/tests/test_review_fixes.py: matched by a TEST_GLOB = False
kernel/tests/test_stages.py: matched by a TEST_GLOB = False
None of the five production modules appears in MODULE_BUDGETS, COMMAND_MODULE_BUDGETS, or any
GROUP_BUDGETS tuple; and _check_production — which is what enforces MAX_FUNCTION_LINES,
prohibited names, Any at trust interfaces, and environment reads — iterates over exactly those two
dicts (conformance/rewrite_architecture_check.py:5657-5667). No TEST_GLOBS pattern matches the
test modules C01–C09 will grow. Current sizes: validators.py 1381, stages.py 719,
emission.py 590, policy.py 380, gates.py 295.
Consequence: the whole capability sits in the checker's blind spot, so a green architecture check
after Phase B is evidence of nothing about this code's size or shape, and EXC-005 ("a helper earns
its place by serving direct and queued consumers now") has no mechanical backstop — the exact-head
content review is the only thing that will catch a resolver that sprawls. One sentence in the
verification section saying so would stop the next reviewer reading the green check as
confirmation.
F2 — "register the exercised action in the existing non-commit action inventory" gets no proof that it is exercised
The inventory is policy.NON_COMMIT_ACTION_CLASSES (kernel/policy.py:88-95), and its own comment
in kernel/manifest.py:296-298 claims the entries "ground in their evaluate() call sites via the
law-binding stage test". They do not. test_policy_action_classes_are_accepted_matrix_vocabulary
(kernel/tests/test_stages.py:27-38) parses the Action Matrix markdown and asserts only that every
runtime action class appears in it. REVIEW_SUPERSEDE is already row 19 of that matrix, so adding
it to the frozenset passes whether or not any code ever evaluates it. The comment is pre-existing
and not this PR's doing, but this PR is the first to rely on that inventory as a registration
mechanism. Either add a grounding assertion (each non-commit class appears at an evaluate(...)
call site) or fix the comment, so a future reviewer does not take manifest membership as evidence
that the action is enforced.
Checked and decided were not findings
Recorded so the next reviewer does not re-spend the time.
- Resolver chain field names are verbatim correct.
acceptedByReviewDecisionRef→
reviewedArtifactRef→ AssertionRecord resolves exactly as §"One owned decision path" describes,
andsourceEventRefis present on the consequence.[E1]. - "An accepted queued assertion remains immutable PENDING_REVIEW" is exactly right and
load-bearing. MeasuredPENDING_REVIEWboth before and after acceptance[E1]; re-review is
blocked by theREVIEWedge (kernel/validators.py:391), not by a state change. Without this,
C04's "previously queued accepted origins both work" would be unreachable. - "Do not require optional
resultingAcceptedConsequenceRefs, which current emitters do not
populate" is correct and necessary. Emitted ReviewDecision keys are exactly
anchorScopes, decidedAt, decidedByPartyRef, decisionOutcomeState, notes, reviewAction, reviewDecisionId, reviewedArtifactFamily, reviewedArtifactRef, schemaVersion[E1]. - "No supported correction is inferred from consequence type alone" is necessary, not
decorative. An observation consequence carriesconsequenceType: STATE_CHANGE_ACCEPTED,
identical to a structural one[B1]. C02's "observation masquerading as structure" row is a real
reachable attack, and the RFC is right to resolve it through the typedSTRUCTURE_PAYLOADrather
than the type string. - C02's "ambiguous current structure" is not reachable through ordinary ingress. A re-assertion
without a supersession pointer refusesCORRECTION_REQUIREDunder D18[E2], so that negative
case needs the corrupt graph fixture the RFC already says it needs — consistent with "corrupt
graph fixtures test refusal; they do not imply an ordinary caller can write raw graph edges". - C08 holds today at the existing validator level. Two independent
Storeobjects on separate
connections, released through a barrier: exactly onePROMOTE_ACCEPTED, oneRETAIN_DRAFT
SUPERSEDED_RECORD_USED, exactly oneLINEAGE_SUPERSEDESedge[E3]. C08 is reachable and
testable as written. - The ordering claim is accurate.
GatePipeline.commitwraps the entireCHAIN— AuthorityGate
included — instore.serialized_tx()(kernel/gates.py:117-132,kernel/store.py:297-312), which
takespg_advisory_xact_lock. So "takes its serialized transaction before authority and semantic
reads" is right, as is the honesty about what it does not promise. - "The legacy lock derives from the demo tenant configuration" is accurate.
_SINGLE_WRITER_LOCK_KEYissha256(config.TENANT_REF)[:8]at import
(kernel/store.py:50-51), andconfig.TENANT_REFis the hardcoded
"tenant:si.ffs.pilot.demo". Note in passing that the key ignores theStore's owntenant_ref
argument — that over-serializes rather than under-serializes, so it is safe, and it is not this
slice's business. - Production closure is real.
/commit,/review/accept,/review/reject,/review/contest
all raise 503GOVERNED_SURFACE_BLOCKEDon the production app (kernel/api.py:62-80). "A legacy
semantic defect, not a production HTTP bypass" is correct. recordClass=CORRECTIONis already refused.kernel/validators.py:765-769admits only
OPERATION_CLAIMandAS_APPLIED_EVIDENCE, thoughCORRECTIONis a shipped contract value
(contracts/core/OFARM_ExecutionRecordPayload_schema_v0_1.json:37). C09's carrier-refusal row
is a preservation, correctly described.- C06's "forged body reviewer" is already covered. A submission naming
reviewerPartyRefgets
HUMAN_APPROVAL_REQUIRED— "a reviewer named inside the submitter's request is forgeable and
never promotes" — and legacy/commitreturns 403ACTOR_BINDING_UNRESOLVEDon a transport
principal mismatch (kernel/legacy_m1/api.py:143-155). The negative test is worth keeping as a
regression, but it is not new ground. - The bundle-digest consequence is anticipated.
kernel/validators.pyis aVALIDATOR_SOURCE
runtime-bundle component and the generated capability manifest is theACTIVE_MANIFEST
component, so any edit here changes the bundle digest and every assertion queued before
deployment refuses at acceptance withPACK_CONFLICT(kernel/validators.py:369-379). C09's
"changed-bundle queued acceptance refuses while new correction of old accepted history remains
possible" already says this. Correct. - The expected-areas list is sufficient for the code.
gates.pyneeds no change: the stage
order lives there but the validator sequence isCOMMON_SEQUENCEin
kernel/validators.py:1214-1219, and the retirement check's stated position is inside
ReviewPromotionGate/ emission. (The list is still incomplete on the artifact side — Blocker 2.) - Prior-review claims spot-checked. "Queuing alone retired nothing" — confirmed
[A2].
"221-line RFC, documentation-only" — confirmed, 221 lines, one file. "Package contract check
passed with 0 failures including architecture constraints" — reproduced independently.
What my method made easier than production
- Wrong PostgreSQL major. 16.13, not the pinned 17.10. Anything version-specific — locale
provider behaviour, planner-dependent lock timing — would not surface. The advisory-lock and
READ COMMITTEDsemantics C08 depends on are not version-sensitive in the relevant way, but I
did not prove that on 17.10. - Not the locked baseline. x86_64 Linux with the repository's own dependency locks, but not
the three-cluster two-run baseline, no native evidence, no publication receipt. My results say
nothing about reproducible-build identity or BuildKit provenance. - Single process, in-process pipeline for six of ten probes.
[E3]'s competing writers are two
threads with separateStoreobjects and separate connections in one interpreter — genuinely two
connections contending for the advisory lock, but not two processes, and not two hosts. A real
cross-process race could expose ordering my barrier does not. - Demo tenant fixtures throughout. Every actor, grant and identity is the shipped fictional
demo set. The operation-only attacker is a party I inserted with exactly two grants — cleaner
than any real principal, which makes the K-02 reproduction easier than production, not harder.
That direction is the safe one for a Blocker about a missing check, and the wrong one for
reasoning about grants that overlap in practice. [C1]'s subject collision is a property of the demo fixture. Both sprays target
field:demo.kmetija.a.gerk-1000001because the demo submission builder hardcodes it. In a real
deployment operations on different fields would not collide. The finding survives — same-field
operations are the normal case, and C03's tuple cannot separate them — but the fixture makes the
collision universal where production makes it common.- I could not test the proposed behaviour. Nothing is implemented; every "after this slice"
statement above is derived from the RFC's text applied to measured current behaviour. Blocker 1
in particular is an inference about what the refusal list will do, grounded in two measured
facts (contestable today, correctable today) and one quoted rule (D21). If Phase B implements the
refusal more narrowly than the RFC's prose states, the deadlock may not materialise — which is
itself a reason to make the prose precise. - Freeze impact unproven. As noted in Blocker 1, neither disputed consequence entered the
passport basis in my fixture, so I did not observe aDISPUTE_OPENfreeze refusal for the
observation or compliance targets. I state the deadlock, not a measured freeze block.
Full transcript
Base ff092c414db9fa24dbd6ab86c7722db89e0c95b5; CPython 3.12.13; PostgreSQL 16.13; repository
dependency locks; probe module deleted after the run.
[A1] structural predecessor consequenceType: STATE_CHANGE_ACCEPTED
[A1] attacker grants: ASSERT_OPERATION_CLAIM, REVIEW_ACCEPT (no ASSERT_STRUCTURE, no REVIEW_SUPERSEDE)
[A1] outcome: PROMOTE_ACCEPTED problems: []
[A1] replacement consequenceType: EXECUTION_CONFIRMED
[A1] structural predecessor superseded: True
.
[A2] queue outcome: RETAIN_DRAFT | predecessor still in force: True
[A2] reviewer = advisor, grants REVIEW_ACCEPT / REVIEW_REJECT_OR_CONTEST / RECEIVE_READ_DATA (no REVIEW_SUPERSEDE)
[A2] accept outcome: PROMOTE_ACCEPTED problems: []
[A2] replacement consequenceType: EXECUTION_CONFIRMED
[A2] structural predecessor superseded: True
.
[B1] OBSERVATION consequenceType: STATE_CHANGE_ACCEPTED
[B1] CONTEST http: 200 RETAIN_DRAFT problems: []
[B1] DISPUTE edge attached: True
[B1] same-class CORRECTION today: PROMOTE_ACCEPTED problems: []
[B1] disputed predecessor retired: True
.
[B2] COMPLIANCE consequenceType: COMPLIANCE_STATUS_ACCEPTED
[B2] CONTEST http: 200 RETAIN_DRAFT problems: []
[B2] DISPUTE edge attached: True
[B2] same-class CORRECTION today: PROMOTE_ACCEPTED problems: []
[B2] disputed predecessor retired: True
.
[B3] OPERATION_CLAIM superseding an OBSERVATION consequence: PROMOTE_ACCEPTED | retired: True
.
[C1] A subject tuple: ('FIELD', 'field:demo.kmetija.a.gerk-1000001')
[C1] B subject tuple: ('FIELD', 'field:demo.kmetija.a.gerk-1000001')
[C1] identical: True
[C1] correcting an UNRELATED same-subject operation: PROMOTE_ACCEPTED problems: []
[C1] unrelated operation retired: True | the other untouched: True
.
[D1] AI-assisted direct correction: REQUIRE_REVIEW ['HUMAN_APPROVAL_REQUIRED'] | retired: False
[D1] SOFTWARE_AGENT queued acceptance: REQUIRE_REVIEW ['HUMAN_APPROVAL_REQUIRED'] | retired: False
.
[E1] claimState while queued: PENDING_REVIEW
[E1] claimState AFTER acceptance: PENDING_REVIEW
[E1] consequence.sourceEventRef present: True
[E1] acceptedByReviewDecisionRef -> reviewedArtifactRef == assertion: True
[E1] ReviewDecision keys: ['anchorScopes', 'decidedAt', 'decidedByPartyRef', 'decisionOutcomeState', 'notes', 'reviewAction', 'reviewDecisionId', 'reviewedArtifactFamily', 'reviewedArtifactRef', 'schemaVersion']
[E1] resultingAcceptedConsequenceRefs populated: False
.
[E2] first: PROMOTE_ACCEPTED | re-assert without supersedes: RETAIN_DRAFT ['CORRECTION_REQUIRED']
.
[E3] two independent Stores: {'B': ('PROMOTE_ACCEPTED', []), 'A': ('RETAIN_DRAFT', ['SUPERSEDED_RECORD_USED'])}
[E3] LINEAGE_SUPERSEDES edges against the predecessor: 1
.
10 passed, 1 warning in 4.74s
The one warning is the existing StarletteDeprecationWarning about httpx with
starlette.testclient, the same one the prior review records.
Final assessment
The problem is real, reproduced on both paths, and correctly diagnosed. The relationship-plus-
authority decomposition is the right shape, the provenance resolver is described accurately against
the code, and the honesty about what the transaction convention does not promise is a genuine
strength — I checked that paragraph against serialized_tx and it is precise.
Two things must close before the decision card. Blocker 1 is a governance regression this slice
introduces: it removes the only resolution path for disputes it still permits to be opened, while
the RFC's text asserts D21 is preserved. Blocker 2 is an unreachable verification plan: the
focused tests EXC-003 requires cannot pass the baseline the same section mandates without editing
an artifact the slice claims not to touch. Both are cheap to fix in the RFC text; Blocker 1 may
also warrant one extra check in _validate_contest, which is already in the slice's file list.
Not ready for the complete decision card. Re-review after the Blockers are addressed. This
review is by samovers, the PR author, and does not satisfy an independent-review gate.
Replace the unapproved version-1 proposal with version 2 for Delivery #379. Preserve D21 resolution for observation and compliance, clarify authority inputs and evidence limits, and keep shared-connection transaction ownership separate. Runtime implementation still awaits semantic approval.
|
Focused Phase A fix review completed at 8475a17, Delivery #379 / draft PR #380. Primary trust boundary: semantic promotion and supersession authorization. Scope stayed within it. Two coordinated AI reviewers assessed the B1 fix and affected authority/compatibility invariants, and the inventory/evidence/simplicity clarifications. The lead inspected both reports and confirmed the published design and Delivery match. Zero Blockers remain in the revised Phase A; no new Follow-ups or Preferences were identified. These are AI reviews posted through the same GitHub account, not independent external human approval. The earlier version-1 card and ready assessment remain withdrawn.
The design still uses one shared relationship check, one retirement check and one validated transient target; it adds no duplicate policy registry or durable correction state. The mandatory package contract check passed with 0 failures, including applicable architecture constraints, before the commit. Whitespace passed and the worktree is clean. The merged-base-to-head diff contains only the proposed 309-line RFC; no runtime implementation exists. No expensive baseline, admission, implementation approval or merge authority is supplied by this review. Version 2 changes the permitted correction set from the withdrawn proposal and requires its own complete same-task card and later exact user approval. Next: present the complete OFARM2-LEGACY-CORRECTION-AUTHORIZATION-001 version-2 decision card naming PR #380 before implementation. |
samovers
left a comment
samovers
left a comment
There was a problem hiding this comment.
PR #380 — re-review of Phase A, version 2
Verdict: zero demonstrated in-scope design blockers. Zero new follow-ups. Ready for the version-2 semantic-approval decision.
The existing shared-connection transaction-ownership follow-up remains open; documenting its limits does not fix it.
Reviewed head: 8475a176b4eb3334bdbd381dd4fb6dd83f82b7ea
Base: ff092c414db9fa24dbd6ab86c7722db89e0c95b5
The PR remains draft and documentation-only. Its sole changed file is the 309-line OFARM_Legacy_Correction_Authorization_RFC_v0_2.md. There is no runtime implementation or semantic approval at this head. This is a content/design review, not semantic approval, baseline admission, implementation acceptance, merge authorization or deployment authority.
1. The D21 contradiction is resolved in the design
My earlier version-1 review missed a valid blocker: refusing observation and compliance corrections while preserving their contests would remove the established resolution path for those disputes. That earlier assessment was wrong on this point. D21 makes supersession the resolution mechanism; it does not permit silently clearing a dispute. The later review identified the contradiction.
Version 2 now preserves compatible correction for all four existing emitting families:
| Source assertion family | Accepted consequence | Required relationship |
|---|---|---|
| Structure | STATE_CHANGE_ACCEPTED |
Same exact typed identity and its sole current structural predecessor |
| Operation claim | EXECUTION_CONFIRMED |
Same source family and exact subject |
| Observation | STATE_CHANGE_ACCEPTED |
Same source family and exact subject |
| Compliance | COMPLIANCE_STATUS_ACCEPTED |
Same source family and exact subject, with consistent compliance-carrier provenance |
This retains the necessary distinction between observation and structure despite their shared consequence type, without disabling observation corrections. It also leaves CONTEST and REJECT admission unchanged rather than trying to repair the contradiction by withdrawing another permission. See the RFC's C02/C03/C10 and existing policy mappings.
The added C10 is the right preservation test: each family must actually go through acceptance, contest, queued correction and authorized supersession. Merely registering a family in an enum would not prove that its dispute-resolution path survives. The verification section explicitly rejects that substitute.
2. Retirement authorization now has an executable contract
The revised design specifies the previously underdefined authority call: REVIEW_SUPERSEDE, stage PROMOTION, the transport-bound acting party, current farm scope, actingAgentRef, aiAssistance, and current revocation checking. Only exact ALLOW permits emission—not merely “anything other than DENY.”
That matches the existing evaluator's outcome model and preserves its human/agent distinction. It does not invent a new rule that AI-assistance metadata alone changes the accountable actor's classification. Both accepting paths must use the shared retirement check after ordinary review admission and before emission.
The capture-versus-effect separation also remains correct: an ordinarily authorized author may queue a compatible inert correction, but the actual accepting reviewer needs both acceptance and supersession authority. A body-named reviewer supplies neither. This is consistent with the pinned canonical Authority Action Matrix.
3. The expanded provenance checks remain bounded
The compliance addition distinguishes two real execution stages rather than requiring a nonexistent record: an incoming correction validates its submitted complianceClaim; an old accepted predecessor or queued correction resolves its persisted carrier through the event's COMPLIANCE_CLAIM edge. The latter must agree on record kind, source event, farm and subject, including the resolved identity type. No new observation carrier or governing-rule-set equality requirement is introduced. See the RFC's correction-proof rules and existing validation.
The design also preserves two important historical cases: an accepted queued assertion may still have immutable PENDING_REVIEW claim state, and a valid accepted predecessor may belong to an older RuntimeBundle. Neither is confused with the existing current-bundle requirement on a queued assertion being accepted now.
The same-subject limitation is now stated honestly. For nonstructural claims, matching subject and family does not prove that two records describe the same occurrence or proposition. A sufficiently authorized reviewer can still retire an explicitly selected different claim on that subject. The explicit predecessor selects the lineage; this slice does not solve deferred claim or operation identity. That is a disclosed boundary, not a newly discovered blocker.
4. The earlier concurrency follow-up is contained, not closed
C08 now expressly requires separate connections and disclaims same-app concurrent-request isolation. The ordering section states that atomicity claims assume exclusive connection ownership and acknowledges the legacy app's shared Store/connection. This addresses the overclaim identified in my earlier review.
The underlying distinction remains material: cursors sharing a Psycopg connection share its transaction. Separate-Store tests cannot establish independent transaction ownership for overlapping handlers using one connection. Keep that follow-up open under the transaction/runtime work; no fix is present in this documentation-only PR.
5. Verification planning is improved without waiving failures
The RFC now includes the generated test inventory and its regeneration command:
python conformance/run_review_baseline.py update-inventoryIt requires the pinned interpreter and dependencies, distinguishes collection from execution, and treats expected paths as scope predictions rather than an exhaustive approval list.
The two reported extraction-consistency failures remain explicitly failing evidence, concerning missing review records for conformance/review_baseline_test_inventory.json and kernel/tests/test_rewrite_architecture_check.py. The revised text does not turn “no new failures” into a replacement for a required PASS. Its conditional applicability is consistent with conformance/CONFORMANCE.md. This is not an all-checks-green disposition.
During this review I confirmed that the current-head lightweight workflow, run 34133781134, completed successfully and inspected its job-step results. That is documentation/package verification, not evidence that the correction implementation works.
Disposition and verification limits
No further Phase A changes required from this review. Proceed with the replacement decision card for OFARM2-LEGACY-CORRECTION-AUTHORIZATION-001, version 2. This remains an implementation/conformance RFC; no active baseline-law amendment is required.
I did not run local repository tests, rerun the extraction checker, or independently reproduce the HTTP/PostgreSQL scenarios in this pass. K-02 remains unclosed until the implementation and its own evidence are reviewed. Semantic approval, implementation acceptance and merge authorization remain separate.
This review is submitted as COMMENT, not approval. It creates no independent external human approval, baseline admission, merge authority, release or deployment authority.
Resolve one compatible predecessor with exact accepted provenance for direct submission and queued acceptance. Require the acting reviewer's explicit REVIEW_SUPERSEDE before retirement and retain its authority receipts. Preserve all four correction/dispute families, immutable lineage, replay, rollback, current-bundle queue admission, and existing transaction ownership. Use fictional fixtures and generated inventory for this same boundary. Delivery #379; PR #380; decision OFARM2-LEGACY-CORRECTION-AUTHORIZATION-001 v2. Primary boundary: semantic promotion and supersession authorization.
Exact-head implementation content review — PR #380 / Delivery #379Reviewed head: 2823561. Base: One coordinated implementation review covered provenance/compatibility and retirement/emission/evidence, followed by lead consolidation. Zero demonstrated Blockers; no new Follow-ups or Preferences. These are coordinated AI reviews, not independent external human approval. The existing transaction-ownership and extraction-record limitations remain open.
The reviewers independently checked the recorded local evidence. Aggregate across six JUnit runs and targeted reruns: 238 distinct cases with latest result PASS, zero unresolved test failures. Correction suite: 110 passed. Package contract (0 failures), Ruff, whitespace, applicable architecture/profile checks and generated manifest verification pass. This is supplemental Darwin ARM64/Python3.12.13/PostgreSQL17.10 evidence, not a single full suite or locked Linux baseline. The known Starlette/httpx warning remains recorded. Extraction consistency remains FAIL (two existing missing-review-record findings) for the baseline inventory and rewrite-architecture test. The extraction records/checker were not changed; baseline inventory regeneration is distinct from changing extraction-inventory consistency records. This disposition preserves the failure evidence and existing applicability rules; it grants no waiver. Same-task semantic approval remains bound to version 2 and PR #380: task No hosted result, publication receipt, merge readiness or deployment authority is claimed. Next: a separate exact-head admission comment may request the existing locked baseline and publication sequence; final packet and later exact-head human merge authorization remain required. |
|
Request the existing admitted baseline and publication sequence for PR #380 after its zero-Blocker exact-head implementation review: #380 (comment) . Implementation and cheap checks are complete. Scope remains semantic promotion and supersession authorization; this is technical baseline admission only, not human merge approval or deployment authority. OFARM2_BASELINE_ADMISSION |
|
Scoped implementation review after hosted failure — head The prior complete review at The new head changes only the two expected digest literals in Scoped reviewer and lead consolidation: zero Blockers at this full SHA. No new Follow-ups or Preferences; previously disclosed separate work remains. Semantic promotion and supersession authorization remains the only primary boundary, and EXC-001–006 are preserved. This two-line repair adds no runtime, activation, catalog or authority change and requires no new semantic decision under AGENTS.md. Cheap checks on the repair: both affected suites 223 passed, zero failures/errors/skips, using no database; mandatory package contract PASS, zero failures, including architecture; Ruff and whitespace PASS. Test collection did not change. Full failing hosted log, before/after JUnit and component diagnosis remain in the workspace evidence directory. This is an exact-head implementation review, not human merge authorization. Fresh baseline admission may now be created. No workflow rerun or earlier-head artifact will substitute for new-head evidence. Next: fresh exact-head admission, required hosted verification and trusted publication, then the later human final-acceptance packet. |
|
Admit the completed and reviewed correction implementation at the exact head below. Scoped fix review and lead consolidation report zero Blockers: #380 (comment). This is a fresh admitted source run after the prior head's two stale digest assertions were corrected and 223 focused tests plus package/architecture checks passed. No workflow attempt rerun, merge or deployment is authorized. OFARM2_BASELINE_ADMISSION |
|
PR #380 implementation verification completed at Source run 34149023176, attempt 1 passed both locked full baselines (4,379 tests each, one known warning each), clean-run equivalence, 23 platform tests, both native verifier jobs and the trusted handoff. Local correction/compatibility evidence, the 223-test digest repair, package/architecture checks and the exact-head zero-Blocker review remain recorded. These are executed results; collection alone is not presented as execution. Publisher 34151241570, attempt 1 authenticated the ticket and live admission but failed at The producer computes these digests from executed source in No publisher code, gates, selected manifest declaration, or workflow attempt was altered to clear the failure. Scope of PR #380 remains semantic promotion and supersession authorization. The earlier two stale digest assertions were repaired within that boundary and separately reviewed before fresh admission. Known extraction consistency FAIL (2) remains disclosed under its documented applicability; independent transaction/lifecycle/graph/K-03/K-04 work remains separate. Next: resolve the separate publication-custody prerequisite, then obtain fresh applicable admission/publication evidence before the final same-task exact-head merge-authorization packet. PR #380 remains draft and unmerged. |
samovers
left a comment
samovers
left a comment
There was a problem hiding this comment.
PR #380 — Constrain and authorize legacy correction supersession (implementation)
Independent exact-head content review at 64d3cbd60828c353bf284f2b7bbbfdbb6f5cd018, base
ff092c414db9fa24dbd6ab86c7722db89e0c95b5. Classified against the RFC's own C01–C10
matrix and EXC-001–006, and against the "Complete vertical slice" boundary statement.
Disposition: 1 Blocker (two independently measured facets), 1 Should fix, 1 Follow-up,
2 Preferences. The K-02 fix itself works — I reproduced the attack at this head on both
paths and it is refused, including when the attacker holds full REVIEW_SUPERSEDE. The
Blocker is not in the fix; it is in a check the shared helper applies to submissions that
have no correction relationship at all.
Position relative to the existing record
/pulls/380/reviews holds three review objects, all samovers, all at Phase A heads
(28a16d3, 8475a17). None is at the implementation head. The implementation reviews
exist as issue comments 5573898542 (head 2823561), 5574098553 and 5574394917 (head
64d3cbd), also all samovers, described as "coordinated AI specialist reviews and lead
consolidation", disposition zero Blockers. The Phase A re-review says plainly of itself:
"I did not run local repository tests… K-02 remains unclosed until the implementation and its
own evidence are reviewed."
This pass is also samovers. It does not satisfy an independent two-reviewer gate and
should not be recorded as one — say so in the admission packet rather than letting three
zero-Blocker self-reviews plus this one read as external clearance. I measured first and read
the prior comments afterwards. I reproduce their central claims and I disagree with the
zero-Blocker disposition: the finding below is invisible to the suite those comments cite,
and their hosted run (34149023176, 4,379 tests) would pass with it present — I confirm that
independently in the A/B below.
Method and environment
Everything below was executed. Fresh anonymous clone of samovers/OFARM2, git fetch origin refs/pull/380/head, worktrees at head and base, disposable databases per run.
- CPython 3.12.13, built from the
v3.12.13GitHub tag — the interpreter the
architecture check's fixed source-snapshot descriptor demands. - ruff 0.15.5 from
requirements-review-tools.lock; runtime deps from
requirements-review-baseline.lock. - Live PostgreSQL 16.13 on x86_64 Linux, real
Store, realGatePipeline, real legacy
FastAPI app viaTestClient— every probe goes through the real HTTP route.
Deviations, stated where they matter. This is PostgreSQL 16.13, not the pinned 17.10,
and x86_64 Linux, not Darwin ARM64 nor the locked three-cluster baseline. The kernel Store
does not enforce the 17.10 pin (only tenant provisioning does), so every result below is on
the code path under review, but a 17.10-specific behaviour would not appear here. I ran no
hosted workflow, verified no publication receipt, and did not reproduce the evidence-custody
failure in comment 5574394917 — that is outside this boundary and I take the author's
account of it at face value.
One environment trap worth recording. My first two full-suite runs used a 3.12.3 venv and
both died at ~53% with TypeError: 'NoneType' object is not callable inside pytest's
traceback formatter. That is the 3.12.3 artifact, not a repository failure. Everything below
was re-run on 3.12.13.
Authority chain, checked by digest before anything else
The approved Phase A head is 8475a17. Post-approval RFC diff:
$ git diff 8475a17 64d3cbd -- docs/rfcs/OFARM_Legacy_Correction_Authorization_RFC_v0_2.md
3 hunks, 21 lines changed
All three are status text and approval evidence: the status header, the "Verification and
approval" paragraph naming Codex task 01a07cc8-4157-7b33-a0ca-becb772e0e8b, and the "Next"
line. No C-row, no EXC entry, no boundary sentence moved after approval. The head RFC is
316 lines, sha256:6c31aea81d4c441e4ee802338a98fb97e365f1eeae85094f795edfa0333352a3.
git diff --check ff092c4 64d3cbd is clean.
Full change, from my own extraction:
| File | +/− |
|---|---|
conformance/review_baseline_test_inventory.json |
552/2 |
kernel/tests/test_correction_authorization.py |
796/0 |
kernel/tests/test_correction_transactions.py |
248/0 |
docs/rfcs/…_RFC_v0_2.md |
316/0 |
kernel/validators.py |
218/111 |
docs/REVIEW_DISPUTE_SEMANTICS.md |
41/5 |
kernel/stages.py |
37/0 |
kernel/manifest.py |
9/9 |
kernel/policy.py |
7/6 |
profile_si_ffs/test_fixtures/demo_records.py |
7/7 |
kernel/emission.py |
6/7 |
profile_si_ffs/OFARM_Capability_Manifest_si_ffs_pilot_v0_1.json |
3/2 |
kernel/tests/test_profile_selection_validation.py, test_runtime_bundle.py |
1/1 each |
K-02 at this head, on the real HTTP path
Attacker holds ASSERT_OPERATION_CLAIM + REVIEW_ACCEPT, no ASSERT_STRUCTURE, and
targets an accepted structural consequence — the audit scenario verbatim. I ran the same
attack a second time with REVIEW_SUPERSEDE added, to separate the relationship defence
from the authority defence:
[P1 nosup/direct] RETAIN_DRAFT ['CORRECTION_REQUIRED'] predecessor_superseded=False truth_unchanged=True
[P1 nosup/queued] RETAIN_DRAFT ['CORRECTION_REQUIRED'] predecessor_superseded=False truth_unchanged=True
[P1 sup/direct] RETAIN_DRAFT ['CORRECTION_REQUIRED'] predecessor_superseded=False truth_unchanged=True
[P1 sup/queued] RETAIN_DRAFT ['CORRECTION_REQUIRED'] predecessor_superseded=False truth_unchanged=True
K-02 is closed on both paths, at the relationship stage, before the authority question is
even asked. Full retirement authority does not buy the cross-family retirement. That is the
decomposition the RFC promised and it holds.
Blocker 1 — the correction proof's farm-anchor check runs on ordinary acceptance, and permanently strands claims and truth that carry no correction at all
Where. kernel/validators.py:217, the second line of _assertion_event:
def _assertion_event(ctx: GateContext, assertion: dict) -> dict:
_correction_farm(ctx, assertion)
event_ref = _correction_edge(ctx, assertion["assertionRecordId"], "EVENT_SOURCE")
event = _correction_record(ctx, event_ref, "ofarm.semanticeventenvelope.v0.1")
_correction_farm(ctx, event) # <-- line 217_correction_farm (:206) requires the literal entry {"scopeType": "FARM", "scopeRef": ctx.farm_ref} to be in the record's anchorScopes.
That invariant is true of every record the proof otherwise touches — the AssertionRecord
(emission.py:54), the AcceptedEventConsequence (emission.py:207) and the ReviewDecision
all hard-code anchorScopes to exactly the farm entry. It is not true of the
SemanticEventEnvelope. Its anchorScopes is the caller's targetScopes
(kernel/stages.py:225-235):
scopes = sub.get("targetScopes") or [{"scopeType": "FARM", "scopeRef": ctx.farm_ref}]targetScopes is a required field of contracts/kernel/OFARM_CommitIngressRequest_schema_v0_1.json
(required: [… 'targetScopes', 'idempotencyKey'], minItems: 1) with a nine-value
scopeType enum — FARM, SITE, FIELD, ZONE, CROP_CYCLE, LOT, FACILITY, OPERATION, DEPLOYMENT, TENANT. ScopeContainmentValidator (validators.py:456-460) checks each entry
is contained in the farm; it deliberately does not require the farm entry itself. So a
submission naming only the field it concerns is a first-class, contract-shaped, validator-
approved input. CommitBody.submission is a bare dict (legacy_m1/api.py:32), so it
reaches the pipeline unfiltered through the public /commit route with no special privilege.
_assertion_event is called from two places (:313 inside _validate_correction, :574
inside GovernanceAcceptanceValidator), and the second one runs unconditionally on every
non-reject acceptance, correction or not.
Facet 1 — an ordinary queued claim becomes permanently unpromotable
Same submission, same fixture, same reviewer, head vs base. All four families, targetScopes = [{"scopeType": "FIELD", "scopeRef": demo.FIELD}], no supersedesConsequenceRef anywhere:
################ HEAD 64d3cbd ################
[STRUCTURE_ASSERTION ] QUEUE -> RETAIN_DRAFT ACCEPT -> RETAIN_DRAFT ['SCOPE_NOT_AUTHORIZED'] conseq=0
[OPERATION_CLAIM ] QUEUE -> RETAIN_DRAFT ACCEPT -> RETAIN_DRAFT ['SCOPE_NOT_AUTHORIZED'] conseq=0
[OBSERVATION_ASSERTION ] QUEUE -> RETAIN_DRAFT ACCEPT -> RETAIN_DRAFT ['SCOPE_NOT_AUTHORIZED'] conseq=0
[COMPLIANCE_ASSERTION ] QUEUE -> RETAIN_DRAFT ACCEPT -> RETAIN_DRAFT ['SCOPE_NOT_AUTHORIZED'] conseq=0
[STRUCTURE_ASSERTION ] REJECT -> RETAIN_DRAFT [] <- reject still works
[OBSERVATION_ASSERTION ] DIRECT -> PROMOTE_ACCEPTED [] conseq=1 <- self-review still promotes
################ BASE ff092c4 ################
[STRUCTURE_ASSERTION ] QUEUE -> RETAIN_DRAFT ACCEPT -> PROMOTE_ACCEPTED [] conseq=1
[OPERATION_CLAIM ] QUEUE -> RETAIN_DRAFT ACCEPT -> PROMOTE_ACCEPTED [] conseq=1
[OBSERVATION_ASSERTION ] QUEUE -> RETAIN_DRAFT ACCEPT -> PROMOTE_ACCEPTED [] conseq=1
[COMPLIANCE_ASSERTION ] QUEUE -> RETAIN_DRAFT ACCEPT -> PROMOTE_ACCEPTED [] conseq=1
The claim commits cleanly, sits in the advisor queue, and no reviewer with any authority
can ever accept it — the refusal is a property of the stored event, not of the reviewer.
REJECT still works, so the claim can be closed but never promoted. The same submission
promotes normally if the author self-reviews it; it is stranded only if it goes to a distinct
reviewer. That asymmetry is the shape of the defect.
Facet 2 — accepted truth minted through the direct path becomes uncorrectable
Because the direct path still promotes, this head can create accepted consequences whose
source event carries no farm entry. _validate_correction then calls _assertion_event on
that origin (:313) and refuses:
############ HEAD 64d3cbd ############
[field-only] accepted consequence created directly: conseq:25c2f7c0732b4e9e
[field-only] CORRECT -> RETAIN_DRAFT ['SCOPE_NOT_AUTHORIZED'] old_superseded=False
[farm] CORRECT -> PROMOTE_ACCEPTED [] old_superseded=True
############ BASE ff092c4 ############
[field-only] CORRECT -> PROMOTE_ACCEPTED [] old_superseded=True
[farm] CORRECT -> PROMOTE_ACCEPTED [] old_superseded=True
In-force accepted truth that no correction can ever retire. D21 makes a governed superseding
correction the only resolution for a contest, so a dispute opened against such a
consequence has no exit. This is the same failure mode as version 1's Blocker 1 — the one
version 2 was written to remove — arriving through a different door. Version 1 closed the
resolution path by refusing whole families; version 2 closes it for any consequence whose
event does not literally name the farm.
Classification
Against the RFC's own matrix this is C10: "Every currently supported emitted assertion
family retains its lawful acceptance → contest → compatible same-family correction path under
existing review/evidence rules and the new retirement authority." Measured, none of the four
does, for this class of input. It also contradicts the slice's own boundary sentence
("Preserve existing gates") and EXC-005's "a helper earns its place by serving direct and
queued consumers now" — the helper is serving a consumer it was not designed for.
The counter-argument, and why I do not think it holds
One could argue no shipped caller sends non-farm targetScopes: the demo fixtures and
profiles/si_ffs/outputs.py:398 all send the farm entry, and sub.get("targetScopes") or [FARM] defaults for anyone who omits it. I do not think that survives. targetScopes is
contract-required with an enum built around non-farm scopes; ScopeContainmentValidator
exists specifically to admit them; kernel/tests/test_profile_runtime_routes.py:105,149
already drives non-farm targetScopes. And the failure is silent by construction — a
RETAIN_DRAFT, not an error — so in service it would present as claims that inexplicably
never promote, with the reason text "Correction relationship refused" on a submission that
contains no correction. If the position really is "non-farm targetScopes is unsupported",
that belongs in ScopeContainmentValidator as an explicit refusal at submission, not as an
unpromotable claim discovered at review time.
Cheap fix
The farm anchor this proof needs is already carried by the assertion, the consequence and the
review decision. The event's anchorScopes are caller scopes, already containment-checked at
submission, and are the wrong record to demand it from. Two changes, both inside the slice's
own file list:
- Drop
_correction_farm(ctx, event)at:217(or make it a containment test rather than a
literal-membership test). Facet 2 closes with this alone. - Split the helper.
GovernanceAcceptanceValidatoronly needs the event identity — the
exactly-one-EVENT_SOURCE-edge rule that EXC-004 was actually about — to feed
ctx.acceptance_event_ref. The family/subject/anchor proof belongs on the correction path
only. That closes facet 1 and removes the misleading diagnostics at the same time.
Should fix 1 — the shared proof is imposed on non-correction promotion, past the stated boundary
Even with the anchor check fixed, GovernanceAcceptanceValidator:570-578 runs the full
_assertion_event on every acceptance: exactly one EVENT_SOURCE edge, exact envelope kind,
primaryEventFamily agreement, and event["subjectRefs"] == [assertion["subject"]["subjectRef"]].
Any of those failing refuses an ordinary promotion with _correction_refusal, whose title is
literally "Correction relationship refused".
The declared primary trust boundary is semantic promotion and supersession authorization,
and C05 asks acceptance to revalidate persisted correction intent. Applying correction-grade
provenance proof to every queued claim is a wider behaviour change than the RFC authorizes,
and it is the mechanism that turned Blocker 1 from a correction-path bug into a
general-promotion outage. Reason codes on a non-correction refusal should also not say
"correction".
I checked the sub-conditions individually and the other three are currently unreachable:
_link_assertion writes exactly one EVENT_SOURCE edge; COMMIT_CLASS_TO_ASSERTION_TYPE is
injective and the family map agrees by construction; and the subjectRefs divergence noted
under non-findings is refused earlier. Only the anchor condition bites today. The Should fix
is about the coupling, not about a second live defect.
Follow-up 1 — the declared D5 residual, measured
The RFC is explicit that same-subject compatibility "is deliberately bounded compatibility,
not proof of a unique operation or claim identity". I measured what that buys an attacker
after the slice lands. Two distinct spray operations, different executionRecordPayloadId,
different eventTime, sharing the demo fixture's single (FIELD, demo.FIELD) subject tuple:
[P2 queue] RETAIN_DRAFT []
[P2] PROMOTE_ACCEPTED old_superseded=True
old_erp=erp:correction-test.5ed6e1ac20ba new_erp=erp:correction-test.3e84491ff4ba
An actor holding ASSERT_OPERATION_CLAIM + REVIEW_ACCEPT + REVIEW_SUPERSEDE can still
retire any in-force operation consequence on that subject by asserting an unrelated
operation against it. This is in-boundary and declared, so it is not a defect. But the
decision card should carry it in this measured form rather than as the prose "same subject
does not prove a unique operation": K-02 narrows from cross-family retirement to
within-family, same-subject retirement, and the demo profile gives every distinct field
operation the same subject tuple. Naming it concretely is what stops the next slice from
assuming operation identity is already solved.
Preferences
P1 — next() reverse lookup at validators.py:218. EXC-004's whole point is removing
"first of several" selections, and _assertion_event introduces one:
commit_class = next((name for name, family in policy.COMMIT_CLASS_TO_ASSERTION_TYPE.items()
if family == assertion.get("assertionType")), None)I confirmed the map is injective today (4 entries, 4 distinct values), so this is
deterministic. But its determinism is a property of the current data, not of the code, in a
function whose sibling lines refuse rather than pick when a relationship is ambiguous. A
module-level inverted dict built once, or an explicit uniqueness assertion, would make it
match its own file's standard.
P2 — the fixture now grants REVIEW_SUPERSEDE to FARMER and ADVISOR by default.
demo_records.py adds it to both standing grants. That is the right call for the RFC's
positive controls, and both remain NO_INHERIT farm-scoped as required. The consequence
worth naming: every pre-existing correction test now has retirement authority for free, so
nothing outside the two new modules can detect the retirement check being dropped. Measured —
the only mention of REVIEW_SUPERSEDE in any other test file is
test_m2_review.py:242, the standalone-verb refusal. My neutralization matrix below confirms
it: disabling _authorize_retirement breaks 21 tests, all of them in the two new modules.
Calibration — are the new tests load-bearing?
110 new tests that pass prove nothing on their own. I neutralized each central mechanism at
head, one at a time, and re-ran both new modules:
| Mutation | Result |
|---|---|
N1 — _authorize_retirement returns GatePass() immediately |
21 failed, 89 passed |
N2 — drop the family + consequence-type equality in _validate_correction |
7 failed, 103 passed |
N3 — drop the old_subject != new_subject check |
4 failed, 106 passed |
N4 — restore the first-edge fallback in _correction_edge |
6 failed, 104 passed |
All four are load-bearing; none of the mechanisms is decorative. Two details worth recording:
- Under N3,
test_full_authority_does_not_allow_different_subject_or_identity[STRUCTURE_ASSERTION]
still passes — structure is independently caught by_check_structure_current. The subject
check is the sole guard only for the three non-structural families. - Under N1, the
software-partyanddeclared-agentparameters of
test_correction_preserves_existing_human_and_assistance_semanticsstill pass: those actors
are already refused by the ordinary admission gate before the retirement gate is reached, so
they discriminate nothing about the new check. The test's own comment says exactly this
("The ordinary admission gate already owns this refusal. Do not weaken it merely to force a
nonhuman actor as far as the retirement gate."). That is honest test-writing and I record it
as credit, not a finding. Only theassistance-onlyparameters exercise the new path, and
they do so through the receipt assertion rather than a refusal.
Mechanical companions, reproduced independently
Suite A/B, head vs base, one process each, CPython 3.12.13, live PG 16.13:
head 64d3cbd : 3926 passed, 345 skipped, 4 failed, 104 errors, 1 warning (435s)
base ff092c4 : 3816 passed, 345 skipped, 4 failed, 104 errors, 1 warning (365s)
+110 passed, and the failed/error name sets are byte-identical between head and base
(diff of the sorted 108 FAILED/ERROR lines is empty). All 108 are my harness: 104
RuntimeError: OFARM_TENANT_PROVISIONING_PG_ADMIN_DSN must identify a dedicated PostgreSQL 17 service plus the four test_postgresql_provisioning.py cases. No head-only failure. The
one warning is the existing StarletteDeprecationWarning. The two new modules alone:
110 passed in 67s.
Review baseline inventory. Not hand-edited — I regenerated it:
$ python conformance/run_review_baseline.py update-inventory
wrote conformance/review_baseline_test_inventory.json with 4379 pinned tests
$ diff committed regenerated -> identical
sha256 1c61e9f03f64ec211410fe5191af4ac838282899be2ea4a38d2018da3bffd67c
Nodeid-set diff against base: +110 added, 0 removed, 104 from
test_correction_authorization.py and 6 from test_correction_transactions.py. entryCount
4,269 → 4,379. The arithmetic closes exactly against the 110 collected tests.
Capability manifest. The diff is two lines — publishedAt and one new
REVIEW_SUPERSEDE entry in supportedActionClasses. That is a faithful regeneration of what
build_manifest produces from the amended NON_COMMIT_ACTION_CLASSES; nothing else drifted.
Gates at head, CPython 3.12.13 + ruff 0.15.5:
| Check | Head | Base |
|---|---|---|
conformance/ofarm_pkg_contract_check.py |
PASS (0 failures) | — |
conformance/rewrite_architecture_check.py |
PASS | — |
conformance/ofarm_profile_extraction_consistency_check.py |
FAIL (2) | FAIL (2) |
ruff check . (repo-wide) |
2 errors | 2 errors |
The two extraction failures are the identical pre-existing missing-review-record paths
(conformance/review_baseline_test_inventory.json,
kernel/tests/test_rewrite_architecture_check.py) at both heads. The RFC's disclosure of
this is accurate and it does not substitute "no new failures" for a PASS. Ruff is unchanged.
What the green architecture check does not say. kernel/validators.py,
stages.py, emission.py, policy.py and manifest.py appear in no MODULE_BUDGETS,
COMMAND_MODULE_BUDGETS or GROUP_BUDGETS entry, and no TEST_GLOBS pattern matches either
new test module. _check_production iterates only those dicts, so MAX_FUNCTION_LINES = 80
never reaches this change. The RFC's "no arbitrary line-count exception is needed" is true
because the checker is silent here, not because the code is small — worth stating plainly in
the change report. For the record, measured directly: every new helper is 3–47 lines;
GovernanceAcceptanceValidator.run went 104 → 107 and ReviewPromotionGate.run 103 → 109,
both already over 80 at base; and StructureSemanticsValidator.run shrank from 128 to
under 80. Net, the change improves this file rather than worsening it.
Checked and decided were not findings
So the next reviewer does not re-spend this:
- Post-approval RFC edits — 3 hunks, 21 lines, status and approval evidence only. No
ceiling, allowlist, C-row or EXC entry moved after8475a17. NON_COMMIT_ACTION_CLASSESgainingREVIEW_SUPERSEDEgrants nothing. Its only
consumers aremanifest.py:168and:298.REVIEW_ACTION_AUTHORITYstill omits it, so the
standalone verb still default-denies.- A denied retirement does not poison the queued claim. After a
DENY
(AUTHORITY_DENIED, predecessor untouched), a properly granted reviewer accepts the same
assertion and the correction completes:PROMOTE_ACCEPTED,old_superseded=True. - Idempotent replay is fail-closed. Replaying a denied acceptance under the same
idempotencyKeyafter the grant lands returnsREPLAY_REUSED_RESULTwith the cached
AUTHORITY_DENIED, not a late promotion. A fresh key promotes. This matches the RFC's
replay statement and errs in the safe direction. - A refused retirement still writes its evidence — the
REVIEW_SUPERSEDE
authorizationdecisionrequest/result/tracetriple and aPromotionTraceare
persisted on refusal, not only on success. - The
subjectRefsderivation divergence.stages.py:235builds
[sub.get("subjectRef") or ctx.farm_ref]while_build_assertionuses
sub.get("subjectRef", ctx.farm_ref)— these disagree for a present-but-falsy
subjectRef, which_assertion_event:222would then refuse. Unreachable:
ScopeContainmentValidatorrefuses""andnullwithSCOPE_NOT_AUTHORIZEDat both
heads before an assertion is emitted. Latent, not live. _structure_identity's typed-payload requirement adds no new refusal —
StructureCarrierValidator:746already restricts carriers to
STRUCTURE_PAYLOAD_IDENTITY_TYPE.- CONTEST is unaffected by the acceptance changes:
GovernanceAcceptanceValidator:508
returns into_validate_contestbeforeis_rejectis computed, so the new block never runs
for a contest. C09's "CONTEST and REJECT admission remain unchanged" holds. - Both
LINEAGE_SUPERSEDESwrites (emission.py:224,:301) and the
LINEAGE_SUPERSEDES_INTENTwrite (:166) now readctx.correction_predecessor_ref, and
both accepting emitters sit behind_authorize_retirement(stages.py:672,:723). There
is no emission path to a retirement edge that skips the authority check — I traced all three
call sites, not just the ones the tests drive.
What my method made easier than production
- PostgreSQL 16.13, not the pinned 17.10, and x86_64 Linux, not the locked baseline or the
Darwin ARM64 supplemental run. A 17.10-specific behaviour would not appear in anything above.
The 104 provisioning errors are exactly that gap showing itself. TestClient, not a network deployment. Every probe shares one process and oneStore.
The RFC is candid that the legacy app shares a connection across handlers and that
separate-Storeevidence does not prove request-level isolation; I did not improve on that,
and my concurrency reading is entirely the PR's owntest_correction_transactions.py
neutralized, not an independent race harness.- My oracle for "ordinary traffic" is the demo fixture plus the ingress contract. I showed
that a contract-shaped, validator-approvedtargetScopesvalue breaks; I did not survey
every real client to show one sends it. If the SI profile provably never emits non-farm
targetScopes, Blocker 1's reachability narrows — its existence does not, and facet 2's
uncorrectable-truth outcome would still be reachable by anyone using the documented API. - I ran no hosted workflow and verified no publication receipt. The evidence-custody
failure, the native verifier jobs, the two-run equivalence and the trusted handoff are all
taken from the author's comments, not measured. - My contest probe did not open a dispute (
RETAIN_DRAFT, no problems) — likely a wrong
body shape on my side. Both heads behaved identically, so the correction comparison stands,
but I have no first-hand measurement of D21 resolution end-to-end; my facet-2 argument rests
on the correction refusal plus D21's text, not on a stranded dispute I produced.
Final assessment
The core of this change is right and I could not break it. The relationship/authority
decomposition does what it claims: K-02 is refused on both paths at the relationship stage,
full REVIEW_SUPERSEDE does not buy a cross-family retirement, the retirement gate precedes
both emitters with no bypass, every non-ALLOW outcome refuses, and the four neutralizations
show the tests actually pin all of it. The mechanical companions — inventory, manifest, gates,
ruff, suite A/B — are exactly what they claim to be; I reproduced each from scratch and found
no drift, no hand-edit, and no head-only test failure.
The Blocker is a scope leak, not a flaw in the fix. A helper written to prove a correction
relationship is being run on submissions that are not corrections, and one of its conditions
is false for a legitimate, contract-required input shape. The result is claims that can never
be promoted and accepted truth that can never be corrected — and by D21, disputes that can
never be resolved. That is the same class of harm version 2 was written to prevent, which is
why I hold it as a Blocker rather than a Should fix, and why I do not think a green 4,379-test
run rebuts it: nothing in that suite drives a non-farm targetScopes through queued
acceptance.
Both fixes are small and sit inside the slice's own file list. I would re-review at the next
head.
Not ready for baseline admission or the final packet. Re-review after Blocker 1 is closed.
This review is by samovers, the PR author, and does not satisfy an independent-review gate;
neither do the three prior self-reviews on this PR.
samovers
left a comment
samovers
left a comment
There was a problem hiding this comment.
PR #380 — implementation re-review
Verdict: changes required. Two in-scope implementation blockers, plus the existing separate publication hold.
Reviewed head: 64d3cbd60828c353bf284f2b7bbbfdbb6f5cd018
Base: ff092c414db9fa24dbd6ab86c7722db89e0c95b5
The current head was rechecked before posting and still matches the reviewed commit. This review concerns the implementation across 14 changed files, not the earlier design-only version. The PR remains draft and unmerged. Submitted as a COMMENT review; the content disposition is changes required, not approval, baseline admission, implementation acceptance, merge authorization or deployment authority.
B1 — Malformed structural schema identifiers escape governed refusal
Priority: P2 — must fix.
Location: kernel/validators.py:227–231, _structure_identity().
The new correction proof looks up the caller's payload.schemaVersion in a dictionary before checking that it is a string:
identity_type = policy.STRUCTURE_PAYLOAD_IDENTITY_TYPE.get(
payload.get("schemaVersion")
)A JSON array or object therefore raises TypeError instead of _CorrectionProofError. The surrounding correction validator catches only _CorrectionProofError. This occurs before the ordinary structural-carrier validator gets its opportunity to reject the malformed payload. See SupersessionValidator and validation sequencing.
The reachable input is an otherwise usable STRUCTURE_ASSERTION submission naming a valid structural predecessor, with either of these payload values:
"schemaVersion": []"schemaVersion": {}I executed the fetched function body in an isolated probe. Results:
schemaVersion=[] → TypeError: unhashable type: 'list'
schemaVersion={} → TypeError: unhashable type: 'dict'
schemaVersion=null or an unknown string → _CorrectionProofError
Impact: this does not authorize retirement, but it breaks the governed-refusal contract. Source tracing shows the exception escaping the legacy /commit handler and rolling back the transaction instead of returning a RuntimeProblem with its durable refusal trace. The handler catches ContractViolation and KeyError, not this TypeError; the pipeline writes its refusal trace when a stage returns GateRefusal.
Smallest fix: check the schema identifier's type before the dictionary lookup and raise _CorrectionProofError for unsupported shapes. Do not add a broad exception catch around the pipeline.
Add HTTP/pipeline regressions using a valid predecessor and array/object schema identifiers. Assert a governed refusal, a persisted trace, no successor or retirement edge, and an unchanged predecessor.
B2 — The new event check rejects valid field-anchored captures, including non-corrections
Priority: P2 — must fix.
Locations: kernel/validators.py:212–224, _assertion_event(), and its unconditional acceptance call at approximately line 574.
Existing ingress preserves explicit targetScopes as the event's anchorScopes. Thus a submission targeting an existing field may legitimately produce:
"anchorScopes": [
{"scopeType": "FIELD", "scopeRef": "field:example"}
]The existing scope validator establishes farm containment by resolving that field's IdentityRecord and checking its farm anchor. It does not require the event to repeat a literal FARM scope. Meanwhile, the emitted assertion is explicitly farm-anchored. These are different representations of containment, both produced by the existing path. Sources: ingress normalization, _assert_contained / ScopeContainmentValidator, and assertion emission.
The new _assertion_event() calls _correction_farm(ctx, event), which accepts only literal membership of the farm scope in the event's anchors. It does not resolve the field's containment. More importantly, GovernanceAcceptanceValidator now invokes this helper for every acceptance before checking whether any correction intent exists.
Consequently, an otherwise valid field-targeted capture can enter the queue but then be refused at ordinary acceptance with:
SCOPE_NOT_AUTHORIZED
Correction provenance is not anchored on this farm
This is not merely stricter validation of a malformed correction: it changes non-correction acceptance behavior. It also prevents otherwise compatible corrections from using accepted historical predecessors whose original events have field-only anchors. That conflicts with the intended preservation of ordinary behavior and usable accepted history. See the acceptance implementation and version-2 RFC.
The isolated event-resolution probe confirms the discriminator: a field-only event fails, while the same event passes when a literal farm anchor is included. Its fixture contains a field identity anchored on the correct farm.
Smallest fix: validate source-event farm containment using the existing containment semantics, while retaining the unique-edge, exact-kind, subject and scalar-reference checks. Do not require rewriting immutable historical events or remove source validation altogether.
Add a preservation test that queues and accepts an ordinary observation or operation with explicit field-only targetScopes, plus a compatible correction whose predecessor originated from that shape. Include cross-farm negative controls so the repair does not weaken isolation.
What remains sound
The central authorization change is still appropriately placed: one checked predecessor feeds the emitters, and both accepting paths perform the separate REVIEW_SUPERSEDE evaluation before retirement. The tests distinguish compatibility failures from missing authority, exercise the four-family dispute-resolution paths, and retain rejection without retirement. The transaction tests explicitly use separate connections and observe the advisory-lock wait rather than relying on thread timing. These strengths do not cover the two input/representation cases above.
Sources: retirement gate, correction authorization tests, and correction transaction tests.
Verification and publication
The hosted conformance log inspected during this review shows 4,379 passing tests in each complete run, successful clean-run equivalence, generated-manifest verification, and 23 passing root platform tests. Those results establish the existing suite's outcome; they do not invalidate findings for inputs the suite does not exercise.
The separate publisher failed at rebuilding authoritative conformance claims; its authoritative uploads and final-receipt step were skipped. Publication remains a separate acceptance prerequisite, not something to fix by weakening verification or removing the legitimate manifest change in this PR.
Evidence limits: the new probes executed copied function bodies with a fake Store on Python 3.13.5. They were not full repository tests, live HTTP/PostgreSQL reproductions, or the pinned Python 3.12.13 baseline. The end-to-end consequences above are source-traced. The supplemental probe source and JSON results were retained in the originating chat; they are not repository or hosted-baseline evidence. I did not rerun the full repository suite, reproduce the publisher's custody failure, or independently authenticate a final publication receipt.
Required disposition
Fix B1 and B2 in the owning validator and focused tests, regenerate affected mechanical artifacts, and obtain review and applicable evidence for the new head. These are implementation/conformance corrections; no active baseline-law amendment or architecture rewrite is needed. Keep the previously identified shared-connection follow-up and the publication-custody prerequisite separate.
Do not carry forward the current zero-blocker implementation assessment.
Validate structural schema identifiers before lookup and apply the existing containment rule to every source-event anchor. Preserve single refusal receipts, immutable field-scoped history, and the existing correction provenance and retirement checks. Add 26 HTTP regression cases, regenerate the test inventory, and refresh the two aggregate bundle digest expectations. Package contract PASS; correction and compatibility 221 PASS; bundle suites 223 PASS. Delivery #379, PR #380.
|
Scoped B1/B2 repair review — exact head Both demonstrated blockers in reviews 5140352486 and 5141739268 are fixed:
Two bounded AI reviews checked only the repair and affected invariants, then confirmed identical committed bytes at the full SHA above. Blockers: 0. No added Follow-ups or Preferences. The validator review covered provenance, family/subject agreement, queued bundle admission, D18, evidence and retirement authorization. The independent test/companion review covered all 26 added cases, the exact inventory delta and rebuilt bundle digest. This does not restart or replace the earlier full content review. Actual supplemental verification: Darwin ARM64 / CPython 3.12.13 / PostgreSQL 17.10 ARM64, fictional fixtures, explicit routes to one disposable container:
Five-file repair: 282 insertions / 7 deletions; runtime change: 20 net lines. The identity-verified disposable container and tmpfs data were removed. Scope stays within semantic promotion and supersession authorization, under the existing same-task version-2 approval. Extraction consistency still reports the same two missing review records; the existing applicability is unchanged and no gate is waived. Earlier hosted success at Next: resolve the separately governed publication prerequisite, then obtain fresh applicable admission, hosted verification and publication before final human acceptance. |
samovers
left a comment
samovers
left a comment
There was a problem hiding this comment.
PR #380 — scoped repair re-review
Verdict: both previous blockers are resolved. Zero new in-scope blockers or follow-ups. No further code changes identified in this pass.
The PR is not yet merge-ready: fresh exact-head baseline evidence and successful publication remain pending. It is still draft and unmerged.
Reviewed head: d71c1d4cc304a50efc3983a77906f74686ff0946
Previous reviewed implementation: 64d3cbd60828c353bf284f2b7bbbfdbb6f5cd018
Base: ff092c414db9fa24dbd6ab86c7722db89e0c95b5
This review covers the B1/B2 repair, its regression tests, mechanical companions and affected invariants—not a restart of the earlier architecture or full implementation review. It resolves the findings in my previous implementation review at the reviewed repair head. This is a content review, not semantic approval, baseline admission, final implementation acceptance, merge authorization or deployment authority.
B1 — Malformed structural schema identifiers: resolved
_structure_identity() now checks that schemaVersion is a string before using it as a dictionary key. Arrays and objects therefore take the existing _CorrectionProofError path rather than escaping as TypeError. The correction boundary still converts that error into a governed refusal; no broad exception handler was added.
The two added regressions submit array/object schema identifiers through the legacy HTTP path using an otherwise valid structural predecessor. They assert the consequential behavior: a persisted failure trace, no emitted assertion, no supersession intent or retirement edge, and an unchanged predecessor. That directly addresses the missing coverage in the previous head.
Sources: repaired validator and HTTP regression tests.
B2 — Valid field-anchored events rejected: resolved
_assertion_event() no longer requires a literal FARM anchor on the source event. It validates the anchor collection's shape and passes every anchor through the existing _assert_contained() decision. A field-only event can therefore establish containment through its governed identity, without rewriting its immutable event record.
The repair also avoids the opposite defect: accepting an event merely because one anchor is valid. A local farm anchor does not conceal a subsequent foreign farm or foreign field. The negative tests exercise both accepted-predecessor events and queued-correction events.
The refusal handling is appropriate. When containment has already logged a GateRefusal, _CorrectionProofError carries it back to _correction_refusal(), which returns the same refusal unchanged. This preserves its reason and avoids logging a second validation failure. The tests explicitly require exactly one failing validation entry.
Sources: source-event containment and refusal handling and positive and negative regression controls.
Regression coverage and scope
The 26 new cases cover the repair's relevant dimensions:
| Cases | What they establish |
|---|---|
| 2 | Array/object structural schema IDs produce governed refusals. |
| 8 | All four families retain field-only ordinary acceptance and complete contest → correction → authorized retirement paths, with original event bytes preserved. |
| 16 | Foreign, mixed and malformed source-event anchors refuse at accepted-origin and queued-correction boundaries. These are explicitly synthetic Store-read faults. |
The positive tests do not merely call the repaired helper: they use the HTTP/pipeline/Store composition and inspect stored lineage, source-event references, authority receipts and unchanged history.
The repair remains bounded to five previously owned files, with 20 net runtime lines. The remaining changes add tests, update the generated inventory from 4,379 to 4,405, and refresh two expected bundle-digest literals without weakening the surrounding assertions. The capability manifest, retirement gate, emitters and authority policy are unchanged by this repair.
The unique source-edge, exact-kind, event-family, subject and scalar-reference checks remain present. The correction proof still establishes the predecessor before publishing its checked reference; the scope repair does not replace those checks with containment alone.
Sources: exact repair comparison, validator, and test inventory.
Verification: independently checked versus reported
I executed 36 isolated source-excerpt probes; all passed. They cover malformed schema identifiers, valid field-only scopes, foreign and mixed scopes, malformed anchor shapes, unavailable/wrong-kind identities, duplicate or missing source edges, and source-reference disagreement. Refusal cases also check single logging and preservation of the original containment refusal.
These probes use copied function bodies with a fake Store, registry and logger on Python 3.13.5. They are not repository pytest, live HTTP/PostgreSQL execution or the pinned baseline. The supplemental probe source and results were provided with the ChatGPT review; they are not repository or hosted-baseline evidence.
I also confirmed that current-head lightweight workflow 34228881147 completed successfully and inspected its job-step results, including package/architecture and whitespace checks. This is not the full Kernel baseline.
The repair report records 221 passing correction/compatibility cases plus 223 passing bundle cases in its supplemental Darwin ARM64 / Python 3.12.13 / PostgreSQL 17.10 environment. The 26 new regressions are included in the 221, not additional to them. I did not independently rerun those suites or rebuild the aggregate bundle digest in this pass.
Remaining acceptance conditions
The earlier 4,379-test two-run baseline belongs to 64d3cbd60828c353bf284f2b7bbbfdbb6f5cd018, not this repaired head. It cannot be carried forward as execution evidence for d71c1d4cc304a50efc3983a77906f74686ff0946. The PR correctly labels it historical and retains the separate publication-custody hold and missing final receipt. Historical runs: source 34149023176 and publisher 34151241570.
The shared-connection transaction-ownership follow-up remains open. The two previously disclosed extraction-consistency failures also remain reported failures; this review neither closes them nor waives an applicable gate.
Disposition: mark B1 and B2 resolved for content review at d71c1d4cc304a50efc3983a77906f74686ff0946. Keep the PR draft until the publication prerequisite and fresh applicable exact-head verification are satisfied. These are implementation/conformance repairs and require no active baseline-law amendment. Final human acceptance and exact-head merge authorization remain separate.
samovers
left a comment
samovers
left a comment
There was a problem hiding this comment.
PR #380 — re-review of the implementation at d71c1d4
Independent re-review at head d71c1d4cc304a50efc3983a77906f74686ff0946 ("Preserve governed
correction refusals and contained event scopes"), prior reviewed head
64d3cbd60828c353bf284f2b7bbbfdbb6f5cd018, base
ff092c414db9fa24dbd6ab86c7722db89e0c95b5.
Disposition: 0 Blockers. 1 new Follow-up (out of boundary), 3 carried forward,
1 prior Should fix downgraded to a Preference.
My Blocker from review
5140352486
is closed — both facets, verified with the same probes that found it, now matching base
line for line. The fix is the containment option I recommended, implemented better than
I proposed: it validates every anchor rather than only proving the farm is present,
so it is strictly stronger than the pre-#380 behaviour as well as stronger than the
64d3cbd behaviour.
What I got wrong last pass
This head also fixes a defect I did not find. At 64d3cbd, a schemaVersion of [] or
{} in a correction's structure payload — an ordinary caller-supplied field on the public
/commit route — raised an unhandled TypeError out of the gate pipeline:
### head 64d3cbd
[ []] ESCAPED THE BOUNDARY: TypeError: unhashable type: 'list'
[ {}] ESCAPED THE BOUNDARY: TypeError: unhashable type: 'dict'
[ 17] RETAIN_DRAFT ['CORRECTION_REQUIRED']
[ None] RETAIN_DRAFT ['CORRECTION_REQUIRED']
### head d71c1d4
[ []] RETAIN_DRAFT ['CORRECTION_REQUIRED']
[ {}] RETAIN_DRAFT ['CORRECTION_REQUIRED']
The RFC's threat model names payload as untrusted data requiring validation, so an
escaping TypeError from it is a validation failure, and I would have classed it a
Blocker at 64d3cbd had I found it. I probed the payload for semantic attacks and never
tried a wrong-typed one. That is a gap in my method, not a lucky catch on the author's part.
For the record, I measured its blast radius at 64d3cbd: it is availability and
auditability, not integrity. The transaction rolls back cleanly, the predecessor stays in
force, the store is usable immediately afterwards, and a subsequent correction promotes
normally — but no draft envelope is persisted, so unlike every other refusal it leaves no
governed refusal record:
[crash] TypeError: unhashable type: 'list'
[truth] unchanged=True old_superseded=False
[events] before=7 after=7 (refusals normally persist the draft envelope)
[recovery] next correction -> PROMOTE_ACCEPTED old_superseded=True
Blocker 1 — closed, verified with the original probes
The change at kernel/validators.py:216-232 replaces the literal farm-membership test on
the source event with a loop over every anchor through the repository's own
_assert_contained, plus explicit refusals for missing, empty and malformed anchor lists,
and it preserves the governed refusal that containment already logged instead of re-wrapping
it as "Correction relationship refused".
Re-running my two probes unchanged at the new head:
Facet 1 — ordinary queued acceptance, targetScopes = [{"scopeType":"FIELD","scopeRef":demo.FIELD}], no correction anywhere:
[STRUCTURE_ASSERTION ] QUEUE -> RETAIN_DRAFT ACCEPT -> PROMOTE_ACCEPTED [] conseq=1
[OPERATION_CLAIM ] QUEUE -> RETAIN_DRAFT ACCEPT -> PROMOTE_ACCEPTED [] conseq=1
[OBSERVATION_ASSERTION ] QUEUE -> RETAIN_DRAFT ACCEPT -> PROMOTE_ACCEPTED [] conseq=1
[COMPLIANCE_ASSERTION ] QUEUE -> RETAIN_DRAFT ACCEPT -> PROMOTE_ACCEPTED [] conseq=1
All four families now match base exactly (64d3cbd returned
RETAIN_DRAFT ['SCOPE_NOT_AUTHORIZED'] conseq=0 for all four).
Facet 2 — accepted truth minted through the direct path is correctable again:
[field-only] CORRECT -> PROMOTE_ACCEPTED [] old_superseded=True
[farm] CORRECT -> PROMOTE_ACCEPTED [] old_superseded=True
Also unchanged: K-02 is still refused on all four variants, at the relationship stage,
including with full REVIEW_SUPERSEDE:
[K-02 nosup/direct] RETAIN_DRAFT ['CORRECTION_REQUIRED'] superseded=False truth_unchanged=True
[K-02 nosup/queued] RETAIN_DRAFT ['CORRECTION_REQUIRED'] superseded=False truth_unchanged=True
[K-02 sup/direct] RETAIN_DRAFT ['CORRECTION_REQUIRED'] superseded=False truth_unchanged=True
[K-02 sup/queued] RETAIN_DRAFT ['CORRECTION_REQUIRED'] superseded=False truth_unchanged=True
Does the relaxation open anything?
This is the question that matters for a fix that removes a check, so I attacked it three
ways rather than reading it.
1. Containment is stronger than what it replaced, not weaker. _assert_contained
(validators.py:107-140) refuses TENANT/DEPLOYMENT outright, requires a FARM ref to
be ctx.farm_ref, and requires every other scope to resolve, to be an IdentityRecord,
and to be anchored on ctx.farm_ref. The old _correction_farm only checked that the farm
entry was present and ignored every other anchor — so an event anchored
[{FARM, thisFarm}, {FIELD, anotherFarmsField}] passed the old check and refuses now. The
author's mixed-farms / mixed-fields parameters test exactly that, and my N5 below
confirms they fail if the old check is restored.
2. The check is unreachable through the real ingress — it is defence-in-depth. Every
foreign anchor shape is already refused at submission by ScopeContainmentValidator, so no
such event can be stored through the API at all:
[ingress foreign-farm ] RETAIN_DRAFT ['SCOPE_NOT_AUTHORIZED'] assertions=0
[ingress foreign-field ] RETAIN_DRAFT ['SCOPE_NOT_AUTHORIZED'] assertions=0
[ingress mixed ] RETAIN_DRAFT ['SCOPE_NOT_AUTHORIZED'] assertions=0
[ingress tenant ] RETAIN_DRAFT ['SCOPE_NOT_AUTHORIZED'] assertions=0
[ingress unresolved ] RETAIN_DRAFT ['IDENTITY_UNRESOLVED'] assertions=0
That is why the new test uses a synthetic event-read fault, and its docstring says so. I
record it as the correct oracle here, and as a limitation worth naming in the change report:
nothing proves the event-anchor check against a foreign anchor that arrived through the
real ingress, because the real ingress cannot produce one. Its value is containment
against store-level corruption, which the threat model already excludes — so this check is
belt-and-braces, and the primary cross-farm guard remains _correction_farm on the
predecessor consequence, review decision and identity records, all of which still demand the
literal farm entry.
3. Timing asymmetry, checked and clean. Containment now runs at acceptance time against
ctx.store state, where ScopeContainmentValidator ran it at commit time. I looked for a
scope that resolves at commit and not later: IdentityRecords are append-only (Kernel
rule 1) and _ensure_identity_record only ever adds, so nothing that resolved can stop
resolving. A structure assertion naming its own not-yet-materialized identity in
targetScopes is refused at commit (IDENTITY_UNRESOLVED), so it cannot be queued into that
state either.
The new tests are load-bearing
Three neutralizations at d71c1d4, one at a time, re-running the two correction modules
(136 tests):
| Mutation | Result |
|---|---|
N5 — restore the pre-fix _correction_farm(ctx, event) literal check |
12 failed, 124 passed |
N6 — drop the isinstance(schema_version, str) guard |
2 failed, 134 passed |
N7 — drop exc.refusal preservation, re-wrapping the containment refusal |
8 failed, 128 passed |
N5's 12 splits informatively: 8 are
test_field_only_event_scopes_preserve_acceptance_and_correction (my regression, all four
families × ordinary-acceptance and correction-chain) and 4 are the mixed-* parameters —
so the suite now pins both the relaxation and the strengthening. N7 confirms the
"preserve it once" property is genuinely asserted, via the == 1 FAIL-gate count rather
than by inspection. That test also drives a real /review/contest and asserts
_lineage_has_dispute(successor), which closes the D21 end-to-end path my own probe could
not reach last pass.
Follow-up 1 (new, and explicitly OUT of this PR's boundary) — the same defect class is live at base on the plain commit path
Having missed the schemaVersion crash, I swept the class rather than the instance: every
caller-controlled field the commit path feeds to a dict lookup, a frozenset membership test,
or a SQL parameter. Most are correctly refused by contract validation with a governed 4xx
(commitClass, actingAgentRef, aiAssistance, targetScopes). Three are not — and they
need no correction, no supersedesConsequenceRef, and nothing this PR touches. A plain
OBSERVATION_ASSERTION:
############ HEAD d71c1d4 ############ ############ BASE ff092c4 ############
[plain subjectType=[] ] ESCAPED TypeError [plain subjectType=[] ] ESCAPED TypeError
[plain subjectType={} ] ESCAPED TypeError [plain subjectType={} ] ESCAPED TypeError
[plain subjectType=[1] ] ESCAPED TypeError [plain subjectType=[1] ] ESCAPED TypeError
[plain subjectRef=[] ] HTTP 200 RETAIN_DRAFT [plain subjectRef=[] ] HTTP 200 RETAIN_DRAFT
[plain subjectRef={} ] ESCAPED ProgrammingError [plain subjectRef={} ] ESCAPED ProgrammingError
[plain subjectRef=[1] ] ESCAPED UndefinedFunction [plain subjectRef=[1] ] ESCAPED UndefinedFunction
Byte-identical at both heads. Root causes, from the tracebacks:
subjectType—kernel/validators.py:460,PromotionTargetValidator.run:
if subject_type not in policy.CONSEQUENCE_SUBJECT_TYPESis a frozenset membership test
on raw caller input. An unhashable value raises before any refusal can be built.subjectRef—kernel/validators.py:473→_assert_contained→:128
ctx.store.get_record(scope_ref)→kernel/store.py:583, where adict/listreaches a
psycopg placeholder and the database driver's error surfaces to the caller
(cannot adapt type 'dict',operator does not exist: text = smallint[]).
This is pre-existing base behaviour and PR #380 should not be asked to fix it — that
would be exactly the "broad unrelated cleanup" EXC-004 forbids. But it belongs in the
delivery record, because it is the same class as the defect this PR did fix inside its
boundary, and the fix's own guard style (isinstance(schema_version, str)) is the pattern
that closes it. A driver-level exception reaching an ingress caller is also worth its own
severity judgment by whoever owns that surface.
Carried forward from the previous review
Downgraded: prior Should fix 1 → Preference. _assertion_event still runs the full
correction proof on every non-reject acceptance (GovernanceAcceptanceValidator:588), not
only on corrections. The harmful part is gone: containment no longer refuses lawful traffic,
and a containment failure now reports the governed containment reason rather than
"Correction relationship refused". What remains is a latent coupling — the other three
conditions (exactly one EVENT_SOURCE edge, envelope kind, family agreement, subjectRefs
equality) would still refuse an ordinary promotion with correction wording. I re-confirmed
all three are unreachable at this head, so this is now a readability and blast-radius
preference, not a defect. Splitting a minimal _acceptance_event from the full proof would
still be the cleaner shape.
Follow-up: the declared D5 residual stands unchanged — an actor holding
ASSERT_OPERATION_CLAIM + REVIEW_ACCEPT + REVIEW_SUPERSEDE can still retire any in-force
operation consequence sharing the subject tuple, measured last pass. Still in-boundary and
declared; still worth carrying into the decision card in measured form.
Preference: the next() reverse lookup at validators.py:234 is unchanged. Injective
today; determinism is still a property of the data rather than the code.
Preference: the fixture's standing REVIEW_SUPERSEDE grants are unchanged, so the two
correction modules remain the only tests that can detect the retirement check being dropped.
Mechanical companions, re-verified at this head
Change since the reviewed head — 5 files, 282 insertions, 7 deletions:
kernel/validators.py (26 lines), kernel/tests/test_correction_authorization.py (+125),
conformance/review_baseline_test_inventory.json (134), and the two runtime-bundle digest
literals. git diff --check clean.
No new post-approval RFC drift. git diff 64d3cbd d71c1d4 -- docs/ is empty — the
RFC and REVIEW_DISPUTE_SEMANTICS.md are byte-identical to the head I already checked, whose
only post-approval edits were status and approval evidence.
Suite A/B, CPython 3.12.13, live PostgreSQL 16.13, one process each:
head d71c1d4 : 3952 passed, 345 skipped, 4 failed, 104 errors, 1 warning (649s)
base ff092c4 : 3816 passed, 345 skipped, 4 failed, 104 errors, 1 warning (365s)
+136 passed, and the sorted 108 FAILED/ERROR name sets are identical to base
(diff empty) — all of them my PG16 harness's tenant-provisioning gate. No head-only failure.
The two correction modules alone: 136 passed.
Review baseline inventory — regenerated with update-inventory and byte-identical to the
committed file:
entryCount ff092c4=4269 64d3cbd=4379 d71c1d4=4405
vs 64d3cbd: added 26 removed 0 (all kernel/tests/test_correction_authorization.py)
vs ff092c4: added 136 removed 0
entriesSha256 b1fbb1055f1531440f3ba4aff4ecc1e179f01692c7a35b1d40a29f9e8e9d23b0
+136 matches the suite delta exactly.
Runtime bundle digest. The two literals move
ee43613b9fbee4d2be60388e981ad917b419e6dcd5d33c3daf95a7761c4bd4a9 →
7b4c820d299aabc196b26e4f515d313a542fbaf37d027002fa46bdb69d8be461, as expected from a
kernel/validators.py edit. test_profile_selection_validation.py and
test_runtime_bundle.py pass at head (224 tests together with my ingress probe), so the new
literals agree with what the code computes from the checked-in catalog rather than being
pasted from a failing run.
Gates:
| Check | d71c1d4 | ff092c4 |
|---|---|---|
ofarm_pkg_contract_check.py |
PASS (0 failures) | — |
rewrite_architecture_check.py |
PASS | — |
ofarm_profile_extraction_consistency_check.py |
FAIL (2) | FAIL (2) |
ruff check . repo-wide |
2 errors | 2 errors |
The two extraction failures are the identical pre-existing paths. Unchanged from my last pass.
Checked and decided were not findings
- The
_correction_farmcalls that remain — on the predecessor consequence, its review
decision, the compliance claim, the identity record and the assertion — all still demand
the literal farm entry, and all those records hard-codeanchorScopesto exactly that. The
fix relaxed only the one record whose anchors are caller-supplied. Correct target. - Double-logging.
_assert_containedcalls_refusal, which logs; the new code raises
with the refusal attached and_correction_refusalreturns it unchanged. Exactly one
VALIDATION FAIL_*gate entry, asserted by the new test and confirmed by N7. _assert_containedat acceptance time vs commit time — no scope that resolved can stop
resolving; see "timing asymmetry" above.- Extra reads. The containment loop adds one
get_recordper non-FARManchor on every
acceptance and twice per correction. Bounded bytargetScopeslength; not worth a finding. schemaVersionof17andnullwere already handled at64d3cbd— only the
unhashable types escaped, which is why the guard isisinstance(..., str)rather than a
presence check.- The
mixed-*cases are a behaviour change against base, not just against64d3cbd.
An event carrying the farm entry plus a foreign anchor was correctable atff092c4and is
refused now. I convinced myself this is a strengthening rather than a regression: such an
event cannot be created through the ingress at either head, so no lawful history is
stranded by it.
What my method made easier than production
- PostgreSQL 16.13, not the pinned 17.10, x86_64 Linux, not the locked baseline. The 104
provisioning errors are that gap. Unchanged from last pass. TestClient, one process, oneStore. My concurrency evidence is still the PR's own
test_correction_transactions.pyneutralized, not an independent race harness.- I did not re-run the hosted workflows or verify a publication receipt. The evidence-
publication custody defect reported in comment5574394917is outside this boundary and I
take the author's account of it as given; it remains an open blocker to publication that
this head does not address and is not asked to. - My "same defect class" sweep is not exhaustive. I tried unhashable and wrong-typed
values on fourteen caller-controlled fields across three submission shapes. A field I did
not name, or a type I did not try, could still escape. The three I found are pre-existing,
so nothing here changes this head's disposition — but "0 Blockers" means "none found by
this method", and this method has now missed one defect on this PR already.
Verdict
The Blocker is closed, correctly, and with a fix that is stronger than the one I asked for.
The regression tests pin the fix in both directions, the strengthening included, and they
reach the D21 contest-to-correction path end to end. Nothing in the relaxation opens a
cross-farm path: containment is strictly tighter than what it replaced, and I could not
construct a foreign anchor that reaches the store through the real ingress at all. The
mechanical companions all reproduce from scratch — inventory byte-identical, digests
consistent with the code, gates and ruff unchanged, and a suite whose failure set is
identical to base.
No Blockers at d71c1d4. From my side this head is ready for baseline admission, with
the new Follow-up filed against whoever owns the ingress surface rather than against this
Delivery, and the D5 residual carried into the decision card in its measured form.
Two things I will not let pass silently. First, I missed a Blocker-class defect at
64d3cbd, and the author found it — my previous "0 Blockers besides this one" was worth less
than it read. Second, this review is by samovers, the PR author, as are all four prior
reviews and the issue-comment reviews on this PR. No independent reviewer has looked at
this change. Whatever the disposition says, that gate is still open.
|
The separate publication-custody prerequisite is now Delivery #381 and draft PR #382, based on current main. It contains a reviewed Phase A only and is awaiting its own version-1 same-task semantic approval before publisher implementation. PR #380 remains draft and unmerged at PR #380 depends on the separately accepted and merged prerequisite, followed by its own fresh applicable admission, hosted verification and final publication receipt. No failed workflow attempt will be rerun and no earlier-head evidence replaces those gates. Next: complete the separate approval and implementation in PR #382, then resume PR #380's publication sequence. |
|
Fresh technical admission for unchanged reviewed PR #380 head OFARM2_BASELINE_ADMISSION |
1 participant
Problem and resulting behavior
Closes #379. Tracking Epic #180, under #167. Candidate:
d71c1d4cc304a50efc3983a77906f74686ff0946.Legacy correction could retire an incompatible accepted consequence without supersession authority. This PR validates one explicit predecessor and independently authorizes retirement before direct or queued acceptance emits a successor. For example, operation assertion and acceptance rights no longer permit replacing accepted structural state. Structure, operation, observation and compliance retain compatible correction and dispute resolution.
The B1/B2 repair also makes malformed structural schema IDs produce governed refusals, accepts lawful field-only event scopes, and refuses every foreign or malformed source anchor while preserving one failure trace. The reviewed head remains unchanged.
Boundary, authority and decision
The primary trust boundary is semantic promotion and supersession authorization. Scope stayed inside it, without semantic deviation or a cross-boundary exception. Protect accepted force, immutable history and lineage, materialization basis and accountable authority receipts. The primary risk is confusing permission to assert or accept with permission to retire incompatible state; relationship proof and independent retirement authority must precede emission inside the existing transaction.
Canonical OFARM retains meaning and identity authority. Existing principal binding and the evaluator retain action, scope, time, revocation and human/agent decisions. Correction validation proves compatibility; promotion mediates retirement; the Store owns the transaction. The task user owns semantic approval and later exact-head merge authorization. Reviewers, CI, publication and GitHub supply findings, evidence, custody and native PR state, not user approval. Caller-supplied references, payload, reviewer names and stored intent remain untrusted; database-owner/arbitrary-writer compromise and compromised code or keys are excluded.
Governing decision:
OFARM2-LEGACY-CORRECTION-AUTHORIZATION-001, version 2, for this PR only. The approved RFC owns the detailed contract. Same-task approval navigation: task01a07cc8-4157-7b33-a0ca-becb772e0e8b; complete cardmsg_0d813f8071772f9a016a9eec105ebc87d2bed1e55768a9ecdb, followed by exact user approvalmsg_01a07cd7-b6ae-7f20-bcc4-a51a1026d77c. Original text and ordered references remain preserved. This description is not a replacement card or merge authority.Publication custody was corrected separately in merged PR #382, Delivery #381. Fresh PR #380 execution merge
aa58864fd84880b7ecdf6818e64b06cb2499013cbinds live main70025a973f36fa92d2c130247c52b96877dcc6e5and this unchanged reviewed head. The prerequisite's receipt is not PR #380 evidence.Accepted effects and invariants
REVIEW_ACCEPTand exactALLOWforREVIEW_SUPERSEDE, usingPROMOTION, current farm, agent/AI context and current revocation checks. Existing authority request, trace and result references enter the promotion trace.CORRECTIONcarriers remain closed. Older-bundle accepted history stays usable; queued assertions still need the current bundle at acceptance. All four families retain acceptance → contest → queued correction → authorized retirement under unchanged CONTEST, REJECT, evidence and self-review rules.Compatibility-negative tests have sufficient authority; permission-negative tests use compatible targets. HTTP/pipeline/Store tests inspect durable receipts, lineage, unchanged history and derived dispute state. Inventory and manifest checks alone do not prove mediation.
Complete scope and code excellence
14 files, 2,517 insertions and 151 deletions against the reviewed merge base
ff092c414db9fa24dbd6ab86c7722db89e0c95b5. The five owning runtime/comment Python files total 297 insertions and 133 deletions: 164 net lines, including comments and formatting. These are PR #380 contribution counts, not a two-tree diff that would mistake later main-only prerequisite changes for deletions.kernel/validators.py,kernel/stages.py,kernel/emission.py,kernel/policy.py,kernel/manifest.py.kernel/tests/test_correction_authorization.py,kernel/tests/test_correction_transactions.py; exact bundle-digest companions:kernel/tests/test_profile_selection_validation.py,kernel/tests/test_runtime_bundle.py.profile_si_ffs/test_fixtures/demo_records.py; generated companions:profile_si_ffs/OFARM_Capability_Manifest_si_ffs_pilot_v0_1.json,conformance/review_baseline_test_inventory.json.docs/REVIEW_DISPUTE_SEMANTICS.md,docs/rfcs/OFARM_Legacy_Correction_Authorization_RFC_v0_2.md.The B1/B2 repair alone is five already-owned files, 282 insertions / 7 deletions and 20 net runtime lines, including 26 added HTTP cases. The two digest expectations retain exact checks over the same 95 selected bundle components and tenant; only validator content changed among those components in that repair. These companions introduce no activation or independent authority. No schema/migration companion is needed.
EXC-001/002: one shared correction proof and one shared retirement check; two transient checked references replace unchecked interpretation, without a second policy registry, durable correction record or authority cache. EXC-003: C01–C05 map to relationship/provenance proof; C06 to evaluator decisions and receipts; C07–C10 to emission, rollback/replay, ordered writers and dispute tests. EXC-004: owned raw-target, first-intent/event and duplicate D18 selection paths were removed; existing containment refusals are reused once. EXC-005: small helpers serve current direct/queued consumers, without a generic graph framework. EXC-006: authority alone permits incompatible retirement, class equality omits subject/identity, and disabling supported families strands disputes. The bounded proof plus existing evaluator supplies the complete correction. Existing applicable dependency/isolation checks remain; numerical architecture budgets do not cover these legacy semantic files and no exception was added.
Review and verification
The September 8 B1/B2 findings are resolved at this exact head. The coordinated scoped review and formal reviews 5142663405 and 5143341884 report zero in-scope Blockers. The audit was not restarted when the publication prerequisite merged. Fresh admission 5631512841 binds this head; historical workflow attempts were not rerun.
Reviewer evidence is attributed separately. Review 5142663405 reported 36 passing copied-function probes using a fake Store/registry/logger on Python 3.13.5; it did not independently rerun the repository suites or rebuild the bundle digest. Review 5143341884 reported its own Python 3.12.13 / PostgreSQL 16.13 Linux run: head 3,952 passed, 345 skipped, 4 failed, 104 errors, 1 warning; base 3,816 passed, 345 skipped, 4 failed, 104 errors, 1 warning. It attributed the identical failing/error set to its PG16 provisioning harness and separately reported 136 passing correction-module cases. Its three repair neutralizations produced 12, 2 and 8 expected failures. These are supplemental reviewer reports, not local reruns or green pinned baselines. Earlier-head hosted results remain historical and do not verify this repaired head.
Material limits, follow-ups and preferences
ASSERT_OPERATION_CLAIM,REVIEW_ACCEPTandREVIEW_SUPERSEDEcan retire an in-force operation consequence sharing the subject tuple. Same subject establishes bounded compatibility, not unique operation, occurrence or proposition identity; the explicit predecessor selects lineage. This approved limitation remains visible. New identity semantics require separate design.subjectType/subjectReffailures on ordinary observation commits at both base and head, including unhashable-type and database-driver exceptions. No correction is needed to trigger them. They remain ingress-owner work, explicitly outside PR Constrain and authorize legacy correction supersession #380; this PR does not claim to fix them.next()reverse lookup remains injective by current data rather than explicit code construction; standing fictional supersession grants leave the two correction modules responsible for detecting removal of retirement mediation. The former “Should fix” was downgraded after lawful scopes and governed refusal reporting were repaired.conformance/review_baseline_test_inventory.jsonandkernel/tests/test_rewrite_architecture_check.py. Preserved applicability requires execution/reporting and does not make PASS a gate when extraction inventory/status and checker inputs are unchanged. No failed check is relabelled PASS and no applicable gate is waived.The reviews were posted through the PR author's GitHub account and do not establish independent human approval. The reviewer's asserted additional independent-review gate is not an adopted workflow requirement. Existing same-task approval and later exact-head user acceptance remain controlling.
Current live check: head unchanged and worktree clean; PR remains open and draft, GitHub reports
CLEANandMERGEABLE. Lightweight package/workflow checks passed. The old pre-admission automaticexecutejob is skipped by design; the fresh admitted source above completed every job. Admission 5631512841 remains unedited with live owner/admin standing. No new formal review, inline finding or revocation appeared. These repository procedure gates are not a claim that branch protection enforces them.Non-effects and final acceptance
No canonical/reference/frozen-contract, schema, migration, database-role, transaction-ownership, credential/principal, signing/custody, audit, runtime-activation, deployment or release change. No real actor receives a grant; no self-review right, claim identity, governing-rule equality, generic graph repair or new correction carrier is introduced. The rule is intended to endure, while the surrounding legacy environment remains pre-deployment; no production readiness or same-app concurrency is claimed. Changed capability, authority, effects, invariants, boundary, named PR or deployment posture require a new decision. Deployment requires independently human-controlled authority.
The final same-task user packet and a later exact-head merge authorization remain separate requirements. CI, receipt verification, this description and semantic approval alone do not authorize merge.
Next: present the complete exact-head final packet and yield for the user's later merge decision.