feat(cron): persist conservative transport receipts - #93009
feat(cron): persist conservative transport receipts#93009Kinkoolino-Hermes wants to merge 20 commits into
Conversation
andrexibiza
left a comment
There was a problem hiding this comment.
Reviewed exact head 11784164298d7b05253acb88349bea086589f0c3 against base f293e7206b4ddd66042329442c6afebc19a8808d, including the scheduler/ledger receipt boundary, Telegram + Matrix typed acknowledgements, media path, recovery/read-side projection, focused tests, existing review history, and adjacent cron/delivery work.
There is one correctness blocker on the current head:
Typed unknown / failed media receipts can still settle the target as delivered and trigger delivery-only side effects.
_send_media_via_adapter() appends every typed receipt to receipts_out, then separately records SendResult.success == False as a media error. The caller surfaces _media_errors into delivery_errors, but does not clear adapter_ok. It then calls _persist_target_text_receipts(..., components={"media"}). That helper considers a preregistered component satisfied whenever record_transport_receipt() persisted its expected key; it does not require receipt.outcome == "delivered". record_transport_receipt() intentionally persists unknown and failed outcomes and returns True when that write succeeds.
That combination is reachable through the real provider adapters added here: Telegram/Matrix can return the exact expected media receipt key with outcome="unknown" after an exception where the external write may already have happened, together with SendResult.success=False. The receipt is correctly preserved as ambiguous evidence, but the scheduler currently counts the key as fully persisted and leaves adapter_ok=True; the subsequent if adapter_ok: branch sets delivered=True and may seed/mirror the session. The execution ledger can later project the aggregate as unknown, but the target-level delivered transition and its side effects have already happened.
This violates the PR's stated invariant ('a target is delivered only when every preregistered text/media component has an explicit provider acknowledgement') and the #70945 lineage requirement that a later media failure be partial/not fully confirmed and that heuristic/ambiguous outcomes never become confirmed delivery.
Required closure:
- receipt-plan satisfaction must require every expected component to persist with
outcome == "delivered", not merely an expected receipt key; - a media
SendResult.success=False/ media error must make the target non-delivered while still preserving typedunknown/failedevidence; - do not seed or mirror the target unless all planned text/media receipts are delivered;
- add an adversarial regression using a real receipt-shaped media result with the expected key but
outcome="unknown"(and ideallyfailed) and assert target delivery is false, no seed/mirror occurs, the ambiguous receipt remains durable, and no blind resend is authorized.
The existing media regression catches a missing receipt key, but not an expected-key receipt whose terminal evidence is non-delivered, which is why this path survives the current suite.
Interlocks/provenance checked: #75876 (@Zhou-Ruichen) is the narrower live-adapter timeout/no-blind-retry precursor and remains complementary; #82782 (@brianrogstad) is the broader execution-accountability predecessor whose untyped whole-delivery evidence this PR is explicitly tightening; #90293 preserves @smfworks/Paula Rossi lineage for producer/model-route atomicity and evidence-poor execution outcomes and is adjacent rather than duplicate. #92931 is also complementary, not a substitute: it hardens cross-connection Bot Mode relay delivery with a lease-backed at-least-once outbox, while this PR is defining cron's provider-receipt trust boundary. I found no reason to collapse those ownership layers or to erase the earlier contributor lineages.
Everything else I inspected in the typed receipt shape is directionally sound: preregistration before side effects, exact target/component/ordinal binding, persistence-boundary reconstruction, conservative unknown handling for legacy evidence, and native Telegram/Matrix provider IDs are the right architecture. The blocker is specifically the media settlement predicate at the scheduler boundary.
Exact-head CI is also not yet a merge receipt: Branch CI is currently red in its validate job, with dependent quick-check/unit-test work canceled; the other observed exact-head workflows (lint, JS/TS, secrets, Dockerfile lint, history/contributor checks) were green. That CI failure should be resolved/re-run after the semantic fix above.
1178416 to
8dff48c
Compare
Resolve tools/cronjob_tools.py by retaining the PR's bounded public projection while preserving upstream gateway-liveness notices for create/list results.
Allowlist the canonical EMPTY_PAYLOAD_ERROR at the public tool boundary while keeping unexpected exceptions generically redacted.
…-transport-receipts-v2
29e0c93 to
b319446
Compare
Preserve conservative transport-receipt semantics while integrating durable incident acknowledgements, delivery-target provenance, and the bounded cron tool projection.
Fail closed instead of coercing malformed attach_to_session values that could silently enable transcript mirroring.
Apply the same strict boolean boundary to create and update so registry callers cannot silently coerce or ignore malformed attachment intent.
…3009-20260830 # Conflicts: # plugins/platforms/telegram/adapter.py # tests/tools/test_cronjob_tools.py
…3009-20260830 # Conflicts: # hermes_cli/cron.py # tools/send_message_tool.py
…3009-20260830 # Conflicts: # hermes_cli/cron.py
What does this PR do?
Adds a conservative, typed transport-receipt contract for cron delivery so Hermes can distinguish provider-confirmed delivery from ambiguous or definite failure outcomes.
A target is
deliveredonly when every preregistered text/media component has an explicit provider acknowledgement. Timeouts, legacysuccess + message_idresults, partial acknowledgements, post-ack persistence failures, and interrupted recovery remainunknownand never trigger a blind resend, mirror, session seed, or replay seed.The implementation keeps requested and actual targets separate, binds evidence to the exact job/fire/component/ordinal/content hash, and revalidates the full typed receipt at the SQLite trust boundary before persistence. Public cron surfaces expose bounded summaries; sensitive editable configuration is available only through the authenticated management-detail endpoint for an explicitly selected profile.
Related Issue
Fixes #70945
Related but not duplicate:
message_id/id, and has no preregistered per-component ordinal/content-hash plan.Type of Change
Changes Made
TransportTarget/TransportReceipttypes and additive receipt preservation onSendResult.fire_atseparate from mutable claim heartbeat time.unknownwithout key-share/send retry; Telegram photo-to-document fallback is limited to an exact typedBadRequest("photo_invalid_dimensions")rejection.targets_deliveredonly when every planned component is acknowledged at the exact preregistered platform/chat/thread; partial/fallback acknowledgements remain preserved evidence without falsely settling the requested target.How to Test
PYTHONDONTWRITEBYTECODE=1 uv run pytest -q tests/cron.web/, runnpm run check.git diff --check.Verified on the submitted head before publication:
main.main, all 54 changed file contents matched the reviewed patch byte-for-byte and 87 overlap/delta tests passed.git diff --check, scope review, and credential/private-key/token pattern scan: passed with zero findings.Checklist
Code
fix(scope):,feat(scope):, etc.)pytest tests/ -qand all tests pass — the complete Matrix file retains eight unrelated failures reproduced on clean currentmain; all affected-area gates passDocumentation & Housekeeping
docs/, docstrings) — N/A: no user-facing configuration or workflow is addedcli-config.yaml.exampleif I added/changed config keys — N/A: no config keys changedCONTRIBUTING.mdorAGENTS.mdif I changed architecture or workflows — N/A: contributor workflows are unchangedScreenshots / Logs
N/A — this is a transport-accounting and public-response hardening change; verification is covered by automated tests above.
— 🤖 Bot-authored PR description · LLM: gpt-5.6-sol · not written by a human