PR #1547 remediation follow-ups: 7 defects and 3 blind oracles introduced by the review-response commits

[numarasSigmaSoftware]

Filed from the re-review of #1547 at d82b3a3cb. Every item here targets code the REMEDIATION added, not the PR's original subject — deferred deliberately rather than fixed in-round, because each round of fixes was producing the next round's findings (this round measured 9 of 9 findings as fix-cascade). The blocker was reverted; these ride separately.

Defects

1. complete_task admits approved and in_progress — task_management.py. Deriving the pre-check from TERMINAL_STEP_STATUSES (the repository primitive's guard) admits two statuses CANCELLABLE_STEP_STATUSES excludes deliberately: approved is committed before execute_approved_media_buy, in_progress marks adapter side-effects underway. A buyer may now not cancel an in_progress step but may complete one — the more destructive of the two. Suggested: a named BUYER_COMPLETABLE_STEP_STATUSES beside CANCELLABLE_STEP_STATUSES carrying the split's reasoning; the authority for a buyer-facing terminalization is the buyer-cancel sibling, not the repository primitive.

2. Refusal detected then discarded — background_approval_service.py. _mark_approval_complete/_mark_approval_failed log the refused transition but still return None unconditionally, so the caller sends completed regardless. The buyer receives completed for a step committed as canceled/rejected. The docstring's "this polling thread is the only observer of its own writes" is false — the buyer is the other observer.

3. reject_pending_execution's CAS result is discarded — operations.py. Return value dropped, then commit + rejected webhook + success flash unconditionally. Reachable when the step is still approvable but the buy has moved on (activating/active/completed): step stamped rejected and buyer notified while the buy row is untouched. The approve path in the same function does branch on its outcome.

4. Three forwarded economics params are still dropped downstream — _build_update_request only writes currency/pacing/daily_budget when budget is also present. Measured: _build_update_request(media_buy_id=…, paused=True, currency='EUR', pacing='even', daily_budget=250.0) dumps ['media_buy_id', 'paused']. flight_start_date/flight_end_date do apply.

Blind oracles (tests that cannot fail in the direction they name)

5. The A2A forwarding test asserts one frame too early — it spies at core_update_media_buy_tool, upstream of the drop in item 4, so three of its five asserted values die downstream and it stays green. Extend the assertion past the spy to the built req.

6. The webhook characterization test is unfalsifiable — measured deliver_ok is False in 0 of 24 combinations, so assert not (deliver_ok and not register_ok) can never fail. Performing the tracked registration-tightening leaves it green. Assert the divergence positively where it exists so the eventual fix reddens deliberately.

7. The update-callback test dropped its sibling's ordering assertion — production rejects the callback before any durable write; moving the validator after the context and workflow step are persisted leaves the test green. The create sibling four lines up does grade it (context_mgr.assert_not_called()).

8. A unit test does a live DNS lookup — test_push_notification_url_validation.py resolves example.com through check_url_ssrf(resolve_dns=True). With gethostbyname raising, both shapes return REJECTED: the accept case fails while the parity case stays green — precisely the failure the file's own docstring says it exists to prevent. The sibling at test_protocol_webhook_ssrf.py already patches socket.gethostbyname.

9. The refusal branch in item 2 has no oracle at all — reverting both sites to the unconditional success log leaves pytest tests/unit byte-identical (5999 passed, 9 skipped, 26 xfailed). No test imports the module except the reaper test, which touches only the registry accessors.

Nits carried

Stale "vice versa" symmetry claim survives verbatim at webhook_validator.py:335 on a security boundary · _log_refused_transition says "already terminal" where the primitive also returns None for not-found · media_buy_list.py:251 keeps a bare code="SERVICE_UNAVAILABLE" literal while two siblings were converted · the A2A exclusion list's staleness guard checks the excluded name still exists but not that it is still dead · _record_refused_unsafe_url is an eighth hand-written copy of the delivery-logging gate in one file · test_a2a_skill_invocation.py hand-transcribes TERMINAL_STEP_STATUSES in the very test arguing against hand-listing · 5× identical skill-call scaffold in test_a2a_transport_contract.py · log_safe has no test coverage against ~62 call sites.