fix(phase4): H1 — close FAIL-OPEN static fallback to PAYG via Cost Gate - #87231
Draft
pooyan6 wants to merge 15 commits into
Draft
fix(phase4): H1 — close FAIL-OPEN static fallback to PAYG via Cost Gate#87231pooyan6 wants to merge 15 commits into
pooyan6 wants to merge 15 commits into
Conversation
…s API
Root cause: two independent bugs in convert_messages_to_anthropic()
(agent/anthropic_adapter.py), the final conversion step before every
Anthropic messages.create() call, both producing HTTP 400 "text content
blocks must contain non-whitespace text":
1. _ensure_leading_user_turn() synthesized a filler user turn with
content [{"type": "text", "text": " "}] (a single space) whenever the
built payload didn't start with role=user (e.g. after context
compaction leaves a leading assistant summary). The space is itself
whitespace-only, so the guard traded a "leading assistant turn" 400
for the "text content blocks" 400 it now hits. Fixed to reuse the
existing non-blank _EMPTY_TEXT_PLACEHOLDER ("(empty)").
2. _convert_user_message() filtered blank text blocks from list-type
user content with an all-or-nothing check:
all(blank for b in blocks if b.type == "text"). This is vacuously
true when a message has zero text-type blocks (silently destroying
valid non-text blocks like images/documents it never inspected), and
false as soon as any single text block is non-blank — which let a
*sibling* blank text block sit untouched next to valid content and
reach Anthropic as-is. Replaced with per-block filtering (mirroring
the assistant-side logic already in _convert_assistant_message),
preserving all non-blank/non-text blocks and relocating any
cache_control marker carried by a dropped block.
Also added _scrub_blank_text_blocks(), a final defense-in-depth pass run
as the last step of convert_messages_to_anthropic() (after every other
transform, including nested tool_result content lists) so a blank text
block from any current or future producer never reaches the wire. It
logs only structural metadata (message index, role, content location,
block index/type) — never message text, tool arguments, tokens, or
credentials.
An earlier local patch to sanitize_api_messages() (agent_runtime_
helpers.py) attempted to fix this by rewriting blank assistant content
before the OpenAI->Anthropic conversion step, but the real leaks were
introduced downstream of that sanitizer, inside the Anthropic-specific
converter itself — the patch never touched the actual defect and has
been fully reverted (agent_runtime_helpers.py is back to its committed
state; verified via `git diff` showing no changes).
Verified against a real Telegram message end-to-end: the gateway no
longer produces the "text content blocks must contain non-whitespace
text" error on a fresh conversation turn.
Testing:
- 9 new end-to-end regression tests in test_anthropic_adapter.py
(TestFinalPayloadHasNoBlankTextBlocks) covering content="",
content=" ", content=[{"type":"text","text":""}], mixed blank+valid
text, blank text next to a valid tool block, an assistant tool-call
message with blank content, the leading-synthesized-user-turn case,
and a blank text block nested inside a tool_result's own content list.
- Fixed one pre-existing test that had asserted the broken " " filler
behavior as correct.
- Full tests/agent/ + tests/run_agent/ suite (4671 tests) run against
both the patched tree and a stashed pre-fix baseline: identical 148
pre-existing failures in both runs (unrelated subsystems — codex
app-server integration, credential-pool interrupt handling, OpenAI
client lifecycle), zero failures unique to either side.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Add tools/self_improvement_guard enforcing that governed/orchestrator profiles cannot mutate SKILL.md/references/memory unless explicit SELF_IMPROVEMENT authorization (env HERMES_SELF_IMPROVEMENT=1 or profile config self_improvement.enabled=true). Three choke points: - _spawn_background_review: refuse to spawn the autonomy fork - skill_manager_tool._apply_skill_write_gate: refuse review-origin writes - memory_tool: refuse review-origin writes (batch + single) Non-governed profiles unaffected. Regression tests added.
…uth — env-only bypass removed (Claude review governance finding)
…ited rollout) Intercept at agent/chat_completion_helpers.try_activate_fallback: for an explicitly-marked governed PGF mission, re-run the reviewed policy chain (QuotaCollector->RoutingPolicyEngine->CostGate->Brain/Executor) instead of promoting the static fallback_providers chain. Inactive for normal chat and holding-hossein. Fails open to static chain on error (rollback). Persists routing record + quota snapshot. PAYG escalation never auto-approved; anomaly guard stops expensive calls on confirmed near-exhaustion. 8 smoke tests.
…y replan (Stage C) In run_conversation (task/turn boundary, before the first provider invocation) for governed PGF missions: deterministic task classifier -> collect live quotas -> RoutingPolicyEngine + CostGate -> Brain/Executor assignment, so the legacy default (e.g. DeepSeek/OpenRouter) is never invoked first and corrected later. Stage C replan: re-reads live quotas every task boundary, never inherits the previous session provider blindly. Free/cheap workers never promoted to Brain. Persists pre-invocation audit record (task class, quota snapshot, brain, executor, billing). Gate inactive for normal chat / holding-hossein; fails open to legacy path on any error. 16 gate tests; fallback suites green.
…tches the runtime provider/model _apply_selection no longer sets bookkeeping attributes only: it resolves the selected Brain to a concrete provider+model, swaps agent.provider/model/ requested_provider, clears stale config-context-length and transport cache, and persists an activation ledger (failed provider, selected brain, activation result, retry provider/model). route_governed_fallback returns True only after the swap applies; an unmapped/un-activatable brain fails closed (returns False) so the legacy static fallback chain is preserved, never silently overridden. Adds 3 tests, incl. the integration proof that provider A failure -> policy picks B -> the next model invocation targets B, A not retried.
…overned_brain on activation failure Closes the provider/requested_provider desync Claude flagged: if _apply_selection fails after setting agent.requested_provider to the rejected brain, the exception path now also restores requested_provider and _pgf_governed_brain, so downstream routing cannot read a stale requested provider pointing at the failed brain.
…fresh agents The prior guard-only-restore could not undo a set-to-None mutation when the agent had never had the attribute (fresh agent), leaving requested_provider pointing at the rejected brain. Switch to a _MISSING sentinel captured before mutation and a _restore_attr helper that both restores a prior value AND removes a mutation made this call (agnostic of prior None vs absent). Adds a fresh-agent rollback test; 20 gate tests green.
…ligibility - FREE worker runtime mapping (nemotron/stepfun -> :free endpoints). - route_pre_invocation dispatches FREE plans to the free model for eligible classes (mechanical/test/summarization/normal); critical reasoning never free. - FREE_QUALITY_GATES deterministic checks; free output provisional until pass. - free-utilization + escalation records persisted. - 25 gate tests green.
…temError rewrapping drop) append_message / async token accounting surfaced the CPython SystemError 'returned NULL without setting an exception' from a stale sqlite3.Connection. _execute_write now detects it, reopens the writer connection once, and retries the write; a second occurrence propagates. The DB file is healthy (integrity_check ok). Adds 3 regression tests (180 state tests green, ruff clean).
…oss-thread race) Claude review risk: reopening self._conn outside self._lock let another writer mid-use be observed during reassignment (the likely primary fault is cross-thread share of self._conn between the main path and the daemon token-writer). Wrap the reopen in the same lock the write path uses so the connection swaps atomically. Partial-write idempotency is preserved by the existing transaction rollback. 3 regression tests green, ruff clean.
…roved' no longer hijacks mechanical tasks Stage-8 canary B exposed a real defect: _TASK_KEYWORDS used unanchored substring matching, so the CODE_REVIEW keyword 'pr' fired inside 'approved'/'update', misrouting a mechanical patch task to CODE_REVIEW (Claude) and defeating the FREE execution lane. Short high-collision keywords (pr, cp, mv, sed, ls, find, date) are now matched as whole words; 'pr' as a standalone token still routes to CODE_REVIEW, and 'pull request'/'review' unchanged. Adds 5 regression tests; 30 gate tests green, ruff clean.
…stale brain on FREE dispatch
B (R1): task table stores cp/mv/ls/find with trailing space ("cp ") but
_WORD_ONLY_KEYWORDS had bare forms, so word-boundary protection never applied
to them; scp could be misread as cp -> mechanical. _matches now normalizes the
keyword for the word-only check and matches trailing-space forms as a
whole-token prefix with a non-alphanumeric boundary.
D (R1): on FREE-lane dispatch clear agent._pgf_governed_brain to None so audit
never shows a leftover Brain for a free-worker mechanical task.
Adds 5 regression tests; 35 gate tests green, ruff clean.
…emove dead stepfun Live canary (2026-08-15) verified: qwen/qwen3-coder:free and nex-agi/nex-n2-pro:free are NOT free-tier on this OpenRouter key (PAYG) so they cannot enter the pool; stepfun/step-3.7-flash:free endpoint no longer served. Pool now ranked by measured capability from benchmark: nemotron (proven) > nemotron_ultra (stronger reasoning) > north_mini_code (cohere, code-named) > gpt_oss (reserve). _FREE_WORKER_TO_RUNTIME maps each to its verified-free OpenRouter slug; all route via openrouter. _run_free_worker cascades to the next ranked worker when a pool member's mapping is missing (intra-pool resilience) but an unknown/non-pool worker still fails closed — never over-promotes to PAYG/Claude. 39 gate tests green, ruff clean.
…te (no PAYG without authorization)
Required invariant: no PAYG provider invocation may occur from a static/legacy
fallback path unless Cost Gate authorization exists first.
- try_activate_fallback: for a governed PGF mission (agent._pgf_governed_mission),
call gate_static_fallback_payg before any fallback provider/model swap. It
fires even when gate_active() is False (policy gate inactive/legacy) or errored
— closing the exact fail-open hole that caused prior ~€5 OpenRouter PAYG spend.
Outcome {OPERATOR_REQUIRED, DENIED, GATE_ERROR} => candidate marked unavailable,
NOT activated, chain skips it, operator escalation surfaced. FREE/INCLUDED pass
through unchanged; non-governed agents untouched.
- pgf_routing_gate.gate_static_fallback_payg: explicit billing-class resolution
(*:free/*:batch=FREE; claude/anthropic+openai_codex=INCLUDED; explicit PAYG set;
unknown=fail-closed) -> FREE/INCLUDED no reservation; PAYG -> evaluate_cost_gate
(default €0 budget, cumulative BudgetLedger atomic reserve under file lock)
with a STABLE decision_id keyed on provider/model so repeated fallbacks
reuse/verify the prior reservation (no double-reserve; ledger dedups by id).
Returns AUTHORIZED / OPERATOR_REQUIRED (persisted OperatorCostDecision) /
GATE_ERROR (fail-closed). Always persists fallback-gate-*.json audit with
provider/model/billing/gate-decision/decision_id/remaining-budget/result/time.
- 9 regression tests (A-I: FREE/INCLUDED passthrough, PAYG €0 blocked+operator
decision, authorized-once, gate-error fail-closed, no-double-reserve, cumulative
exhausted, legacy-PAYG-cannot-escape, non-governed preserved). 67 gate+fallback
tests green; ruff clean. Bounded Claude post-review: VERDICT YES.
Draft PR; NOT deployed.
alt-glitch
added
type/bug
Contributor
|
Nice catch on the fail-open path — the static/legacy fallback chain silently reaching PAYG without Cost Gate authorization is the same seam the fallback-family adjudications mapped earlier this month (#80423 and #81209, with #82421 as the endorsed successor), so this closes a real hole in that lineage. Worth a cross-link on review so the family history is visible; otherwise the boundary (authorization before any PAYG egress from a static chain) looks right. |
Contributor
fix(phase4): H1 — close FAIL-OPEN static fallback to PAYG via Cost Gate
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
H1: No PAYG from static/legacy fallback without Cost Gate authorization
Closes the fail-open path that caused prior ~€5 OpenRouter PAYG spend (a governed mission whose FREE plan had no runtime mapping → NO_PROVIDER → static fallback chain → PAYG, bypassing the Cost Gate).
Invariant enforced
What changed
agent/chat_completion_helpers.pytry_activate_fallback: for a governed PGF mission, callgate_static_fallback_payg()before any fallback provider/model swap. It fires even whengate_active()is False (policy gate inactive/legacy) or errored — the exact fail-open hole.OPERATOR_REQUIRED/DENIED/GATE_ERROR→ candidate marked unavailable, NOT activated, chain skips it, operator escalation surfaced. FREE/INCLUDED pass through unchanged.agent/pgf_routing_gate.py:gate_static_fallback_payg()— explicit billing-class resolution (*:free/*:batch=FREE; claude/anthropic + openai_codex=INCLUDED; explicit PAYG set; unknown=fail-closed) → FREE/INCLUDED no reservation; PAYG →evaluate_cost_gate(default €0 budget, cumulativeBudgetLedgeratomic reserve under file lock) with a stable decision_id keyed on provider/model so repeated fallbacks reuse/verify the prior reservation (no double-reserve).AUTHORIZED/OPERATOR_REQUIRED(persistedOperatorCostDecision) /GATE_ERROR(fail-closed).fallback-gate-*.jsonaudit (provider/model/billing/gate-decision/decision_id/remaining-budget/activation-result/timestamp).Tests
test_h1_static_fallback_gate.py— scenarios A–I: FREE passthrough, INCLUDED passthrough, PAYG €0 blocked + OperatorCostDecision, PAYG authorized-once, gate-error fail-closed, repeated-fallback no-double-reserve, cumulative-exhausted blocked, legacy-PAYG-cannot-escape, non-governed preserved. 9 new tests; 67 gate+fallback suites green; ruff clean.Review
Bounded Claude post-review: VERDICT YES — closes all identified fail-open paths, no double-reservation, FREE/INCLUDED intact, non-governed unchanged.
Not deployed. Not merged.