Repository navigation
fix: apply synthetic safety guards to retry execution evidence - #316
Conversation
Signed-off-by: Srinivasa Dinakar Jainti <dinakarjs@gmail.com>
Signed-off-by: Srinivasa Dinakar Jainti <dinakarjs@gmail.com>
|
🟡 Contributor Check: MEDIUM
Automated check by AgenTrust Contributor Check. |
altrudev
left a comment
There was a problem hiding this comment.
Harness-owner review of the bounded deterministic retry-evidence correction at 69d3ef6:
The source diff addresses the reported failure: retry-1 execution now calls the same authorization/admission/revocation/replay/missing-evidence evaluator used for main-lineage execution, rather than unconditionally reporting established. Retry receipt and admission are explicit causal predecessors. Seven positive/negative/gate-weakened comparisons cover workload, key, policy, downgrade, revocation, replay and missing execution evidence. Main-lineage timeout/response binding still use the main execution observation, preventing retry evidence from overwriting it.
Scope note: this is a source review, not an independent re-execution of the advertised 24 tests or a live-runtime acceptance claim. The PR reports passing hosted checks and an isolated mutation result; those should remain labeled as contributor-run evidence until independently replayed.
One documentation/semantics request before treating this as a stronger gate: retry-dispatch and retry-receipt are always established, even for a contradicted retry authorization. Please explicitly state that they represent scripted synthetic observations rather than authorized delivery and add an assertion that invalid authorization cannot be interpreted as permitted execution. Also consider a compound case with missing execution evidence plus rejected authorization, so unavailable does not mask a definitive refusal in any consumer interpreting individual boundaries.
I find the core retry-execution correction directionally sound and limited to synthetic evidence; no assertion of live, hardware, package, or full software milestone conformance.
altrudev
left a comment
There was a problem hiding this comment.
Independent EDRG replay on Prometheus against exact head 69d3ef6 (Python 3.13.5): python3 -m unittest discover -s tests -v PASS 24/24; git diff --check 5f9b48e... 69d3ef6... PASS. I then replaced only the retry execution result with unconditional Outcome.ESTABLISHED in a temporary working tree. The suite failed with seven subtest failures (the seven retry refusal/unknown cases). Source restored, 24/24 PASS, no remaining diff. This independently demonstrates the new regression controls detect the original retry overclaim.
Additional compound-case observations worth tightening before approval: revoked_before_use=True together with missing_execution_evidence=True reports overall unknown and retry execution unavailable, with no definitive revocation refusal in the retry execution evidence. workload=workload-X plus missing evidence produces overall refused but retry execution unavailable. Please clarify precedence between known safety rejection and missing outcome evidence, and add a compound regression asserting a known revocation cannot be misread as permitted execution. Separately, with dispatch=False on main, retry dispatch/receipt/execution are still synthetic established; please document that retries are independent scripted attempts, not proof of live authorized dispatch. This review verifies the software fix and mutation sensitivity, not live execution or full milestone acceptance.
altrudev
left a comment
There was a problem hiding this comment.
Follow-up to my independent EDRG run: please preserve known refusal over missing execution evidence. In harness/adapters/deterministic.py, execution_outcome(admitted) currently returns UNAVAILABLE as soon as missing_execution_evidence is true, before consulting revocation/replay/admission/authorization. Consequently revoked_before_use=True + missing_execution_evidence=True + retry_lineage=True yields overall unknown rather than a clear refusal. Recommended bounded fix: compute revoked/replayed first, return CONTRADICTED when not auth_ok or not admitted or revoked or replayed, and only then return UNAVAILABLE for missing execution evidence; otherwise ESTABLISHED. Document that CONTRADICTED is the synthetic authorized execution claim, not proof that no physical effects occurred. Add six compound tests (workload/key/policy/downgrade/revocation/replay plus missing evidence), verifying retry execution CONTRADICTED and overall refused, while a gate-weakening mutation yields UNAVAILABLE/unknown. Document retry dispatch/receipt as scripted synthetic observations.
The earlier independent baseline 24/24 and seven mutant kills remain valid for original 69d3ef6. I attempted to run this additional patch but Prometheus went offline before applying it, and the GitHub account has no push permission to agentrust-io/integrations or dinakarjs/integrations. Therefore this follow-up change is not applied or tested upstream; do not interpret this as approval or completion.
|
@dinakarjs @imran-siddique Follow-up correction is committed and independently tested against PR #316 head Patch (git-format-patch, based directly on PR #316 head): https://gist.github.com/altrudev/a518d58f65c0f11c74c2139f55689d0d Local commit on Prometheus: Independent evidence: 25/25 Apply on the contributor branch (from PR #316 exact head): download the patch, inspect it, and run |
Integrate the non-overlapping regression controls from Altru patch dff4a395; shared refusal precedence already landed at 6ac5fff. Clarify synthetic permitted-execution outcomes do not prove physical side-effect absence. Co-authored-by: Val Rukhaylo <altrudev@users.noreply.github.com>
altrudev
left a comment
There was a problem hiding this comment.
Independent harness-owner EDRG review of exact integrated head f372052dfe369443059fa4835c402e8972756841 on Prometheus (Python 3.14.4): 27/27 unittest tests PASS; diff whitespace/integrity check PASS. Deliberately replacing the known-guard refusal condition with False generated 29 failing tests/subtests; restoring the original source returned 27/27 PASS with no residual diff. GitHub reports 11/11 PR-triggered hosted workflows successful at this head, including CodeQL, lint, integration validation and smoke tests.
I checked the integrated correction for the previously reported issue: shared main/retry guard evaluation, compound invalid+missing-evidence cases, explicit gate-weakening twins, guarded retry execution, main timeout lineage preservation and clear separation of scripted synthetic transport from permission to execute. The prior compound finding is resolved within the stated deterministic software scope.
APPROVED for this bounded synthetic retry-evidence correction only. This approval does not establish live independently-operated peer execution, hardware attestation, released-protocol integration, or completion of the parent confidential-workflow harness milestone. Maintainer approval/merge remains under repository governance.
Retry observations in the deterministic confidential-workflow harness previously reported execution=established even when retry authorization was contradicted or inputs were revoked, replayed, downgraded or missing execution evidence. Overall refusal remained intact, but retry evidence overstated its outcome.
Share admission/execution guards across main and retry lineages and add causally bound retry receipt/admission observations. The review follow-up explicitly documents retry dispatch/receipt as scripted synthetic transport observations, which do not establish authorized delivery or permission to execute. Known guard refusals now take precedence over missing execution evidence in the shared evaluator; the missing-evidence mutation cannot override another rejecting guard. Main-lineage timeout uncertainty and response binding remain unchanged.
Seven invalid/unknown input classes retain positive twins and gate-weakening controls. New tests cover workload/key/policy authorization refusals despite established synthetic transport, plus six compound rejection classes with missing evidence, both normally and under the missing-evidence mutation. Incorporate Altru’s six compound rejecting-guard weakening controls: after removing the rejecting guard, missing evidence still yields unavailable/unknown. Clarify that the synthetic permitted-execution claim does not prove physical side-effect absence and retries remain independently scripted when main dispatch is disabled.
Requested upstream by Imran under integrations#199; follow-up addresses the harness-owner source review at #316 (review). The count-only Marketplace correction remains inherited from the initial branch.
Contributor-run validation at f372052: 27 harness tests pass on Python 3.12.14; targeted Ruff 0.15.0 and git diff --check pass. A process-only mutation restoring missing-evidence-first precedence fails all 12 new compound comparisons, with no test errors. Earlier validation at 69d3ef6 covered five catalog generator tests, --check, and seven fork workflows; those results are not asserted for this new head. All 11 hosted technical workflows passed at f372052, including lint, integration validation, documentation links, examples, demos, integrity checks and CodeQL. Maintainer approval remains required. Altru independently replayed the original 69d3ef6 and tested patch dff4a395 (https://gist.github.com/altrudev/a518d58f65c0f11c74c2139f55689d0d). Its runtime correction already matches 6ac5fff; f372052 integrates the additional test and semantics contributions with co-author attribution. Altru independently replayed integrated head f372052 on Python 3.14.4: 27/27 tests and the diff whitespace/integrity check passed. Disabling the known-guard refusal condition produced 29 failing tests/subtests; restoring it returned 27/27 passing with no residual diff. The harness-owner review approved this bounded synthetic correction and confirmed the prior compound finding resolved: #316 (review). Independent maintainer approval of the current head remains required.
Deterministic software only; no live retry, protocol API, package target, hardware or conformance claim. Maintainer review and approval required.