Require explicit boolean confirmation for legacy acceptance - #384
Conversation
Record the K-03 input contract, authority containment and focused verification before implementation. Require actual boolean confirmation before governed ingress while preserving existing review and replay authority. Phase A for Delivery #383 under M1 review semantics; runtime changes await the same-task decision.
|
Phase A design review for Delivery #383 / draft PR #384 at exact head Result: zero design Blockers. This is the lead AI's review of the proposed contract and its fit to the existing code, not an independent human review, runtime implementation review, semantic approval or baseline admission. There is no baseline-admission footer in this comment. The K-03 defect remains present until the later approved implementation. The exact diff contains only Reviewed the committed RFC and draft PR description against the recorded K-03 finding, the focused 15-case reproduction,
Executed evidence for preparation:
Blockers: none. New Follow-ups or Preferences from this review: none. Existing separate work remains visible: K-04 self-review consistency; plain wrong-typed subject input; broad #180 lifecycle/shared-connection ownership; #184 graph work; and the two previously reported missing extraction review records. No expensive baseline, new admission, extraction-diagnostic rerun or implementation verification was performed for this design-only head. Next: present the complete same-task |
samovers
left a comment
samovers
left a comment
There was a problem hiding this comment.
PR #384 — Phase A design review
Verdict: no design Blockers found. Ready for the semantic-approval step, not for merge.
Reviewed head 4d2c7d2eb1d5b932d1947b39683b04f70b92456b against base b6017da1dbfac80b5c2e98641aaa34d96691d087. This is a design-only draft: one RFC, 190 added lines, no runtime changes. The confirmation defect is therefore still present at this head.
This is an AI-authored design review, not independent human review, semantic approval, baseline admission, implementation verification, or merge authorization.
Findings
No blocking findings in the proposed design. I checked the RFC against the existing parser, pipeline, promotion gate, replay writer, HTTP adapters, emission code, and ingress tests—not just the PR description.
1. The proposed rejection point is correct.
GatePipeline.commit() calls parse_ingress_header() before serialized_tx(). Adding the optional boolean check there can reject malformed confirmation before idempotency lookup, authority evaluation, or governed writes. Checking only in ReviewPromotionGate would be too late: normalization can already write records or take the replay path. The proposed design addresses both direct callers and the legacy HTTP route.
Sources: kernel/gates.py (GatePipeline.commit), kernel/stages.py (parse_ingress_header, IngressNormalizer.run) at the reviewed head.
2. The four-consumer change matches the actual code.
I confirmed the four truthiness checks: compliance self-review routing, bounded structure self-review routing, body-named distinct-reviewer routing, and the final direct-confirmation branch. One function-local confirmed = sub.get("confirmAccept") is True gives them consistent interpretation without adding stored state or another authority mechanism. Existing review-authority and correction-retirement checks remain downstream.
Source: kernel/stages.py (ReviewPromotionGate.run, _authorize_retirement) at the reviewed head.
3. The replay requirements preserve an important distinction.
The pipeline computes a digest of the whole submission, excluding only caller-supplied sourcePayloadDigest; the replay writer compares that digest with the original. Preserving the raw submission therefore matters: omission and explicit false must not be normalized into the same identity. Rejecting malformed confirmation before lookup also prevents a previously used key from bypassing the new admission rule. The RFC explicitly requires both properties.
Sources: kernel/gates.py (_source_digest), kernel/emission.py (ReplayWriter.write), and the RFC's input contract and R02–R03 at the reviewed head.
4. The HTTP and queued-review compatibility claims are supported.
The legacy /commit adapter checks actor binding before calling the pipeline and already maps IngressHeaderViolation to the fixed, payload-free 422 response. The queued accept/reject/contest adapters construct governance submissions without confirmAccept, and the review gate handles their target-based branch separately from direct confirmation. The proposed optional-field rule does not introduce a new flag requirement for those routes.
Sources: kernel/legacy_m1/api.py (_install_commit_route, _review_submission, _install_review_routes) and kernel/stages.py (ReviewPromotionGate.run) at the reviewed head.
5. The scope is appropriately bounded.
The design does not need a schema migration, new review right, historical rewrite, or production activation. Production governed routes currently return the explicit blocked response; this RFC does not change them. Deferring K-04 self-review consistency and broader lifecycle work is reasonable rather than expanding a confirmation-type fix into a larger authorization change.
Sources: kernel/api.py (_production_app) and the RFC's non-effects at the reviewed head.
What still needs proof
The RFC already specifies the relevant implementation tests. The decisive evidence will be absence of transaction entry and governed writes, not merely absence of an accepted consequence. That must cover malformed confirmation with a previously accepted idempotency key, as well as falsey invalid values such as null, 0, "", [], and {}. Valid draft, direct-acceptance, replay, and queued-review controls must continue to behave as specified. These are pending implementation requirements, not missing design requirements.
Validation limits: I did not execute tests or reproduce the PostgreSQL cases during this review. The reported 42 passing ingress tests and 15-case defect reproduction are author-reported preparation evidence, not independently rerun results or proof that the fix works. The PR correctly distinguishes those claims. No expensive baseline or admission was requested by this review.
Disposition: no requested design changes and no new follow-up findings. Proceed through the required semantic approval, implement the bounded change, and review the resulting runtime head before merge.
Require actual boolean confirmation before the legacy pipeline transaction and consume literal true consistently in the existing review gate. Preserve draft capture, replay, review authority and production closure. Add 70 focused cases and the prescribed inventory, with 162 local cases passing. Implements approved decision OFARM2-LEGACY-REVIEW-CONFIRMATION-001 v1 for Delivery #383 under M1 review semantics.
|
Coordinated implementation review of PR #384 / Delivery #383 at exact head Verdict: zero demonstrated in-scope Blockers. No new Follow-ups or Preferences. This is one coordinated AI content review with separate runtime/HTTP and parser/compatibility/documentation/inventory assignments plus lead consolidation. It is not independent human approval, baseline admission or merge authorization. The original audit was not restarted. The sole boundary remains legacy review-confirmation admission under approved decision
Verified execution and collection evidence:
The fresh extraction diagnostic remains FAIL (2) for missing review-record coverage of the root test inventory and Existing Follow-ups remain: K-04 self-review consistency; plain wrong-typed subject ingress; #180 lifecycle/shared-connection ownership; #184 graph work; extraction review records. No preference is being promoted to a blocker and no unrelated fix is bundled to clear review. No expensive baseline was requested or monitored during this review. The formal review of earlier design head Next: recheck the live head, review findings and repository standing, then create a separate admission comment through the existing default-branch gate. Required hosted evidence and later exact-head user merge authorization remain outstanding. |
|
Admit the reviewed PR #384 head for the existing hosted baselines and trusted publication path. The coordinated implementation review reports zero demonstrated in-scope Blockers at this exact head. The same-task version-1 semantic approval was directly retrieved and verified. The six-file change remains within legacy review-confirmation admission. Local checks passed: 162 distinct tests, package/architecture, Ruff, whitespace and unchanged generated capability artifacts. The prescribed inventory adds exactly 70 cases to 4,475. The extraction diagnostic's two failures remain explicitly reported under the documented applicability determination; no applicable gate is waived. Live head, open draft state, no close/reopen transition, absence of new review findings and repository-owner standing were rechecked. The disposable local database was removed after tests. This comment supplies only technical admission, not final user acceptance, merge authority or deployment permission. OFARM2_BASELINE_ADMISSION |
samovers
left a comment
samovers
left a comment
There was a problem hiding this comment.
PR #384 — implementation review at exact head f2a3930
Verdict: 1 Blocker, 2 Preferences, no separate Follow-ups. Classified with AGENTS.md "Review
classifications" (Blocker / Follow-up / Preference; the high-risk Blocker fields are given in full).
The K-03 fix itself is correct and its tests carry real weight — I reproduced the defect at base, watched the
PR's own tests fail there, and ran a mutation matrix against the parser. The Blocker is not in what the four
new lines do. It is in the claim the PR and its approved decision make about them: R04 says literal true cannot
bypass a disallowed self-review. At this head it can. Adding "reviewerPartyRef": null to a confirmed
compliance assertion makes its own asserter accept it and mints a COMPLIANCE_STATUS_ACCEPTED consequence.
The queue path refuses the same self-acceptance. This is pre-existing (base behaves identically), it sits in
the exact consumers this PR rewrote, and no prior review or the 2026-09-06 audit found it.
- Head
f2a3930eab90037a21f8aa05c29f506a49037d6f, baseb6017da1dbfac80b5c2e98641aaa34d96691d087.
git ls-remoteat the end of this review:refs/pull/384/headstillf2a3930,mainstillb6017da. - Six files,
+945/−8, matching the description exactly (numstat: inventory 352/2, semantics doc 12/0,
RFC 207/0,stages.py8/4, ingress tests 82/2, new module 284/0). RFC: 207 lines, 14,494 bytes, blob
390f1929.git diff --check: clean.
Where this pass sits
Every earlier review object on #384 is samovers: the Phase A comment (5633091839, zero design Blockers), the
formal design review 5177725734 at 4d2c7d2 (no tests run), the coordinated implementation review 5633318108 at
this head (zero Blockers, no new Follow-ups/Preferences), and admission 5633333452 (blockers=0). This is an
independent execution pass at the same head. It confirms the parser-side claims of 5633318108 with fresh runs
on a different platform, and adds the one thing none of them tried: hostile values in the other input the
four confirmation consumers read.
It is also a samovers object, so it does not supply an independent-human review either.
Because admission 5633333452 already exists with blockers=0, AGENTS.md:297-301, 333 apply: the Blocker
fix (or claim correction) needs its own exact-head review before the admitted evidence can be reused.
Environment, and what it made easier than production
- CPython 3.12.13 built from the GitHub tag; Linux x86_64; PostgreSQL 16.13, not the 17.10 the RFC's
evidence used. The kernelStoredoes not enforce the 17.10 pin; none of the code under review touches SQL. - PyPI is blocked from this container. I installed 30 of the 49
requirements-review-baseline.lockpins
from a Linux wheelhouse already in this workspace, withpip --require-hashes --no-index, so every wheel
that did install is hash-verified against the lock. Missing:cryptography,PyJWT,psycopg-pool,
google-cloud-kmsand their dependencies, and the pinned Ruff. Consequences are stated at each result below. - All HTTP probes use the repo's own
create_test_app+fresh_envfixtures anddemorecords — the same
harness the PR's new module uses, so the fixture is no more permissive than the PR's own evidence.
It is, however, the development adapter: production governed routes are closed (kernel/api.py), exactly
as K-03 itself was only reachable through this adapter.
What I verified
1. The PR's own suites
pytest kernel/tests/test_ingress_normalization.py kernel/tests/test_review_confirmation.py
112 passed, 1 warning in 15.17s # 91 parser/transport + 21 HTTP/Store, as claimed
pytest <the 50 compatibility selectors from compatibility-v1-provenance.json>
48 passed, 1 warning in 20.92s
The two missing compatibility cases are the test_application_runtime.py production-closure tests: that
module fails to import google.cloud.kms_v1 here. Neither it nor anything it imports is changed by this PR.
2. The tests carry regression weight
Head's two test files copied onto base code:
52 failed, 60 passed
The 52 are exactly the malformed cases: 20 direct parser, 20 HTTP 422, 12 HTTP/Store matrix. The other 18 new
cases pass on base by design — they are compatibility controls (digest/identity, authority/evidence/reviewer
negatives, D17 positive, actor-binding precedence).
3. K-03 at base versus head, beyond the RFC's matrix
JSON literals that Starlette's parser accepts but the RFC's 15-value matrix does not list:
confirmAccept literal |
base | head |
|---|---|---|
NaN |
200 PROMOTE_ACCEPTED, 1 consequence |
422 fixed body |
Infinity |
200 PROMOTE_ACCEPTED, 1 consequence |
422 fixed body |
| 50-digit integer | 200 PROMOTE_ACCEPTED, 1 consequence |
422 fixed body |
duplicate key, "true" then true |
200 PROMOTE_ACCEPTED |
200 PROMOTE_ACCEPTED |
duplicate key, true then "true" |
200 PROMOTE_ACCEPTED |
422 fixed body |
Duplicate keys are last-wins in one parsed dict that both the parser and the gate read, so there is no
parser/gate disagreement — not a finding.
4. Mutation matrix (one mutation of kernel/stages.py at a time, restored after each)
| Mutation | 112 PR cases | 48 compat |
|---|---|---|
| M1 delete the parser check | 52 failed | 48 passed |
M2 gate is True → bool(...) (parser kept) |
0 failed | 48 passed |
M3 parser admits int (isinstance(v, (bool, int))) |
6 failed | 48 passed |
M4 parser check only for OPERATION_CLAIM |
16 failed | 48 passed |
M5 parser admits null |
3 failed | 48 passed |
The parser is load-bearing and every weakening I tried is caught. M2 is Preference 1 below.
5. Inventory
run_review_baseline._load_test_inventory accepts the committed file (canonical, entryCount 4,475,
entriesSha256 160c1ac7…33db8c). Base → head: +70 / −0, no attribution changes. --collect-only of the
two changed modules gives 112 node IDs, set-equal to their 112 inventory rows. I could not regenerate the whole
inventory: full collection hits the missing google-cloud-kms import.
6. Repository gates, base and head
| Check | base b6017da |
head f2a3930 |
|---|---|---|
ofarm_pkg_contract_check.py |
TEMPORAL CANDIDATE PASS, RESULT: FAIL (1 failures) |
identical |
rewrite_architecture_check.py |
1 failure | identical |
ofarm_profile_extraction_consistency_check.py |
FAIL (2 failures) |
identical |
The single package/architecture failure on both sides is repository-pinned Ruff formatter is unavailable —
environmental. kernel/stages.py is in no architecture budget table, so a green architecture check would
say nothing about it anyway. The extraction FAIL(2) names the same two paths as the PR's disclosure.
7. Whole kernel suite, base and head
pytest kernel/tests --continue-on-collection-errors, same interpreter, wheels and cluster:
base 6 failed, 3350 passed, 267 skipped, 127 errors in 468.94s
head 6 failed, 3420 passed, 267 skipped, 127 errors in 495.11s
diff <(sorted FAILED/ERROR node IDs, base) <(same, head) → identical, 133 lines
+70 passes, nothing else moves. The 133 identical lines are environmental: 23 modules that cannot import the
missing google, cryptography, jwt or psycopg_pool wheels; 108 cases refusing with
… must identify a dedicated PostgreSQL 17 service; and two architecture tests whose main() returns 1 on
repository-pinned Ruff formatter is unavailable.
Blocker
B1 — reviewerPartyRef: null turns literal-true confirmation into a disallowed compliance self-acceptance; R04 is false as committed
Violated invariant. RFC R04: "Literal true cannot bypass existing acceptance requirements or
accountability … true with … disallowed existing self-review … cannot directly accept." The PR description
repeats it: "true still requires all existing authority, evidence and eligibility checks." D8, as
implemented in the same gate, excludes compliance assertions from self-review.
Supported entry point. Legacy development/conformance POST /commit (kernel/legacy_m1/api.py:142) and
direct GatePipeline.commit — the same surface as K-03 and as the PR's R01–R06 evidence.
In-scope actor and preconditions. A party whose transport principal matches actingPartyRef and who holds
compliance-assertion and REVIEW_ACCEPT authority on the farm, with durable evidence — demo.FARMER, the same
preconditions as the K-03 reproduction.
Exact path at head.
parse_ingress_header(stages.py:61):confirmAcceptis an actualtrue→ admitted.- Compliance validation and evidence sufficiency pass.
ReviewPromotionGate.run,stages.py:630-631:
sub.get("reviewerPartyRef", ctx.acting_party) == ctx.acting_party. The key is present, so.get
returnsNone,None != acting_party→ compliance self-review routing is skipped.stages.py:658-659:sub.get("reviewerPartyRef") not in (None, ctx.acting_party)→Noneis in the tuple
→ distinct-reviewer routing is skipped.review_route_reasonsis empty,confirmedis true, the actor holdsREVIEW_ACCEPT
→emit_self_review_promotion()(stages.py:730).
This is the presence-versus-value mistake the RFC explicitly closes for confirmAccept ("present null … is
malformed"), one field over, in the same four consumers.
Minimal reproduction (real HTTP, fresh_env, x-acting-party: party:demo.farmer.one):
{"submission": {
"commitClass": "COMPLIANCE_ASSERTION", "actingPartyRef": "party:demo.farmer.one",
"farmRef": "<demo.FARM>", "idempotencyKey": "probe:<uuid>", "eventTime": "2026-06-10T09:00:00Z",
"evidenceRefs": ["evidence:demo.spray.photo.1"], "requestedPromotionTarget": "COMPLIANCE_FACT",
"payload": {"complianceClaim": {"statement": "fictional demo: self-reviewed compliance claim",
"assertedStatus": "CLAIMED_COMPLIANT", "governingRuleRefs": ["<config.EVIDENCE_POLICY_REF>"],
"subjectScopeRef": "<demo.FARM>"}},
"confirmAccept": true,
"reviewerPartyRef": null
}}Measured, identical at base and head:
reviewerPartyRef |
outcome | consequences | ReviewDecision |
|---|---|---|---|
| omitted | REQUIRE_REVIEW HUMAN_APPROVAL_REQUIRED |
0 | — |
party:demo.farmer.one (self) |
REQUIRE_REVIEW HUMAN_APPROVAL_REQUIRED |
0 | — |
party:demo.advisor.one |
REQUIRE_REVIEW HUMAN_APPROVAL_REQUIRED |
0 | — |
null |
PROMOTE_ACCEPTED |
1, COMPLIANCE_STATUS_ACCEPTED |
decidedByPartyRef: party:demo.farmer.one, REVIEW_ACCEPT |
The queue path disagrees. Same actor, same claim captured with confirmAccept: false, then
POST /review/accept by the asserter: RETAIN_DRAFT ['HUMAN_APPROVAL_REQUIRED'], 0 consequences
(validators.py:582, SELF_ACCEPTABLE_ASSERTION_TYPES). unreachable_authoritative_records() is [] after
the direct acceptance — the bad fact is fully reachable accepted truth.
Material consequence. An asserter self-certifies a compliance fact that D8 reserves for a distinct
reviewer, with a ReviewDecision that records it as a normal review act. That is the same class of harm as K-03
(accepted force without the required review act), on the same surface.
Why the existing tests miss it. test_g1_compliance_self_review_still_routes and test_97(a) omit the
field; test_http_true_preserves_existing_acceptance_requirements[reviewer-own-act] uses a distinct string.
Nothing sends a present null.
Structure is not exposed. I sent a D17-unbounded structure payload (ofarm.fieldidentitypayload.v0.2) with
and without reviewerPartyRef: null: both RETAIN_DRAFT ['EVIDENCE_INSUFFICIENT'] at carrier validation,
before the gate. The stages.py:645-646 bound is unreachable for unknown kinds either way.
Smallest acceptable fixes — both measured. Applied to head one at a time:
| Candidate | Diff | null compliance probe |
112 PR cases | 48 compat |
|---|---|---|---|---|
A. gate: (sub.get("reviewerPartyRef") or ctx.acting_party) == ctx.acting_party at :631 and :646 |
2/2 | REQUIRE_REVIEW |
pass | pass |
B. parser: a present reviewerPartyRef must be a non-empty str, else IngressHeaderViolation |
+5 | 422 fixed body | pass | pass |
B mirrors this PR's own contract for confirmAccept and fails before the transaction. A is narrower and
keeps "" routing as a distinct reviewer as it does today.
How to pay it — the task user's decision. Either route clears the Blocker:
- Fix in this PR (A or B plus a present-
nullcompliance negative in the HTTP module). This changes
R01 or R04 and so, by the RFC's own last section, needs decision version 2. - Correct the claim and split the fix. Narrow R04 and the PR description to what is true, record the
null-reviewer compliance bypass as a known residual, and open a separate Delivery. Also a decision
version change, since R04's text changes.
Counter-argument, stated fully. This PR does not introduce or worsen the bypass, and a K-03 fix is strictly
better merged than not. AGENTS.md rules 3–4 forbid appending cross-boundary fixes to clear review, and the
RFC's authority table assigns "self-review eligibility" to the existing policies, not to this boundary — which
argues for route 2 and against calling the code in scope. I still classify it as a Blocker because the same
table assigns "named reviewer" to the promotion gate this PR edits, and because what would merge is an
approved security decision whose R04 is measurably false for a case one field away from its own R01. What
blocks is the false invariant, and that part is squarely in scope.
For K-04's owner. The audit report's K-04 text says the direct gate "excludes compliance". Under a present
null reviewer it does not. A consolidated eligibility decision that still reads reviewerPartyRef through
.get(key, default) would inherit this hole, so K-04's regression set should include it.
Preferences
P1 — The gate's literal-true interpretation is unpinned
M2 reverts confirmed = sub.get("confirmAccept") is True to truthiness: 0 of 112 PR cases and 0 of 48
compatibility cases fail. That is expected — every supported entry passes parse_ingress_header first, so
the gate only ever sees bool or absence. M3 shows the gate does matter once the parser weakens: with int
admitted, the matrix's integer-one case returns 200 RETAIN_DRAFT instead of accepting. The RFC names the gate check as half of the containment (EXC-006: "== True
would still accept integer one") without a test that could catch its removal.
Optional: one gate-level case that calls pipeline._commit_in_tx with confirmAccept: 1 and expects
RETAIN_DRAFT, or one RFC sentence saying the parser is the only load-bearing control. Either is fine.
P2 — The new lookup/authority assertions in the stub tests cannot fail
_NoTransactionStore.serialized_tx raises, so in test_malformed_direct_header_raises_only_typed_transport_violation
and the HTTP 422 test idempotency_lookup and the authority spy are unreachable whatever the parser does:
lookup_calls == [] and authority.mock_calls == [] restate transaction_calls == 0. In
test_valid_confirmation_preserves_raw_submission_and_digest, _commit_in_tx is monkeypatched, so the same
two assertions are also inert. The real "no lookup, no replay" proof is the HTTP/Store snapshot matrix, which
is sound. The description's "Direct tests prove zero transaction/lookup/authority calls" overstates what the
stub tests add.
Checked and decided were not findings
- Other entry paths.
pipeline.commitis called only fromkernel/legacy_m1/api.py:156, 205and
kernel/demo.py:199. TheUniqueViolationrecovery path inGatePipeline.commitreuses the header parsed
before the first transaction. No path bypasses the parser. - Other
confirmAcceptreaders. Onlystages.py:61and:625outside tests;
profile_si_ffs/test_fixtures/demo_payloads.pypasses a typedbool. - Queued review adapters.
_review_submissionnever setsconfirmAccept; queue accept/reject/contest
compatibility tests pass. reviewerPartyRefof other types. Measured on the compliance probe withconfirmAccept: true:"",
["x"],{"a": 1},falseand0all route as a distinct reviewer →REQUIRE_REVIEW, 0 consequences.
Onlynullis the hole.- Operation claims with
reviewerPartyRef: null. Accepted, but D8 allows self-review for routine
operation claims, so this equals omission. - Test-module budgets.
test_ingress_normalization.py422 lines,test_review_confirmation.py284, both
underMAX_TEST_LINES = 800. - Replay of historical truthy bodies. A body accepted before this fix with
"true"now gets 422 on replay
instead ofREPLAY_REUSED_RESULT. That is the RFC's stated intent (R02), and the surface is development-only. - Stale line references in
docs/REVIEW_DISPUTE_SEMANTICS.md:92(stages.py:213, 455;api.py:188-190)
point at unrelated code. They predate this PR; the new paragraph sits beside them but does not cite them.
Could not check
- The two production-closure compatibility tests and whole-inventory regeneration:
google-cloud-kms,
cryptographyandPyJWTwheels are not in the local wheelhouse and PyPI is blocked here. - Ruff: no Linux build of the pinned 0.15.5 is reachable, so both gate scripts report the one
formatter-unavailable failure on both sides. I am relying on the PR's reported Ruff PASS. - PostgreSQL 17.10 and the prescribed hosted Linux baseline. The hosted admitted run (source 34591136665) was
not inspected; this review claims nothing about it.
What my method made easier than production
- The development HTTP adapter with
x-acting-partystands in for transport authentication. B1 needs no more
than K-03 did, but it is not shown against a deployed route, because none is open. demo.FARMERholds broad grants. A real farmer's grant set may lack compliance orREVIEW_ACCEPT; B1 needs
both, like K-03.- PG16 instead of 17.10, and 30 of 49 locked wheels. Nothing in the diff or probes depends on either, but they
are not the pinned environment. - The mutation matrix only covers the mutations I wrote. A parser defect shared by every mutation (for example
a JSON value type Starlette never produces) would be invisible to it.
Next: decide between route 1 (fix under decision v2) and route 2 (correct R04, split the fix) for B1, then an
exact-head review of that change before the admitted evidence is reused.
|
Withdraw the exact-head baseline admission for PR #384 after the newly demonstrated contractual Blocker B1 in review 5178474311: #384 (review) The source path confirms that present reviewerPartyRef: null skips both compliance self-review and distinct-reviewer routing with literal-true confirmation. This pre-existing eligibility defect contradicts approved RFC R04. The earlier zero-Blocker readiness and final acceptance packet are withdrawn. Original admission, source runs, publication receipt and historical evidence are preserved; none supplies current merge readiness while this Blocker remains. No runtime or approved invariant is changed here. The primary boundary remains legacy review-confirmation admission; eligibility correction requires separately scoped work, and a change to approved R04 requires a new decision version. No baseline rerun, new admission, merge or deployment is requested. OFARM2_BASELINE_REVOCATION |
Define the explicit R04 claim correction and known nullable-reviewer residual after review B1. Link separate eligibility Delivery #385, preserve the approved version-1 record and all runtime/test bytes, and require new same-task approval before activating the changed guarantee.
|
Bounded version-2 Phase A review for PR #384 / Delivery #383 at exact proposal head Verdict: zero design Blockers in the proposed amendment; no new Follow-ups or Preferences. This is an AI-authored bounded design review of B1 and the affected guarantee/proof claims, not a restarted audit, implementation review, human approval, baseline admission or merge authorization. Existing version1 R04 is refuted; the changed version2 guarantee remains unapproved. The sole new committed file is the212-line The amendment correctly treats B1 as a contractual failure despite its pre-existence. It explicitly changes R04 to the actual preserved authorization/evidence/retirement/routing controls and records present-null compliance self-acceptance as incorrect and unresolved. R01-R03/R06 remain intact; R05's unchanged behavior is distinguished from full direct/queue consistency. D8/D17 and other domain authorities are not relaxed. The material proposed tradeoff is visible: after exact version2 approval and fresh checks/final acceptance, PR384 may merge before #385 is fixed, leaving that known repository defect. This does not authorize the defect, production activation or a deployment waiver. #385 owns self-review eligibility enforcement and needs its own design, PR, approval and evidence. The confirmation PR remains one boundary; no combined-authority exception is requested. EXC001-006 remain satisfied: existing parser/gate only, no added state/framework/authority, evidence tied to scoped guarantees, prior truthiness reads stay removed, no new abstraction, and a separate eligibility prerequisite is the explicit alternative. P1/P2 remain Preferences and the proposal corrects their proof attribution without private-entry tests or a compatibility test that would require wrongful acceptance to remain. Executed preparation evidence: mandatory package/architecture PASS, zero failures,9.24s after issue binding and before commit; proposal/staged whitespace PASS; protected-byte comparisons PASS. The fresh extraction diagnostic remains FAIL(2) for the two previously recorded missing review records, with no new diagnostic failure and no waiver. No runtime test, database, mutation probe, expensive baseline or publisher was requested for this design-only amendment. Historical f2a executions are not current admission or evidence of eligibility completeness. Original revocation and historical records remain preserved. After the later exact version2 approval, active claims must be corrected and reviewed at the resulting head before fresh admission, hosted baselines, publication, receipt and final user merge acceptance. This design verdict does not restore technical readiness now. Next: present the complete same-task |
samovers
left a comment
samovers
left a comment
There was a problem hiding this comment.
PR #384 — bounded version-2 re-review
Verdict: no new design Blockers in the proposed version-2 amendment. Ready for the version-2 decision card, but not cleared for merge.
Reviewed head b245529179329367412a5283dc41a0b7a61b4579, including the delta from implementation head f2a3930eab90037a21f8aa05c29f506a49037d6f, against base b6017da1dbfac80b5c2e98641aaa34d96691d087. The latest delta adds only the 212-line version-2 RFC; runtime, tests, inventory, original RFC, and active semantics documentation are unchanged. The PR remains a draft with version-2 approval pending.
This is an AI-authored bounded design re-review, not independent human review, semantic approval, baseline admission, new runtime verification, or merge authorization. It does not clear the outstanding merge-level B1 merely by reviewing its proposed disposition.
B1: the proposed disposition is valid, but the defect remains
The later B1 finding in review 5178474311 is valid. My earlier design review missed the interaction between the two reviewer-field predicates.
The current code still allows a present reviewerPartyRef: null to bypass both routing checks: the compliance check compares None with the acting party, while the distinct-reviewer check explicitly excludes None. With sufficient authority and evidence, literal-true confirmation can consequently reach self-review promotion. The boolean parser correction does not repair that eligibility defect.
Source: kernel/stages.py, ReviewPromotionGate.run, at the reviewed head; B1's execution evidence is attributed to review 5178474311, not rerun here.
Version 2 now handles this honestly. It withdraws the universal self-review guarantee, identifies the concrete residual defect, and treats the narrower R04 as a decision-level change requiring new approval, not an editorial correction. It also states the consequential sequencing choice plainly: PR #384 could merge before #385 is fixed, leaving the bypass present. It does not redefine that behavior as permissible or authorize production use.
That is an acceptable proposed resolution of the false guarantee. It is not remediation of the underlying defect, and B1 is not yet cleared for merge at this head.
Source: docs/rfcs/OFARM_Legacy_Review_Confirmation_RFC_v0_2.md, particularly “Problem and decision,” “Closed version-2 invariant set,” and “Known residual and separate Delivery.”
The separate Delivery is substantive and sufficiently bounded
Issue #385 owns an independently testable eligibility correction—not merely a documentation placeholder. Its acceptance criteria cover null, omitted, self-named, and distinct-body-named reviewers; queued self-review refusal; lawful distinct-reviewer acceptance; permitted operation/structure controls; and actual stored review/consequence absence. It explicitly distinguishes new promotion from replay of historical accepted requests and does not inherit #384's approval.
The amendment also correctly prohibits adding a compatibility test that requires wrongful null acceptance to continue. The separate repair should establish a negative regression, not turn the current defect into a protected behavior. No broader eligibility framework or generic reviewer-field validator is necessary to resolve this finding.
Sources: Delivery #385 acceptance criteria and non-goals; version-2 RFC, “Known residual and separate Delivery.”
P1 and P2: the evidence descriptions are corrected appropriately
P1 — exact-true interpretation: the amendment now recognizes that the strict parser is the control that enforces confirmation typing for supported callers. Once inputs are restricted to booleans or omission, supported-entry tests cannot independently distinguish is True from truthiness at the gate. Keeping the explicit expression is appropriate; inventing a private-entry guarantee solely to make a mutation test fail is unnecessary.
P2 — ordering versus effects: the no-transaction stub establishes rejection before transaction entry. Its lookup and authority assertions do not constitute independently exercised downstream controls. The real database snapshots establish absence of durable changes and replay records—not absence of read-only lookups by themselves. The revised description respects that distinction.
Both remain non-blocking. No additional test machinery is required for this proposed documentation correction.
Sources: kernel/stages.py::parse_ingress_header; kernel/tests/test_ingress_normalization.py (_NoTransactionStore and its consumers); kernel/tests/test_review_confirmation.py (snapshot/matrix controls); version-2 RFC, “Smallest complete change and verification.”
What still prevents merge clearance
The active docs/REVIEW_DISPUTE_SEMANTICS.md confirmation paragraph, approximately lines 99–111, still carries the old review-eligibility wording and links to version 1. Leaving it unchanged is consistent with this being an unapproved proposal, but adding the amendment alone does not complete the claim correction. After valid version-2 approval, that paragraph/link and the other active claims must be aligned with the narrowed guarantee, while retaining version 1 as historical evidence. This work is already specified in the amendment.
I also verified that admission revocation 5634395226 remains present and unedited. It withdraws the previous readiness/final packet rather than erasing historical evidence. The subsequent candidate still needs the prescribed exact-head review, fresh verification/publication, and separate final merge authorization. This review creates no admission and does not restore the old one.
Sources: docs/REVIEW_DISPUTE_SEMANTICS.md, “Explicit confirmation”; version-2 RFC verification sequence; revocation comment 5634395226; root AGENTS.md approval, review, admission, publication, and final-acceptance rules.
Validation limits and disposition
This was a bounded review of the amendment, B1's disposition, affected code/tests, and linked issues. I did not rerun runtime tests, database probes, package checks, or hosted baselines. The reported extraction FAIL(2) remains disclosed; this review neither verifies it afresh nor waives it. Historical test and publication results are not fresh verification of this head.
Disposition: no requested changes to the proposed version-2 design; no new Follow-ups or Preferences. B1 has a valid proposed disposition, but approval and the active-claim correction are still outstanding, and the eligibility defect remains open in #385. Proceed to the complete version-2 decision card, not baseline admission or merge.
Activate decision OFARM2-LEGACY-REVIEW-CONFIRMATION-001 version 2 for PR384 and correct active review-confirmation claims. Preserve the original record, runtime, tests and inventory. Disclose the unresolved eligibility defect in separate Delivery385 and the approved integration sequence.
PR #384 version 2 bounded implementation reviewAI-authored coordinated review, not independent human approval. Verdict: zero demonstrated in-scope Blockers at Exact scope and identity
This is the one bounded implementation review of the approved B1 claim correction and its affected guarantees, building on the earlier implementation and Phase A reviews. It does not restart the kernel audit or re-review unrelated runtime behavior. I read the two-document delta, the approved decision and formal review, the active metadata, and recorded check results. I ran no tests, database commands, workflows or network requests, and changed no repository source. This report is local review evidence outside the PR. BlockersNone demonstrated within this approved correction. B1 disposition: the former universal R04 self-review-eligibility guarantee was false, and pre-existence did not excuse that contractual failure. The later task-user approval explicitly changed that guarantee. The active semantics document now says that literal true supplies confirmation, preserves the existing authority/evidence/retirement/routing paths, and does not certify complete self-review eligibility. It prominently identifies the reachable The controlling implementation text is Workspace Follow-ups and PreferencesKnown separate Follow-up #385 remains unresolved. Otherwise authorized and evidenced compliance self-acceptance with present null reviewer metadata remains incorrect under the existing review rules. This PR neither fixes it nor certifies it safe. Delivery #385 owns the independent eligibility correction and still requires its own high-risk design and approval. The explicitly approved sequence permits PR #384 to merge before #385 is fixed only after fresh required gates and separate final merge authorization. This is the approved scope disposition, not a claim that B1's runtime counterexample disappeared. P1/P2 proof attribution is corrected. The RFC at The EXC001–006 rationale at RFC Approval and metadata bindingI read Formal design review 5186503590 accepted the v2 design and required this post-approval active-claim alignment. I checked that required alignment here; I do not treat the earlier design review as implementation approval. The final local PR and Delivery #383 bodies contain the same contract, exact head, residual, approval provenance and pending fresh gates. Each body's bytes match the saved published readback Verification and limitsThe recorded package/architecture check passed with zero failures in 7.58 seconds against the candidate documentation before commit: its record correctly names parent b245 and The fresh extraction diagnostic at exact 9b37 remains FAIL 2, exit 1: missing review records for the root test inventory and Historical 162 local passes and the old hosted baseline/publication evidence belong to f2a and missed B1. Their current use remains revoked; no old admission, attempt or receipt is restored by this review. The root separately reports exact-head lightweight run 34696051169 attempt 1 PASS with 243 tests and package/architecture, Ruff and whitespace success; I did not independently execute those checks or inspect the live workflow during this review. Fresh immutable baseline admission, source/publisher/receipt verification, final state checks and the exact visible final acceptance packet remain required under existing procedures ( SHA-256 bindings
Next: record this exact-head zero-Blocker verdict and use the existing fresh admission/evidence path, preserving the old revocation and the separately approved final merge gate. |
|
Admit PR #384's completed version-2 correction at the reviewed current head for the existing hosted baselines and trusted publication path. Bounded exact-head implementation review reports zero demonstrated in-scope Blockers under the verified version-2 same-task approval. Package/architecture, whitespace, protected-byte checks and lightweight workflow34696051169 attempt1 passed (243 tests). The known nullable-reviewer eligibility defect remains unresolved in separate Delivery #385 under the explicitly approved scope and sequencing. Extraction FAIL(2) and its existing applicability limit remain disclosed. This is fresh technical admission only, not human approval, eligibility remediation or merge authorization. Preserve historical admission5633333452 and revocation5634395226 unchanged; neither is reused for this head. OFARM2_BASELINE_ADMISSION |
samovers
left a comment
samovers
left a comment
There was a problem hiding this comment.
PR #384 — bounded final review
Verdict: no demonstrated in-scope Blockers. The version-2 claim correction is implemented, and the fresh evidence is consistent with the reviewed head. Ready for final user acceptance under that scope—not an authorization to merge.
Reviewed 9b37d12cc2211b50e66eb3abe8c3e58b8e96f402, against base b6017da1dbfac80b5c2e98641aaa34d96691d087. The pre-publication readback still shows this head, open and non-draft. This is an AI-authored review, not independent human approval, semantic approval, baseline admission, or merge authorization. No workflow was started or rerun by this review.
1. B1’s required claim correction is complete
The outstanding work from the previous review is now implemented in docs/REVIEW_DISPUTE_SEMANTICS.md:99–117. The active paragraph no longer implies complete self-review eligibility enforcement. It explicitly identifies the reviewerPartyRef: null compliance bypass, links to #385, and points to the narrowed version-2 guarantee.
The version-2 RFC likewise replaces the refuted R04 while preserving version 1 as historical evidence. It distinguishes confirmation typing from reviewer eligibility and states the merge-before-#385 tradeoff without changing D8/D17 or permitting production use. This resolves the in-scope contractual overclaim; it does not fix the runtime counterexample.
The latest commit changes only those two documents: 46 additions and 25 deletions. Comparison against the original implementation head f2a3930eab90037a21f8aa05c29f506a49037d6f also confirms that the runtime, owned tests, generated inventory, and version-1 RFC have not changed during the version-2 correction. No eligibility repair or unrelated authority change was slipped into this PR.
2. P1/P2 are addressed without unnecessary machinery
The active explanation now correctly identifies the parser as the control enforcing confirmation types for supported callers. It does not claim that supported-entry tests independently prove the gate’s is True expression against a truthiness mutation.
It also distinguishes pre-transaction ordering evidence from durable-state evidence: the stub’s untouched lookup/authority spies follow from rejection before transaction entry; PostgreSQL snapshots prove absence of durable changes and replay records, not counts of read-only lookups. No private-entry tests, new abstractions, or tests preserving wrongful acceptance were introduced to satisfy these Preferences.
This is consistent with EXC-001 through EXC-006: retain the existing confirmation path, add no duplicate authority or state, align guarantees with their actual proof, withdraw superseded active claims, introduce no new abstraction, and keep the separately approved eligibility prerequisite visible as the alternative. P1/P2 remain non-blocking.
3. Fresh evidence checked beyond the PR description
I downloaded the published baseline artifact 10299198233 and publication receipt 10298589915, recomputed their archive SHA-256 digests, and matched them against GitHub’s artifact metadata. Both matched. The retained archive and result checks were also repeated before posting this review.
- Baseline ZIP:
d4c301cf150f5fbbedd98e7f6728231522e0b741dbbf3a10e679e23b8250e69b. - Receipt ZIP:
2f19359d9bf5b025b176be994fb9bebe09d97c1d81a592ef330261684af088c3. - Receipt JSON:
181f7d5bfbf89cdb751ae3a2d06fda26645278d45d0d23ae6291d72486a086b7.
| Check | Result |
|---|---|
| Baseline run 1 | 4,475 unique tests executed and passed, with no failures, errors, skips, or unavailable cases. |
| Baseline run 2 | Same complete passing inventory. |
| Focused confirmation coverage | 112 confirmation/ingress cases present in each run. |
| Result integrity | Standalone result-file digests match their evidence envelopes; embedded and standalone results agree. |
| Equivalence | Independently confirmed after excluding only the four declared volatile fields: /run/startedAt, /run/finishedAt, /environment/ci/runId, /environment/ci/runAttempt. The two standalone test-result files are byte-identical. |
| Execution binding | Both reports identify the expected execution merge and reviewed tree, with unchanged, clean start/end state. |
These checks corroborate the published baseline summary; they are verification of retained execution evidence, not new runtime test executions by me. No downloaded code or native image was executed.
I also checked the fresh, unedited admission 5646164779 and its binding in the receipt. The receipt and baseline admission objects agree, including the reviewed head and admission-body digest ece197a74dc8f0c3fbf0b36a18e47fac6a7657a7b265cd94a7fb848ce263d41c.
GitHub’s execution-merge object has the expected base/head parents and tree 17b134856f75391df50cf2dc298dc3bcef6c6208. The source run 34696375101 and publisher run 34697972534 job listings report successful conformance, native AMD64/ARM64, and publication jobs. I did not download or re-execute the native image archives. This is not a claim to have independently repeated every producer, publisher, or final mutable-state check.
Remaining limitations and separate work
#385 remains an unresolved Follow-up. An otherwise authorized and sufficiently evidenced compliance asserter can still self-accept using literal-true confirmation and a null reviewer field. Its separate issue retains concrete negative-regression requirements and remains unimplemented. Passing #384’s tests must not be represented as demonstrating that this behavior is safe.
The extraction diagnostic remains FAIL(2), not PASS. conformance/CONFORMANCE.md documents that check as conditional on extraction-inventory consistency changes. This PR does not change that inventory/status lane, and the amendment does not purport to repair or waive the existing missing review records. I found no new extraction-related blocker in this delta. I did not independently rerun that diagnostic or the local package checks.
Approval verification remains a separate responsibility. I inspected the committed approval provenance and posted bounded review record; I did not independently retrieve the original Codex card and user message. Those originals—not the RFC, this review, or successful CI—remain the authority for the version-2 decision and the later exact-head merge authorization. The original-task operator must recheck the required authority and live gates immediately before any merge.
Disposition: no requested changes, no new Blockers, Follow-ups, or Preferences. B1’s claim-correction requirement is satisfied within the recorded version-2 scope. The separate eligibility defect stays open in #385; final merge authorization remains outstanding.
samovers
left a comment
samovers
left a comment
There was a problem hiding this comment.
PR #384 — re-review at exact head 9b37d12
Verdict: B1 is paid. Zero Blockers, 1 Preference. Classified with AGENTS.md "Review classifications".
I raised B1 in review 5178474311
against the claim, and named two acceptable routes. Route 2 was taken: decision version 2 narrows R04,
docs/REVIEW_DISPUTE_SEMANTICS.md now states the residual, and Delivery #385 owns the fix. That closes what
I blocked. The defect itself is unchanged and I re-reproduced it at this head — as the amendment says it is.
- Head
9b37d12cc2211b50e66eb3abe8c3e58b8e96f402, base andmainstillb6017da1dbfac80b5c2e98641aaa34d96691d087. f2a3930→9b37d12is documentation only:docs/REVIEW_DISPUTE_SEMANTICS.md+11/−3 and the new
docs/rfcs/OFARM_Legacy_Review_Confirmation_RFC_v0_2.md(225 lines, 15,566 bytes). Activation commit
b245529→9b37d12is +46/−25, matching the description.git diff --check: clean.- SHA-256 at
f2a3930and9b37d12, independently computed:kernel/stages.py33523ab5ceb9730e…,
kernel/tests/test_ingress_normalization.pycfa91e6987eec11a…,
kernel/tests/test_review_confirmation.py4aca78de24bb5be1…,
conformance/review_baseline_test_inventory.jsonca289e3cedf40193…, RFC v0.12c84fb02ffb99e7f…—
all identical. So my earlier execution evidence atf2a3930still applies byte-for-byte, and nothing
was slipped in beside the correction.
Where this pass sits
Two further samovers reviews exist since mine: 5186503590 at the proposal head b245529 and the bounded
final review 5187418363 at this head. This pass confirms their central claims by re-execution rather
than inspection, and adds two things neither did: a present-null sweep across the other caller-controlled
submission keys (the systematic form of B1), and the supersession-marker point below. It is a samovers
object, so it is still not an independent-human review.
Re-measured at 9b37d12
Same interpreter and cluster as the first pass: CPython 3.12.13 built from the GitHub tag, Linux x86_64,
PostgreSQL 16.13, 30 of 49 locked wheels (cryptography, PyJWT, psycopg-pool, google-cloud-kms
and the pinned Ruff still unreachable — PyPI is blocked here).
pytest kernel/tests/test_ingress_normalization.py kernel/tests/test_review_confirmation.py
112 passed, 1 warning in 17.89s
pytest <the 48 reachable compatibility selectors>
48 passed, 1 warning in 25.48s
run_review_baseline._load_test_inventory -> canonical, entryCount 4475, entriesSha256 160c1ac7…
B1 still reproduces, exactly as disclosed. Compliance assertion, confirmAccept: true, asserter =
demo.FARMER, via the real legacy HTTP adapter:
reviewerPartyRef |
outcome |
|---|---|
| omitted / self / distinct string | REQUIRE_REVIEW HUMAN_APPROVAL_REQUIRED, 0 consequences |
null |
PROMOTE_ACCEPTED, 1 COMPLIANCE_STATUS_ACCEPTED, ReviewDecision decidedByPartyRef = the asserter |
The amendment's description of the residual matches what the code does, including "reachable accepted
compliance consequence, not just an inaccurate label".
New: the present-null sweep
B1 is a presence-versus-value defect. I sent null for ten other caller-controlled keys on an otherwise
valid, otherwise-acceptable operation claim through the real HTTP route, to see whether any other key turns a
refusal into acceptance:
key = null |
result |
|---|---|
subjectType, subjectRef |
RETAIN_DRAFT IDENTITY_UNRESOLVED |
ingressChannel |
422 |
payload |
RETAIN_DRAFT EVIDENCE_INSUFFICIENT |
targetScopes, requestedPromotionTarget, evidenceRefs, eventTime, actingAgentRef, aiAssistance |
PROMOTE_ACCEPTED — same as omitting the key; subject still {FIELD, field:demo.kmetija.a.gerk-1000001} |
Nothing else converts a refusal into acceptance, and none of the accepted cases loses subject identity. So
#385's boundary as written — the reviewer field — is the right size, and this PR does not need a generic
reviewer-field or null-input validator to be complete. (Limits: one commit class, ten keys, this fixture.
emission.py:52-53, 206-207 still read sub.get("subjectType", "FARM") / sub.get("subjectRef", …) with
non-None defaults; they are refused upstream today, which is the already-recorded wrong-typed-subject
Follow-up, not a new finding.)
Evidence claims I checked independently
Read-only, from the GitHub API; no workflow started or rerun.
| Claim in the description | What I found |
|---|---|
| Delivery #385 owns the eligibility fix | Open, "Prevent nullable reviewer metadata from permitting legacy compliance self-acceptance", body cites PR #384, review 5178474311 and #180 |
| Revocation 5634395226 untouched | created_at == updated_at, 2026-09-11T12:24:45Z (12 minutes after B1) |
Fresh admission 5646164779 unedited, body SHA-256 ece197a7… |
unedited; recomputed body digest ece197a74dc8f0c3fbf0b36a… — matches |
| Bounded review 5646162894 | present, unedited, 2026-09-12T13:24:43Z |
| Lightweight 34696051169, source 34696375101, publisher 34697972534, ready-review 34698129108 | all completed / success, attempt 1; lightweight and ready-review at head 9b37d12, baseline and publication on policy b6017da (as pull_request_target implies) |
| Receipt artifact 10298589915 | exists, not expired, 1,398 bytes, belongs to publisher run 34697972534 |
Execution merge 3c27170d, reviewed tree 17b13485 |
parents are exactly b6017da (main) and 9b37d12; its tree equals git rev-parse 9b37d12^{tree} = 17b134856f75391df50cf2dc298dc3bcef6c6208. The hosted baseline therefore ran this head's exact content, which is the claim that matters most and is checkable without any artifact download |
Not checked: the artifact ZIP/JSON digests (download needs credentials I do not use here) and the hosted
logs themselves. The final review 5187418363 reports recomputing those two digests; I neither confirm nor
contradict it.
Disposition of my earlier findings
- B1 — paid, as a claim correction. Both active documents now scope R04 to the controls that exist, name
the counterexample, and state the merge-before-#385 sequence as the approved tradeoff. The amendment
treats it as a decision-level change with its own approval rather than an edit, which is the right weight.
I take no position on whether merging before #385 is wise — that is the task user's call, and the
description now puts it in front of them in plain words. - P1 — accepted, and I withdraw my suggestion. The amendment's rebuttal is correct: inventing a
private-entry test (_commit_in_txwith parser-invalid input) to make a mutation fail would manufacture a
guarantee about an unsupported entry point. The measured fact stands (truthiness andis Trueare
indistinguishable over parser-admitted inputs); the attribution is now accurate, which is all P1 asked for. - P2 — accepted. Active descriptions now separate pre-transaction ordering evidence from durable-state
evidence, and no longer say the stubs prove zero lookup/authority calls.
Preference
P3 — RFC v0.1 still reads as an active approved decision with a refuted R04
docs/rfcs/OFARM_Legacy_Review_Confirmation_RFC_v0_1.md is byte-identical to f2a3930. Its header still
says "Status: approved version-1 implementation decision for [PR #384]" and its R04 row still reads
"Literal true cannot bypass existing acceptance requirements", with no pointer to v0.2. Only v0.2 links
back to it, so a reader who reaches v0.1 first — from the version-1 approval trail, or from the earlier
comments that link that path — sees the refuted guarantee with no marker.
The repo already has a convention for this, set by this author one Delivery ago: after B1 on PR #382,
docs/rfcs/OFARM2_Authenticated_Source_Artifact_Publication_RFC_v0_1.md gained a four-line blockquote —
"Withdrawn after [review B1] … The original version-1 text below is preserved as history" — with the body
untouched. The same shape here would cost four lines and preserve the record exactly as v0.2 requires.
I considered calling this a Blocker and decided against it, for two reasons I want on the record. The
#382 case withdrew the whole version-1 card, whereas here version 1's approval and implementation stand and
only R04 is amended; and v0.2's approved text says the version-1 record "remain[s] unchanged", so adding a
header is a decision the user made, not an oversight I should override. If the marker is deliberately
omitted, saying so in v0.2 in one clause would close the question for the next auditor.
Checked and decided were not findings
- No runtime or test drift. Verified by digest, not by reading the diff.
- The new semantics paragraph. Accurate on every point I can measure: parser rejects before transaction
and lookup, actor-binding precedence, fixed 422, raw digests, the null-reviewer defect, #385 ownership,
production routes closed (kernel/api.pystill 503s the governed surface). - R04 v2's enumerated controls. Missing
REVIEW_ACCEPT, insufficient evidence, omitted/self-named
compliance, distinct non-null reviewer string, non-allow retirement, operation/structure positives and
server-time accountability: all still covered by the 112 + 48 passing cases at this head. - R05 v2's new final clause ("no claim … direct and queued eligibility are fully consistent") is
narrower than the code, not broader — K-04 stays visible. - "Do not add a compatibility test that requires wrongful acceptance to continue." No such test exists in
the diff; the two owned test files are unchanged. - Description arithmetic. +46/−25 for the activation; v0.2 was 212 lines at
b245529and is 225 now.
Could not check
- Ruff, the two
test_application_runtime.pyproduction-closure cases, whole-inventory regeneration: same
missing wheels as the first pass. - PostgreSQL 17.10, the prescribed hosted baseline logs, and the artifact archive digests.
- Whether the version-2 card and approval messages exist as quoted; those live in the task transcript, which
I cannot read. I checked only that the PR's own quoted approval text and identifiers are self-consistent.
What my method made easier than production
- Same as the first pass: development HTTP adapter with
x-acting-party, broadly granteddemo.FARMER,
PG16 rather than 17.10, and a partial locked-dependency set. - The null sweep used one commit class and ten keys chosen by grepping
.get("…", <non-None default>)in
kernel/. A presence-versus-value defect behind a key I did not grep, or on another class, would be
invisible to it.
Next: this is the task user's decision — merge PR #384 with the null-reviewer defect disclosed and #385
open, or require #385 as a prerequisite. Nothing technical in this head blocks either choice.
1 participant
Completed — 2026-09-12
PR #384 merged at
e50ae95f43d0e73b12113463d0e4ea85eebdc14bon 2026-09-12T18:35:52Z through GitHub's normal merge operation with expected head9b37d12cc2211b50e66eb3abe8c3e58b8e96f402. Delivery #383 is completed. The resulting commit has the expected base/head parents and reviewed tree17b134856f75391df50cf2dc298dc3bcef6c6208.Same-task original final packet
msg_0d813f8071772f9a016aa55e9a1ac887d2a9a508a7d55c09f4preceded exact user merge authorizationmsg_01a096e5-d42b-7033-9b1d-9e0c17d166a6. Immediately before merging, the operator retrieved these originals, checked the absence of a later stop or conflict, and rechecked current head/base, scope, reviews, applicable checks, admission, publication and all ten artifact bindings. Both new formal reviews (5187418363 and 5187468711) reported zero Blockers. P3 remains deliberately unimplemented because the approved v2 record preserves v1 unchanged.Scope stayed within legacy review-confirmation admission. Existing baselines remain 4,475 passed twice, with equivalence, platform, native and receipt verification passed; no new runtime tests or baseline reruns were performed for this merge. Extraction remains FAIL (2) under its recorded applicability treatment. Production governed routes remain closed.
Delivery #385 remains separate, unresolved and OPEN; its implementation is unapproved. GitHub closed it at the merge, and the operator restored it to open with its title and body unchanged. The pre-merge description contained a negated issue-closing phrase; the current metadata uses neutral wording to avoid that trigger. Original pre-merge records and the final acceptance packet remain preserved in the same-task evidence.
Next: address #385 through its own scoped design and approval. This merge supplies no implementation authority for that work.
Historical pre-merge packet record
The following text records the state before the final merge authorization and merge above.
Current state: version 2 implemented and verified; final user decision required
Closes #383 when finally merged. Candidate head:
9b37d12cc2211b50e66eb3abe8c3e58b8e96f402, based onb6017da1dbfac80b5c2e98641aaa34d96691d087. PR #384 is open and non-draft. All applicable technical and evidence gates passed; final live review/currentness checks passed at 14:12:15 UTC. The complete exact-head task packet is ready for the separate user decision. The user approved decisionOFARM2-LEGACY-REVIEW-CONFIRMATION-001version 2 in the same task. This is semantic implementation approval, not final merge authority.The version-2 amendment and active review semantics now state the corrected guarantee and unresolved defect. This activation changed only those two documents, with 46 additions and 25 deletions. The runtime, two owned test files, 4475-entry inventory and version-1 RFC remain byte-identical to
f2a3930eab90037a21f8aa05c29f506a49037d6f.The earlier final packet and admission remain withdrawn following B1 review 5178474311. Revocation 5634395226 is untouched. Bounded exact-head implementation review5646162894 reports zero demonstrated in-scope Blockers under the approved version-2 scope. Fresh unedited admission5646164779, source34696375101 and publisher34697972534 attempt1 passed; receipt10298589915 was independently consumed and verified.
Problem, outcome and boundary
K-03 allowed non-boolean truthy input to act as confirmation. The implementation rejects every present non-boolean
confirmAcceptbefore pipeline transaction entry and uses one local exact-true value for the four confirmation consumers. Omission and literal false retain the existing non-confirming path; literal true supplies confirmation. Raw submission bytes and digests remain unchanged.The original R04 claim went too far. With otherwise sufficient authority and evidence,
confirmAccept: truewithreviewerPartyRef: nullcan bypass compliance self-review refusal and distinct-reviewer routing, producing accepted compliance force. The reviewer executed this counterexample at both base and the original implementation head; local source inspection confirmed the unchanged branch path. Its pre-existing origin does not make the old guarantee true.This PR has one capability and one primary trust boundary: legacy review-confirmation admission. Version 2 narrows R04 to the actual controls and evidence. Delivery #385, a separate native child of #180, owns self-review eligibility enforcement. This PR does not implement or authorize that correction.
The approved sequence permits PR #384 to merge before #385 is fixed, after this PR completes fresh verification and receives separate final exact-head user authorization. The null-reviewer compliance defect can remain in that repository state. This is still incorrect behavior under D8/D17, not permission for compliance self-review or a claim that the legacy path is safe. The approval does not authorize deployment or #385 implementation. Production governed routes remain closed. A separately approved eligibility prerequisite is the alternative; no cross-boundary exception is requested here.
Approved guarantees and containment
R04 is a changed decision-level guarantee. R01–R03/R06 retain their guarantees; R05 behavior is unchanged with its existing limitation explicit. The version-1 decision remains preserved as history.
Canonical OFARM/D8/D17 retain review meaning and eligible classes. Transport retains actor binding; the parser owns confirmation shape; existing evaluators, validators and policies retain rights, evidence and eligibility; the gate/emitter retain review records and server times; Store retains transactions and replay. The task user owns semantic and final merge decisions. Reviews, CI, publication and GitHub supply findings, evidence, custody and native state, not user authority.
The protected properties are draft versus accepted force, accountable review records, immutable history and valid replay. Request fields, body-named reviewers and keys remain untrusted even from otherwise authorized parties. Compromised runtime code, direct database writers, stolen keys and database-owner compromise remain excluded. The strict parser contains the arbitrary-truthy confirmation risk; neither that parser nor the exact-true expression proves complete reviewer eligibility.
Small scope and evidence attribution
The approved activation changes the amendment, active confirmation documentation/link and current PR/Delivery/evidence claims. It adds no runtime path, stored state, abstraction, authority or fallback. No new tests are needed merely to change the wording. Never add a compatibility test requiring wrongful null acceptance to persist; #385 owns the negative regression and separately approved implementation.
EXC-001/002: retain one parser/gate path without duplicate authority, validation or state. EXC-003: bind each guarantee to its actual proof and disclose the residual. EXC-004: retain removal of the four truthiness reads and withdraw superseded public readiness and universal-safety claims. EXC-005: add no kernel or test abstraction. EXC-006: a separate eligibility prerequisite remains the credible alternative; combining its independent allow/deny correction is unnecessary.
P1 and P2 remain Preferences, addressed through accurate attribution. P1: supported-entry tests cannot distinguish the gate's exact-true expression from truthiness after the parser has admitted only booleans or omission; the expression is explicit interpretation, not a demonstrated second independent type barrier. P2: no-transaction stubs prove early ordering; zero lookup/authority spy calls follow from that boundary, rather than independently exercising those controls. Real PostgreSQL snapshots prove absence of durable record changes and replay records, not counts of read-only lookups. No private-entry tests or extra machinery are added for these preferences.
Completed verification and limits
ece197a74dc8f0c3fbf0b36a18e47fac6a7657a7b265cd94a7fb848ce263d41c.detect_leaks=0.2f19359d9bf5b025b176be994fb9bebe09d97c1d81a592ef330261684af088c3; JSON SHA256181f7d5bfbf89cdb751ae3a2d06fda26645278d45d0d23ae6291d72486a086b7.7d3988eaae3db171377e2c8f8522ce6168cf330beda41ebaae3a48c736d2baa6. Source/publisher policy isb6017da1dbfac80b5c2e98641aaa34d96691d087; execution merge3c27170d009e377a89bce5377152f3e81781fa4fhas expected parents and reviewed tree17b134856f75391df50cf2dc298dc3bcef6c6208.Both baseline runs used the same three disposable PostgreSQL 17.10 clusters, with verified pairwise isolation between test, security-audit and provisioning lanes, under Linux x86_64/CPython 3.12.13 and pinned dependencies. Observed cleanup removed all clusters and builders. Native image archives were not locally downloaded or re-executed; the trusted publisher authenticated them and the consumer bound their metadata/digests/index. No downloaded content was executed. Source-reader authentication does not supply an uncaptured HTTP transcript; mutable state is valid only for its recorded capture.
Fresh extraction diagnostic at this candidate remains FAIL (2): missing review records for the root inventory and
kernel/tests/test_rewrite_architecture_check.py. The checker, review records and affected scanned files are unchanged; the amendment changes documentation outside scan roots. The conditional extraction-inventory PASS gate does not apply to this delta. This is an applicability determination, not a waiver or PASS claim.Historical 162 local passes remain supplemental against unchanged bytes and missed the known null-reviewer defect. Earlier source/publisher/admission/receipt results remain historical and revoked for current use; none were rerun or substituted. Broader K04, wrong-typed subjects, #180 lifecycle/shared-connection ownership, #184 graph work and extraction review records remain separate.
Non-effects and approval provenance
No canonical/reference/frozen-contract, credential/principal, grant/action, signing/custody, database-role, transaction-ownership, audit/publication mechanism, production activation, release/deployment or historical repair change. No reviewer type contract, eligibility fix, broad K04, subject-validation or #180/#184 lifecycle/graph implementation. Delivery #385 and the parent Epic remain separate unresolved work.
In task
01a07cc8-4157-7b33-a0ca-becb772e0e8b, the complete live version-2 card was original messagemsg_0d813f8071772f9a016aa3f642869487d28746b5cfca3cbf5bat2026-09-11T12:38:40.914Z. The later original user messagemsg_01a095c2-83cb-72c0-87d6-10ff38a65c80at2026-09-12T13:15:48.043Zstates:I approve OFARM2 decision OFARM2-LEGACY-REVIEW-CONFIRMATION-001 version 2.Root directly retrieved and verified both ordered originals. The wrapper approval record is navigation/evidence; those original same-task messages are the authority. No final merge authorization exists.The boolean rule is intended to endure; the legacy development surface and approved integration sequence remain provisional before deployment. A change to capability, boundary, authority, effects/non-effects, R01–R06, irreversible behavior, named PR or production posture requires a new decision version. Fixing #385 under its own approval must never be treated as a regression requiring wrongful acceptance to remain.
Next: present the complete final exact-head packet in the original task and await the user's separate exact merge authorization; immediately before any normal expected-head merge, recheck every existing gate. No admin/auto bypass, direct main push or deployment.