Skip to content

deploy(vera): Sonnet 5 on the Claude subscription + an alarm for the silent fallback - #45

Merged
mabry1985 merged 7 commits into
mainfrom
deploy/sonnet5-oauth-fallback-alert
Aug 23, 2026
Merged

deploy(vera): Sonnet 5 on the Claude subscription + an alarm for the silent fallback#45
mabry1985 merged 7 commits into
mainfrom
deploy/sonnet5-oauth-fallback-alert

Conversation

@mabry1985

Copy link
Copy Markdown
Member

What

Vera's primary lane moves from the gateway to a native Claude subscriptionmodel.provider: anthropic-oauth, model.name: claude-sonnet-5, adaptive thinking at effort high — with routing.fallback_models: ["protolabs/smart"] behind it. Pins go to latest upstream at the same time: core 0.137.1 → 0.144.0, github-plugin v0.3.0 → v0.4.0. pr-reviewer v0.35.0 and protoPatch 0.6.1 are already current.

Already applied to the running instance and verified: a probe turn returned model=claude-sonnet-5 in core telemetry with zero gateway traffic, which is the proof the native lane is actually carrying her.

Why the alarm

The interesting half is what happens when that subscription dies. protoAgent wires langchain's ModelFallbackMiddleware raw, and that middleware swallows the primary's exception with no log, no counter, and no event — verified by reading 0.144.0, not assumed. So the failure mode isn't a broken review. It's a verdict written by a different model, at a different quality bar, with nothing anywhere saying so.

Filed upstream as protoLabsAI/protoAgent#2956 — emit a model.fallback lifecycle event on the ADR 0074 seam (which already broadcasts on the bus, fires plugin hooks, and runs operator lifecycle_hooks reactions), plus a Prometheus counter and a warning log. Until that lands, two checks approximate it from outside:

check_model_fallback.py — infers the fallback from gateway metrics. Native OAuth bypasses the gateway entirely (ADR 0097), so a protoAgent-UA chat completion arriving at the gateway from Vera's container IP is, by construction, a fallback. clawpatch shares the key and the container but lands under user_agent="node" — that label is the whole discriminator. Alarms on growth, not depth (the metric is a lifetime counter; a fixed ceiling would latch red forever — the trap check_review_health.py already documents), with a 60m cooldown so a rate-limit burst is one Discord post rather than twelve.

check_oauth_health.py — watches the cause instead: signed-in, refreshable, and model.name/model.provider coherence (protoAgent#2623). It deliberately does not probe /api/config/test-model, even though that endpoint looks like the perfect liveness check: it returned 429 on four consecutive attempts while a real A2A turn on the same credential completed fine. An alert wired to it would have paged immediately and permanently, for a lane that was working.

Also

vera-watchdog.sh enters the repo. The alerting half of the guard existed only in one box's ~/.local/bin — the same "watchdog living nowhere legible" risk it was written to avoid. Cron on ava now runs fallback every 15m and oauth every 6h alongside the existing two.

Verified

  • 0.144.0 base pulled and its pyproject.toml read (version = "0.144.0"); the #2571 gateway-alias opt-out confirmed still present, which is what makes native-primary + gateway-fallback work at all
  • CI's three-way pin assertion (manifest / Dockerfile / README table) run locally — passes
  • Both new checks run live against the running container; every alarm branch of check_oauth_health.evaluate unit-exercised
  • Fallback detector baselined and re-run: correctly reports no fallback traffic while she's on the subscription

Known gap

The Discord post itself is not end-to-end verified: the Infisical login session on ava has expired, so the last two watchdog runs fell through to the no-secrets path and any alarm they raised would have printed to a log instead of posting. That's tracked separately — the fix is a machine identity rather than a human session.

🤖 Generated with Claude Code

https://claude.ai/code/session_01FnbQVnHNvTsvbnJ1pdUDdD

… silent fallback

Vera's primary lane moves from the gateway to a native Claude subscription:
`model.provider: anthropic-oauth` + `model.name: claude-sonnet-5` (adaptive thinking,
effort high), with `routing.fallback_models: ["protolabs/smart"]` behind it. Native OAuth
bypasses the gateway entirely (ADR 0097); the fallback alias still routes because #2571
lets a namespaced slot name opt out of the native provider, which is why `model.api_base`
and the gateway key stay set even though the primary never touches them.

The problem with that shape is what happens when the subscription dies. protoAgent wires
langchain's `ModelFallbackMiddleware` raw, and that middleware swallows the primary's
exception with no log, no counter and no event — verified against core 0.144.0. So the
failure mode is not a broken review, it is a verdict written by a different model with
nothing anywhere saying so. Filed upstream as protoAgent#2956 (emit a `model.fallback`
lifecycle event on the ADR 0074 seam, plus a counter and a warning); until that lands:

- `check_model_fallback.py` infers it from gateway metrics. Her primary never reaches the
  gateway, so a protoAgent-UA chat completion from her container IP IS a fallback.
  clawpatch shares the key and the container but lands under `user_agent="node"`, which
  is the whole discriminator. Alarms on growth (the counter is lifetime — a ceiling would
  latch red forever, the trap check_review_health.py already documents) with a 60m
  cooldown, so a rate-limit burst is one post rather than twelve.
- `check_oauth_health.py` watches the cause instead: signed-in, refreshable, and
  name/provider coherence (#2623). It deliberately does NOT probe `/api/config/test-model`
  — that endpoint returned 429 on four consecutive attempts while a real turn on the same
  credential completed and telemetry recorded `model=claude-sonnet-5`. An alert wired to
  it would have paged immediately, permanently, for a healthy lane.

`vera-watchdog.sh` gains both modes and enters the repo at last — the alerting half of the
guard existed only in one box's `~/.local/bin`, which is the same "watchdog living
nowhere legible" risk it was written to avoid.

Pins move to their latest upstream: core 0.137.1 → 0.144.0, github-plugin v0.3.0 → v0.4.0
(PR lifecycle events on the bus). pr-reviewer v0.35.0 and protoPatch 0.6.1 are already
current.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FnbQVnHNvTsvbnJ1pdUDdD

@protoreview protoreview Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

QA panel review — WARN

code-review-structural · head ffa8fd4e9556 · formal

Overall risk is low — this is an ops-tooling PR (new health-check scripts, Dockerfile, watchdog, bundle config) with no findings above minor severity. Fix first: dedupe the _api() docker-exec/curl helper in check_oauth_health.py, which is a byte-for-byte copy of the existing helper in check_review_health.py — pull it into a shared module. Second, minor: check_model_fallback.py and check_oauth_health.py both document their core logic as deliberately factored out to be unit-testable, but no tests ship in this PR for either. No panel disagreement — both findings were confirmed verbatim by the verifier against the PR head diff and the pre-existing sibling file. Gap: no protopatch/structural-tool findings appear in this round's input (none carry a source field), so the structural pass looks to have been skipped or produced nothing — worth confirming coverage there. Also no findings surfaced on the new Dockerfile, README.md, protoagent.bundle.yaml, or vera-watchdog.sh, so those files should be treated as under-reviewed rather than clean.

Findings

Severity Location Finding Verified
🟡 minor scripts/check_oauth_health.py:30 The docker-exec/curl _api() helper is copy-pasted verbatim from scripts/check_review_health.py instead of being shared, duplicating auth/timeout/error-handli… confirmed
🟡 minor scripts/check_model_fallback.py The PR documents fallback_requests()/evaluate() as deliberately factored out to be unit-testable, but ships no tests exercising either function. confirmed
findings JSON (machine-readable)
[
  {
    "file": "scripts/check_oauth_health.py",
    "line": 30,
    "severity": "minor",
    "category": "conventions",
    "claim": "The docker-exec/curl `_api()` helper is copy-pasted verbatim from scripts/check_review_health.py instead of being shared, duplicating auth/timeout/error-handling logic across two scripts.",
    "evidence": "def _api(container: str, path: str) -> dict:\n    cmd = f'curl -s -m 25 -H \"Authorization: Bearer $A2A_AUTH_TOKEN\" localhost:7870{path}'\n    out = subprocess.run(\n        [\"docker\", \"exec\", container, \"sh\", \"-c\", cmd], capture_output=True, text=True, timeout=60\n    )\n    if out.returncode != 0:\n        raise RuntimeError(f\"docker exec failed for {path}: {out.stderr.strip()[:200]}\")\n    return json.loads(out.stdout)",
    "verdict": "confirmed",
    "note": "Read both files at PR head via diff + github_read_file: check_oauth_health.py's _api() (lines ~28-34) is identical to check_review_health.py's pre-existing _api() except it drops the one-line docstring. Genuine duplication, no sharing."
  },
  {
    "file": "scripts/check_model_fallback.py",
    "line": 0,
    "severity": "minor",
    "category": "tests",
    "claim": "The PR documents fallback_requests()/evaluate() as deliberately factored out to be unit-testable, but ships no tests exercising either function.",
    "evidence": "\"\"\"Total protoAgent-UA gateway chat requests from ``agent_ip``, and the per-model split.\n\n    Pure over the scrape text so the attribution rule is unit-testable without a live\n    gateway \u2014 the rule is the load-bearing part, not the HTTP.\n    \"\"\"",
    "verdict": "confirmed",
    "note": "Verbatim docstring found in check_model_fallback.py's fallback_requests(); parallel 'testable' docstring exists on check_oauth_health.py's evaluate(). Full PR diff (all files new/modified: Dockerfile, README.md, protoagent.bundle.yaml, check_model_fallback.py, check_oauth_health.py, vera-watchdog.sh) contains no test files. No prior-round request covers this gap (prior_requests empty), so it stands as its own minor finding, not a refuted 'requested but missing' complaint."
  }
]

@protoreview protoreview Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

QA panel review — PASS

code-review-structural · head ffa8fd4e9556 · formal

Overall risk is moderate: two new ops scripts (check_model_fallback.py, check_oauth_health.py) ship with one real functional defect and two lower-severity issues, all confirmed against the actual diff/file content (both scripts are wholly new, so the diff is a complete read).

Fix first: in check_model_fallback.py, _save_state() is called after the try/except that maps scrape failures to exit 2, so a write failure (OSError from path.write_text()) escapes as an uncaught exception instead of the documented operational-error exit code — wrap the save call in the same error handling.

Two lower-priority items round out the list: check_oauth_health.py double-prefixes non-native-provider messages ("OK: OK: ...") because both the branch return and main() prepend "OK:"/"FAIL:"; and its _api() helper is a verbatim copy of the existing helper in check_review_health.py (missing only the docstring), a maintainability nit worth extracting to a shared module.

No disagreement within the panel — all three findings were confirmed on direct line-level inspection, no refutations or downgrades needed. This is a first-round review for this PR, so no prior dispositions apply. Coverage gap: the verifier could not fetch either new file via github_read_pr_file (404s) and relied on the full diff instead — equivalent evidence for new files, but noted here for transparency, not a blind spot in the finding set itself.

Findings

Severity Location Finding Verified
🟠 major check_model_fallback.py:208 _save_state() call after a successful scrape sits outside the try/except that maps failures to exit 2, so a write failure (OSError from path.write_text()) surf… confirmed
nit check_oauth_health.py Non-native branch bakes "OK: " into its own message, and main() also prepends "OK: "/"FAIL: ", doubling to "OK: OK:" for gateway-backed providers. confirmed
nit check_oauth_health.py _api() in check_oauth_health.py is copy-pasted verbatim from check_review_health.py's existing helper. confirmed
findings JSON (machine-readable)
[
  {
    "file": "check_model_fallback.py",
    "line": 208,
    "severity": "major",
    "category": "correctness",
    "claim": "_save_state() call after a successful scrape sits outside the try/except that maps failures to exit 2, so a write failure (OSError from path.write_text()) surfaces as an uncaught exception rather than the documented operational-error exit code.",
    "evidence": "file:check_model_fallback.py \u2014 try/except (lines ~156\u2013161) wraps only _container_ip/_scrape/fallback_requests; _save_state(args.state, new_state) call is at ~208\u2013209, outside that block; _save_state (def at ~142) calls path.write_text(...) with no guard, raising OSError uncaught on failure, propagating past main() as an unhandled exception instead of the documented exit code 2 (docstring at lines 47\u201349).",
    "verdict": "confirmed",
    "note": "Line-by-line reconstruction of the new file confirms: try/except (lines ~156\u2013161) wraps only _container_ip/_scrape/fallback_requests; _save_state(args.state, new_state) call is at ~208\u2013209, outside that block; _save_state (def at ~142) calls path.write_text(...) with no guard, which raises OSError uncaught on failure \u2014 propagates past main() as an unhandled exception (not exit 2). Docstring's exit-2 definition is at lines 47\u201349, matching claim. All cited line numbers check out against the diff."
  },
  {
    "file": "check_oauth_health.py",
    "line": 0,
    "severity": "nit",
    "category": "correctness",
    "claim": "Non-native branch bakes \"OK: \" into its own message, and main() also prepends \"OK: \"/\"FAIL: \", doubling to \"OK: OK:\" for gateway-backed providers.",
    "evidence": "return 0, [f\"OK: model.provider={provider!r} is not a native OAuth lane \u2014 no credential to check.\"] ... print((\"FAIL: \" if code else \"OK: \") + lines[0])",
    "verdict": "confirmed",
    "note": "Read exact lines: `return 0, [f\"OK: model.provider={provider!r} is not a native OAuth lane \u2014 no credential to check.\"]` and `print((\"FAIL: \" if code else \"OK: \") + lines[0])`. Traced: code=0 \u2192 prints \"OK: \" + \"OK: model.provider=...\" = \"OK: OK: model.provider=...\". Quote is verbatim, defect is real."
  },
  {
    "file": "check_oauth_health.py",
    "line": 0,
    "severity": "nit",
    "category": "conventions",
    "claim": "_api() in check_oauth_health.py is copy-pasted verbatim from check_review_health.py's existing helper.",
    "evidence": "cmd = f'curl -s -m 25 -H \"Authorization: Bearer $A2A_AUTH_TOKEN\" localhost:7870{path}'",
    "verdict": "confirmed",
    "note": "Read both: check_review_health.py (default branch, unmodified by this PR) has _api() with docstring + identical cmd = f'curl -s -m 25 -H \"Authorization: Bearer $A2A_AUTH_TOKEN\" localhost:7870{path}' / subprocess.run([...]) / raise RuntimeError(...) / return json.loads(out.stdout). check_oauth_health.py's new _api() (in this PR's diff) is byte-identical body, only missing the one-line docstring. Genuine duplication, quote matches exactly."
  }
]

3 finding(s) excluded from the verdict by in-diff confinement (file not among this PR's changed paths):

  • check_model_fallback.py (major) — _save_state() call after a successful scrape sits outside the try/except that maps failures to exit 2, so a write failure (OSError from path.write_text()) surfa
  • check_oauth_health.py (nit) — Non-native branch bakes "OK: " into its own message, and main() also prepends "OK: "/"FAIL: ", doubling to "OK: OK:" for gateway-backed providers.
  • check_oauth_health.py (nit) — _api() in check_oauth_health.py is copy-pasted verbatim from check_review_health.py's existing helper.

…t page

Both findings from Vera's own panel on this PR (WARN, 2 minor, both confirmed against
the head diff) — she reviewed it on the new claude-sonnet-5 lane, which is a decent
first look at the lane's output.

- `_api()` was a byte-for-byte copy of check_review_health.py's helper. Two copies of
  the auth/timeout/error handling for one endpoint is two places to fix when the port
  or the bearer moves, and the second copy is the one that gets missed. Now
  `scripts/vera_api.py`, flat and installed alongside — cron runs the copies in
  ~/.local/bin, and Python puts a script's own dir on sys.path[0], so a flat module
  imports cleanly there. It MUST be installed too; the header and README say so.
- Both scripts advertised their core functions as "deliberately pure so the rule is
  testable" and shipped no tests. The rule is the load-bearing part: an attribution
  bug in fallback_requests() is either a silent degrade nobody hears about or a pager
  that cries wolf, and neither shows up in a smoke run against a healthy container.
  13 stdlib-unittest cases now cover it — clawpatch's `user_agent="node"` (the whole
  discriminator), other containers' traffic, embeddings, counter summing, and every
  verdict branch of evaluate() including "expired but refreshable is not an alarm".
  CI runs them; stdlib because this CI is python3 + PyYAML and a watchdog suite
  earning a dependency install is the wrong trade.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FnbQVnHNvTsvbnJ1pdUDdD

@protoreview protoreview Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

QA panel review — PASS

code-review-structural · head fae1873135bf · formal

Both carried-over issues from round 1 remain unresolved at the current head SHA and were independently reconfirmed by the verifier against the live diff content.

Fix first: check_model_fallback.py — the _save_state() call sits outside the sole try/except that maps failures to exit 2, so a state-file write failure (OSError) will crash uncaught instead of returning the documented operational exit code 2. This is unchanged since round 1.

Also open: check_oauth_health.py doubles its status prefix ("OK: OK:") for non-native/gateway-backed providers, and the new OAuthHealth tests call evaluate() directly, never main()'s print path, so the bug is untested. Still a nit — cosmetic, not user-blocking, but easy to fix alongside the above.

No panel disagreement this round — both findings were independently re-derived from the diff body itself (file reads 404'd on this SHA, verifier fell back to the PR diff, which is authoritative for this added file). Verification did not surface round 1's third nit (copy-pasted _api() helper in check_oauth_health.py); with no confirmation either way this pass, it's dropped rather than carried as unverified. No structural (protopatch) findings were supplied this round — treat structural coverage of this diff as a gap, not a clean bill.

Prior requests

Prior finding Disposition Why
🔴 check_model_fallback.py:208 open Verifier re-read the diff body at head SHA fae1873 and confirmed the try/except still covers only _container_ip/_scrape/fallback_requests; the _save_state() ca…

Findings

Severity Location Finding Verified
🟠 major check_model_fallback.py:208 _save_state() is called after the only try/except in main() has already exited scope, so an OSError from path.write_text() during state persistence propagates … confirmed
nit check_oauth_health.py Non-native provider branch bakes "OK: " into its own message while main() also prepends "OK: "/"FAIL: " based on the exit code, doubling to "OK: OK:" for gatew… confirmed
findings JSON (machine-readable)
[
  {
    "file": "check_model_fallback.py",
    "line": 208,
    "severity": "major",
    "category": "correctness",
    "claim": "_save_state() is called after the only try/except in main() has already exited scope, so an OSError from path.write_text() during state persistence propagates uncaught instead of returning the documented exit code 2.",
    "evidence": "try:\n        agent_ip = _container_ip(args.container)\n        total, by_model = fallback_requests(_scrape(args.metrics_url), agent_ip)\n    except Exception as exc:  # noqa: BLE001 \u2014 every failure here is operational, exit 2\n        print(f\"UNREACHABLE: {exc}\")\n        return 2\n    ...\n    if not args.no_save:\n        _save_state(args.state, new_state)\n    return verdict",
    "verdict": "confirmed",
    "note": "Re-read PR #45's check_model_fallback.py body in the diff \u2014 try/except covers only _container_ip/_scrape/fallback_requests; _save_state() call is unconditionally after, no surrounding handler. Persists from round 1's identical request, unfixed."
  },
  {
    "file": "check_oauth_health.py",
    "line": 0,
    "severity": "nit",
    "category": "correctness",
    "claim": "Non-native provider branch bakes \"OK: \" into its own message while main() also prepends \"OK: \"/\"FAIL: \" based on the exit code, doubling to \"OK: OK:\" for gateway-backed providers; new tests exercise evaluate() directly and never main()'s print formatting, so the doubling is untested.",
    "evidence": "if provider not in NATIVE_PROVIDERS:\n        return 0, [f\"OK: model.provider={provider!r} is not a native OAuth lane \u2014 no credential to check.\"]\n...\nprint((\"FAIL: \" if code else \"OK: \") + lines[0])",
    "verdict": "confirmed",
    "note": "Verbatim lines confirmed in diff: return 0, [f\"OK: model.provider={provider!r}...\" and print((\"FAIL: \" if code else \"OK: \") + lines[0]). Traced logic: non-native \u2192 code=0 \u2192 \"OK: \" prepended to an already-\"OK: \"-prefixed string. tests/test_watchdog_checks.py's OAuthHealth tests call evaluate() directly, never main()'s print path \u2014 coverage gap claim also verified. Persists from round 1, unfixed."
  }
]

2 finding(s) excluded from the verdict by in-diff confinement (file not among this PR's changed paths):

  • check_model_fallback.py (major) — _save_state() is called after the only try/except in main() has already exited scope, so an OSError from path.write_text() during state persistence propagates u
  • check_oauth_health.py (nit) — Non-native provider branch bakes "OK: " into its own message while main() also prepends "OK: "/"FAIL: " based on the exit code, doubling to "OK: OK:" for gatewa

Unaccounted prior finding(s). An earlier round of this panel confirmed the following, and this round neither reports them, nor says they were fixed, nor refutes them:

  • check_model_fallback.py:208 (major) — _save_state() call after a successful scrape sits outside the try/except that maps failures to exit 2, so a write failure (OSError from path.write_text()) surfaces as an uncaught exception rather than the documented oper

A finding that disappears without a disposition is unproven, not resolved (issue #26). Any standing block stays up until the next round accounts for it — or an operator dismisses this review.

@protoreview protoreview Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Promoting the PASS verdict for head fae1873135bf: all checks terminal-green, zero unresolved review threads. (approve-on-green)

Open findings carried by this approval — non-blocking, but they did not go away:

  • major check_model_fallback.py:208 — _save_state() is called after the only try/except in main() has already exited scope, so an OSError from path.write_text() during state persistence propagates uncaught instead of returning the documen
  • nit check_oauth_health.py:0 — Non-native provider branch bakes "OK: " into its own message while main() also prepends "OK: "/"FAIL: " based on the exit code, doubling to "OK: OK:" for gateway-backed providers; new tests exercise e

Approving a WARN does not resolve its findings (issue #22).

…is no longer silent

Upstream shipped all three gaps this PR filed, within hours, in 0.145.0:

- **#2956** — `ObservableModelFallbackMiddleware` replaces the raw langchain
  middleware at the same call site: a WARNING naming the primary failure and the
  serving fallback, plus a `model.fallback` bus event (ADR 0039) carrying the
  primary exception class, fallback model and index. GraphBubbleUp now propagates
  untouched instead of triggering fallback retries, and when every fallback fails
  the PRIMARY exception is re-raised rather than the last one. Verified present in
  the 0.145.0 image before pinning.
- **#2957** — the OAuth "Test connection" probe now sends the Claude Code identity
  prefix, so it stops reporting a working subscription as rate-limited.
- **#2958** — `secrets_manager.host` tolerates the CLI's `/api` suffix, which
  removes the sharp edge #46 documents.

0.145.0 also carries #2950, which is directly relevant to the exhaustion burst Vera
hit at 09:18Z today: sweep harvests now space themselves with jittered gaps because
back-to-back model calls were "manufacturing rate-limit bursts" on a shared OAuth
account. Three protoAgent PRs merged with no verdict in that window.

Once this rolls, `scripts/check_model_fallback.py` should be RETIRED in favour of
subscribing to `model.fallback` — its own docstring says so, and an event is ground
truth where the gateway-metrics rule is an inference. Leaving it in place for now:
it costs nothing, and the event needs to be seen working first.

github-plugin v0.5.0: the ADR 0095 projects registry is ADDED to an explicit
`github.repos` list rather than hidden by it.

pr-reviewer stays v0.35.0 — the fix for pr-reviewer-plugin#84 (replay of a merged PR
silently returning PASS) is merged on main but not yet tagged.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FnbQVnHNvTsvbnJ1pdUDdD

@protoreview protoreview Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

QA panel review — FAIL

code-review-structural · head af2479f58049 · formal

Overall risk: moderate. The one real runtime bug — _save_state() in scripts/check_model_fallback.py sitting outside the only try/except in main(), so a disk-full/permission OSError on state persistence escapes uncaught instead of returning the documented exit code 2 — has now survived three rounds unfixed and is the fix-first item. It's independently confirmed by every review angle plus the structural (protopatch) pass, so treat it as load-bearing before merge.

Verification also surfaced (and confirmed) a real doc/logic contradiction: the README claims the silent-fallback bug is "FIXED in 0.145.0," which is exactly the core version this PR pins in the Dockerfile/bundle, yet the new watchdog script's own docstring and deletion criterion are written as if the bug is still open — this PR's own version bump already satisfies the condition it says should trigger the script's removal. No panel disagreement on substance here, just a severity call — kept as major since it's an internal self-contradiction that will confuse the next maintainer, not mere prose.

The check_oauth_health.py "OK: OK:" doubled-prefix nit is also reconfirmed with root cause and the observation that tests only exercise evaluate() directly, never main()'s print path — kept as nit, unchanged from prior rounds. No findings were refuted this pass. No coverage gaps: the structural pass ran (not skipped), and all touched files (the fallback script, README, oauth health script) were re-read in full during verification.

Prior requests

Prior finding Disposition Why
🔴 check_model_fallback.py:208 open Verifier re-read main() in the current diff and confirmed _save_state() still sits after the only try/except, unguarded — the same defect flagged in rounds 1 a…

Findings

Severity Location Finding Verified
🟠 major scripts/check_model_fallback.py:208 _save_state() is called after the only try/except in main() has already exited scope, so an OSError from path.write_text() during state persistence propagates … confirmed
🟠 major README.md:100 README claims the silent-fallback middleware bug was 'FIXED in 0.145.0', the exact core version this PR pins in the Dockerfile, yet the new scripts/check_model… confirmed
🟠 major check_model_fallback.py:208 _save_state() is called after the only try/except in main() has already exited scope, so an OSError from path.write_text() during state persistence propagates … confirmed
nit scripts/check_oauth_health.py Non-native provider branch bakes "OK: " into its own message while main() also prepends "OK: "/"FAIL: " based on the exit code, doubling to "OK: OK:" for gatew… confirmed
findings JSON (machine-readable)
[
  {
    "file": "scripts/check_model_fallback.py",
    "line": 208,
    "severity": "major",
    "category": "correctness",
    "claim": "_save_state() is called after the only try/except in main() has already exited scope, so an OSError from path.write_text() during state persistence propagates uncaught instead of returning the documented exit code 2 \u2014 flagged independently by correctness, removed-behavior, cross-file, and conventions review, and confirmed structurally by protopatch.",
    "evidence": "try:\n        agent_ip = _container_ip(args.container)\n        total, by_model = fallback_requests(_scrape(args.metrics_url), agent_ip)\n    except Exception as exc:  # noqa: BLE001 \u2014 every failure here is operational, exit 2\n        print(f\"UNREACHABLE: {exc}\")\n        return 2\n    ...\n    if not args.no_save:\n        _save_state(args.state, new_state)\n    return verdict",
    "source": "protopatch",
    "verdict": "confirmed",
    "note": "Re-read full main() from PR diff (new file, additions visible in full) \u2014 the try/except wraps only agent_ip/scrape/fallback_requests; _save_state() below it is unguarded, so OSError from write_text() propagates uncaught."
  },
  {
    "file": "README.md",
    "line": 100,
    "severity": "major",
    "category": "cross-file",
    "claim": "README claims the silent-fallback middleware bug was 'FIXED in 0.145.0', the exact core version this PR pins in the Dockerfile, yet the new scripts/check_model_fallback.py (and the rest of this same README section) is written entirely on the premise the bug is still unfixed, with its own deletion criterion already met by this PR's version bump.",
    "evidence": "verified against core 0.144.0 \u2014 FIXED in 0.145.0 (#2956), filed as protoAgent#2956. So a dead subscription doesn't\nbreak the review; it quietly changes which model writes the verdict. That is what\n`scripts/check_model_fallback.py` exists to catch, by inference from gateway metrics:",
    "verdict": "confirmed",
    "note": "Diff shows README's 'FIXED in 0.145.0' line, Dockerfile/bundle pin bumped to exactly 0.145.0, and the new script's docstring still says 'If that issue lands... DELETE this script' \u2014 an internal contradiction within this same PR."
  },
  {
    "file": "scripts/check_oauth_health.py",
    "line": 0,
    "severity": "nit",
    "category": "correctness",
    "claim": "Non-native provider branch bakes \"OK: \" into its own message while main() also prepends \"OK: \"/\"FAIL: \" based on the exit code, doubling to \"OK: OK:\" for gateway-backed providers; tests exercise evaluate() directly and never main()'s print formatting, so the doubling is untested \u2014 agreed by all four LLM review angles.",
    "evidence": "if provider not in NATIVE_PROVIDERS:\n        return 0, [f\"OK: model.provider={provider!r} is not a native OAuth lane \u2014 no credential to check.\"]\n...\nprint((\"FAIL: \" if code else \"OK: \") + lines[0])",
    "verdict": "confirmed",
    "note": "Full file read from diff: evaluate() returns (0, ['OK: ...']) for non-native provider; main() prepends another 'OK: ' since code is falsy -> 'OK: OK: ...'. tests/test_watchdog_checks.py calls evaluate() directly, never main(), so doubling is indeed untested."
  },
  {
    "file": "check_model_fallback.py",
    "line": 208,
    "severity": "major",
    "claim": "_save_state() is called after the only try/except in main() has already exited scope, so an OSError from path.write_text() during state persistence propagates uncaught instead of returning the documented exit code 2.",
    "verdict": "confirmed",
    "carried": true,
    "note": "Re-read PR #45's check_model_fallback.py body in the diff \u2014 try/except covers only _container_ip/_scrape/fallback_requests; _save_state() call is unconditionally after, no surrounding handler. Persists from round 1's identical request, unfixed. \u2014 carried from a prior round \u2014 a confirmed blocker/major this round neither fixed nor refuted (protoAgent#2283); it keeps gating until positively cleared"
  }
]

Unaccounted prior finding(s). An earlier round of this panel confirmed the following, and this round neither reports them, nor says they were fixed, nor refutes them:

  • check_model_fallback.py:208 (major) — _save_state() is called after the only try/except in main() has already exited scope, so an OSError from path.write_text() during state persistence propagates uncaught instead of returning the documented exit code 2.

A finding that disappears without a disposition is unproven, not resolved (issue #26). Any standing block stays up until the next round accounts for it — or an operator dismisses this review.

…g the pin

Vera's third-round FAIL on this PR, and she was right on all three — the first has
been open since round 1 and I hadn't been reading her earlier passes.

- **`_save_state()` sat outside the only try/except in `main()`** (major, confirmed
  by four review angles plus the protopatch structural pass). An OSError from
  `write_text()` — full disk, bad permissions — escaped as a traceback instead of the
  documented exit 2. A watchdog that dies with a stack trace instead of its own error
  contract is precisely the failure this whole PR is about. Guarded now, with the
  asymmetry made explicit: a save failure downgrades to exit 2 only when there was
  nothing else to report; a real FALLBACK verdict survives it. Relabelling a silent
  degrade as an "unreachable" outage would send the operator looking in the wrong
  place, and the cost of not saving is a duplicate alert next run — strictly better
  than a missed one.
- **The README claimed #2956 was "FIXED in 0.145.0" while the script's docstring
  still told the reader to delete it once that fix landed** — this PR pins 0.145.0,
  so it already satisfied its own retirement condition. Reconciled around the
  distinction that was missing from both: *pinning a version is not running it.*
  Vera rolls on watchtower after merge, so between pin and roll she is on 0.137.1
  with no event at all. The script retires when the RUNNING instance reports 0.145.0
  and the event has been seen firing — not before, or the window is covered by
  neither.
- **nit: "OK: OK:"** — the non-native branch baked in a prefix `main()` also
  prepends. Fixed, and pinned by a test, since her note that the suite only ever
  calls `evaluate()` and never `main()`'s print path is the reason it survived.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FnbQVnHNvTsvbnJ1pdUDdD

@protoreview protoreview Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

QA panel review — WARN

code-review-structural · head 01f702684f51 · formal

This is a verification-only round: no finder surfaced any new issues, but the two standing majors from round 3 (the _save_state() write in scripts/check_model_fallback.py sitting outside the try/except that maps failures to exit 2, and the README's "FIXED in 0.145.0" claim contradicting the rest of that same section) could not be independently reverified this pass — every attempt to read the live PR/repo (protoLabsAI/qaEngineer#45) returned 404. Per protocol, a failed read is not evidence a fix landed, so despite the round's brief asserting both are now fixed, I'm carrying them forward as open/uncertain rather than closing them on an unconfirmed claim — this is exactly the "silently vanishes into a clean pass" failure mode the process guards against, and the next round needs working repo access to actually settle it.

Fix first (if not already done): the _save_state() OSError-escape in check_model_fallback.py:208 — it's been raised independently across three rounds by correctness, removed-behavior, cross-file, and convention angles, and is the more load-bearing of the two.

No real panel disagreement this round — the finders converged on empty, and the only tension is my choice to distrust that silence given the tooling outage rather than accept it as a clean bill of health.

Coverage gaps: repo/PR access was down for the whole verification step (404 on pinned-SHA, no-ref, PR-file, and PR-diff reads alike), so nothing this round — including the tool-sourced structural (protopatch) pass mentioned as dropped upstream — was actually re-run against current code. Treat this round's "clean" result as unverified, not confirmed, until access is restored. The prior nit about check_oauth_health.py's doubled "OK: OK:" prefix is left out of this pass (nits aren't subject to the mandatory disposition rule) but its status is equally unconfirmed.

Prior requests

Prior finding Disposition Why
🔴 scripts/check_model_fallback.py:208 open Verifier could not confirm the claimed fix — every attempt to read the live PR/repo (#45) returned 404 this round, so the upstream claim …
🔴 README.md:100 open Same repo-access failures (404 on every read path) prevented checking whether the '0.145.0' contradiction with the rest of the README section was resolved; unv…

Findings

Severity Location Finding Verified
🟠 major scripts/check_model_fallback.py:208 _save_state() is called after the only try/except in main() has already exited scope, so an OSError from the state-persistence write propagates uncaught instea… ⚠️ uncertain
🟠 major README.md:100 README states the silent-fallback middleware bug was 'FIXED in 0.145.0' — the exact core version this PR pins in the Dockerfile — yet the new scripts/check_mod… ⚠️ uncertain
🟠 major check_model_fallback.py:208 _save_state() is called after the only try/except in main() has already exited scope, so an OSError from path.write_text() during state persistence propagates … confirmed
findings JSON (machine-readable)
[
  {
    "file": "scripts/check_model_fallback.py",
    "line": 208,
    "severity": "major",
    "category": "cross-file",
    "claim": "_save_state() is called after the only try/except in main() has already exited scope, so an OSError from the state-persistence write propagates uncaught instead of returning the documented exit code 2; the round's claim that this was fixed could not be independently verified because repo access failed.",
    "evidence": "scripts/check_model_fallback.py:208 \u2014 per rounds 1\u20133 review, the _save_state() call site sits outside main()'s try/except block that maps failures to exit code 2; no current diff was retrievable this round to confirm a guard was added (all repo/PR reads returned 404).",
    "verdict": "uncertain",
    "note": "Verifier's attempts to read the live PR/repo (pinned SHA, no-ref, PR-scoped file, PR diff) all returned 404 this round; per protocol a failed read is not evidence of a fix, so this finding is carried forward unresolved rather than dropped."
  },
  {
    "file": "README.md",
    "line": 100,
    "severity": "major",
    "category": "cross-file",
    "claim": "README states the silent-fallback middleware bug was 'FIXED in 0.145.0' \u2014 the exact core version this PR pins in the Dockerfile \u2014 yet the new scripts/check_model_fallback.py and the rest of that same README section are written on the premise the bug is still unfixed; this contradiction's claimed resolution could not be independently verified.",
    "evidence": "README.md:100 \u2014 per round 3 review, the section claims the bug was 'FIXED in 0.145.0' while the surrounding text and the newly added check_model_fallback.py script both assume the bug is still present; no current diff was retrievable this round to confirm the contradiction was reconciled (all repo/PR reads returned 404).",
    "verdict": "uncertain",
    "note": "Same 404 access failures prevented verification this round; failed reads are not evidence a fix landed, so this stays open rather than being closed on the unconfirmed upstream claim."
  },
  {
    "file": "check_model_fallback.py",
    "line": 208,
    "severity": "major",
    "claim": "_save_state() is called after the only try/except in main() has already exited scope, so an OSError from path.write_text() during state persistence propagates uncaught instead of returning the documented exit code 2.",
    "verdict": "confirmed",
    "carried": true,
    "note": "Re-read PR #45's check_model_fallback.py body in the diff \u2014 try/except covers only _container_ip/_scrape/fallback_requests; _save_state() call is unconditionally after, no surrounding handler. Persists from round 1's identical request, unfixed. \u2014 carried from a prior round \u2014 a confirmed blocker/major this round neither fixed nor refuted (protoAgent#2283); it keeps gating until positively cleared \u2014 carried from a prior round \u2014 a confirmed blocker/major this round neither fixed nor refuted (protoAgent#2283); it keeps gating until positively cleared"
  }
]

Unaccounted prior finding(s). An earlier round of this panel confirmed the following, and this round neither reports them, nor says they were fixed, nor refutes them:

  • scripts/check_model_fallback.py:208 (major) — _save_state() is called after the only try/except in main() has already exited scope, so an OSError from path.write_text() during state persistence propagates uncaught instead of returning the documented exit code 2 — fl
  • README.md:100 (major) — README claims the silent-fallback middleware bug was 'FIXED in 0.145.0', the exact core version this PR pins in the Dockerfile, yet the new scripts/check_model_fallback.py (and the rest of this same README section) is wr
  • check_model_fallback.py:208 (major) — _save_state() is called after the only try/except in main() has already exited scope, so an OSError from path.write_text() during state persistence propagates uncaught instead of returning the documented exit code 2.

A finding that disappears without a disposition is unproven, not resolved (issue #26). Any standing block stays up until the next round accounts for it — or an operator dismisses this review.

@protoreview protoreview Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Promoting the WARN verdict for head 01f702684f51: all checks terminal-green, zero unresolved review threads. (approve-on-green)

Open findings carried by this approval — non-blocking, but they did not go away:

  • major scripts/check_model_fallback.py:208 — _save_state() is called after the only try/except in main() has already exited scope, so an OSError from the state-persistence write propagates uncaught instead of returning the documented exit code 2
  • major README.md:100 — README states the silent-fallback middleware bug was 'FIXED in 0.145.0' — the exact core version this PR pins in the Dockerfile — yet the new scripts/check_model_fallback.py and the rest of that same
  • major check_model_fallback.py:208 — _save_state() is called after the only try/except in main() has already exited scope, so an OSError from path.write_text() during state persistence propagates uncaught instead of returning the documen

Approving a WARN does not resolve its findings (issue #22).

…o four

Vera's nit on #45, and correct — I added the fallback and oauth modes to the case
statement and left the header describing the original pair. The install lines and
usage string were already updated; only the prose lagged.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FnbQVnHNvTsvbnJ1pdUDdD

@protoreview protoreview Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

QA panel review — PASS

code-review-structural · head 8331b492ab86 · formal

Final pass: all three issues this panel had been carrying across rounds 1–4 are now resolved. The _save_state() write in scripts/check_model_fallback.py is wrapped in its own try/except OSError, correctly mapping a persistence failure to exit 2 (or preserving exit 1 if a real fallback was already detected) instead of propagating uncaught. The README's "FIXED in 0.145.0" line no longer contradicts the still-present detection script — new prose explains the pin-vs-rollout gap that leaves the script necessary during the deploy window. The check_oauth_health.py "OK: OK:" doubling and the duplicated _api() helper are both cleaned up (shared operator_api_get import, no baked-in prefix). CI now runs the new test suite. No disagreement within the panel this round — the verifier independently corroborated each fix against the diff. No coverage gaps: the previously-flagged files were re-checked directly, and no new findings surfaced.

Prior requests

Prior finding Disposition Why
scripts/check_model_fallback.py:208 fixed _save_state() is now called inside its own try/except OSError block that returns exit 2 when there's no fallback verdict to preserve (or keeps exit 1 if a fall…
README.md:100 fixed README now explains the pin-vs-rollout gap ('pinning a version is not running it... between the pin landing and the roll completing she is on the old core with…

Findings

Severity Location Finding Verified
🟠 major scripts/check_model_fallback.py:208 _save_state() is called after the only try/except in main() has already exited scope, so an OSError from the state-persistence write propagates uncaught instea… confirmed
🟠 major README.md:100 README states the silent-fallback middleware bug was 'FIXED in 0.145.0' — the exact core version this PR pins in the Dockerfile — yet the new scripts/check_mod… confirmed
🟠 major check_model_fallback.py:208 _save_state() is called after the only try/except in main() has already exited scope, so an OSError from path.write_text() during state persistence propagates … confirmed
findings JSON (machine-readable)
[
  {
    "file": "scripts/check_model_fallback.py",
    "line": 208,
    "severity": "major",
    "claim": "_save_state() is called after the only try/except in main() has already exited scope, so an OSError from the state-persistence write propagates uncaught instead of returning the documented exit code 2; the round's claim that this was fixed could not be independently verified because repo access failed.",
    "verdict": "confirmed",
    "carried": true,
    "note": "Verifier's attempts to read the live PR/repo (pinned SHA, no-ref, PR-scoped file, PR diff) all returned 404 this round; per protocol a failed read is not evidence of a fix, so this finding is carried forward unresolved rather than dropped. \u2014 carried from a prior round \u2014 a confirmed blocker/major this round neither fixed nor refuted (protoAgent#2283); it keeps gating until positively cleared"
  },
  {
    "file": "README.md",
    "line": 100,
    "severity": "major",
    "claim": "README states the silent-fallback middleware bug was 'FIXED in 0.145.0' \u2014 the exact core version this PR pins in the Dockerfile \u2014 yet the new scripts/check_model_fallback.py and the rest of that same README section are written on the premise the bug is still unfixed; this contradiction's claimed resolution could not be independently verified.",
    "verdict": "confirmed",
    "carried": true,
    "note": "Same 404 access failures prevented verification this round; failed reads are not evidence a fix landed, so this stays open rather than being closed on the unconfirmed upstream claim. \u2014 carried from a prior round \u2014 a confirmed blocker/major this round neither fixed nor refuted (protoAgent#2283); it keeps gating until positively cleared"
  },
  {
    "file": "check_model_fallback.py",
    "line": 208,
    "severity": "major",
    "claim": "_save_state() is called after the only try/except in main() has already exited scope, so an OSError from path.write_text() during state persistence propagates uncaught instead of returning the documented exit code 2.",
    "verdict": "confirmed",
    "carried": true,
    "note": "Re-read PR #45's check_model_fallback.py body in the diff \u2014 try/except covers only _container_ip/_scrape/fallback_requests; _save_state() call is unconditionally after, no surrounding handler. Persists from round 1's identical request, unfixed. \u2014 carried from a prior round \u2014 a confirmed blocker/major this round neither fixed nor refuted (protoAgent#2283); it keeps gating until positively cleared \u2014 carried from a prior round \u2014 a confirmed blocker/major this round neither fixed nor refuted (protoAgent#2283); it keeps gating until positively cleared \u2014 carried from a prior round \u2014 a confirmed blocker/major this round neither fixed nor refuted (protoAgent#2283); it keeps gating until positively cleared"
  }
]

Unaccounted prior finding(s). An earlier round of this panel confirmed the following, and this round neither reports them, nor says they were fixed, nor refutes them:

  • scripts/check_model_fallback.py:208 (major) — _save_state() is called after the only try/except in main() has already exited scope, so an OSError from the state-persistence write propagates uncaught instead of returning the documented exit code 2; the round's claim
  • README.md:100 (major) — README states the silent-fallback middleware bug was 'FIXED in 0.145.0' — the exact core version this PR pins in the Dockerfile — yet the new scripts/check_model_fallback.py and the rest of that same README section are w
  • check_model_fallback.py:208 (major) — _save_state() is called after the only try/except in main() has already exited scope, so an OSError from path.write_text() during state persistence propagates uncaught instead of returning the documented exit code 2.

A finding that disappears without a disposition is unproven, not resolved (issue #26). Any standing block stays up until the next round accounts for it — or an operator dismisses this review.

@protoreview protoreview Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Promoting the PASS verdict for head 8331b492ab86: all checks terminal-green, zero unresolved review threads. (approve-on-green)

Open findings carried by this approval — non-blocking, but they did not go away:

  • major scripts/check_model_fallback.py:208 — _save_state() is called after the only try/except in main() has already exited scope, so an OSError from the state-persistence write propagates uncaught instead of returning the documented exit code 2
  • major README.md:100 — README states the silent-fallback middleware bug was 'FIXED in 0.145.0' — the exact core version this PR pins in the Dockerfile — yet the new scripts/check_model_fallback.py and the rest of that same
  • major check_model_fallback.py:208 — _save_state() is called after the only try/except in main() has already exited scope, so an OSError from path.write_text() during state persistence propagates uncaught instead of returning the documen

Approving a WARN does not resolve its findings (issue #22).

…t inverted

Vera's Claude subscription exhausted its 7-DAY quota after ~2 days of panel work
(`7d-status: rejected`, `7d-utilization: 1.0`, `retry-after: 356343` = 4.1 days), so
she is back on the gateway: `protolabs/smart` primary, `protolabs/cloud` fallback.

That switch silently inverted the fallback detector. Its rule was "a protoAgent-UA
gateway request from her container IS a fallback" — sound only while the primary was
a native-OAuth lane that bypassed the gateway entirely. With a gateway primary, every
ordinary review matches it, and the 15-minute cron would have paged #alerts
continuously starting minutes after the switch.

The rule now asks the LIVE CONFIG which models are the fallback and counts only
traffic requesting those. Correct for both lane shapes, and it keeps working across
the next switch without anyone remembering to edit it — which is the actual fix,
since nobody was going to remember. It refuses outright (exit 2) if fallback_models
contains the primary, because a no-op lane cannot distinguish anything and crying
wolf is worse than staying quiet. Four tests pin the discriminator.

Also: `vera-watchdog.sh`'s alerts.env fallback moves here from the Infisical branch,
where I had misfiled it. It is watchdog robustness, not secrets work, and keeping it
one branch up meant every install from this branch silently regressed the alert path
— the branch trap the script's own header warns about, which I have now walked into
three times. Verified end-to-end with the env var stripped: still posts to Discord.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FnbQVnHNvTsvbnJ1pdUDdD

@protoreview protoreview Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

QA panel review — WARN

code-review-structural · head 299bc0c16817 · formal

Two standing majors from earlier rounds remain open and unverified (no repo access this pass): the uncaught OSError around _save_state() in scripts/check_model_fallback.py:208 and the README.md:100 FIXED-in-0.145.0 contradiction. Verification changed this round's list: the CI unittest discover tests -v finding was refuted (CPython's discovery parser accepts a positional start directory), and the vera-watchdog.sh:95 fallback comment was confirmed as minor stale-doc drift. Fix-first: settle the two open majors before merge; the stale comment is minor. No disagreement on the surviving items. Coverage gap: scripts/check_oauth_health.py's earlier nits were not re-examined in this verify pass.

Prior requests

Prior finding Disposition Why
🔴 scripts/check_model_fallback.py:208 open Not re-adjudicated this pass and no repo access to confirm the claimed fix; carried forward as open.
🔴 README.md:100 open Not re-adjudicated this pass and no repo access to confirm the claimed resolution; carried forward as open.

Findings

Severity Location Finding Verified
🟠 major scripts/check_model_fallback.py:208 _save_state() is called after the only try/except in main() has already exited scope, so an OSError from path.write_text() during state persistence propagates … ⚠️ uncertain
🟠 major README.md:100 README states the silent-fallback middleware bug was 'FIXED in 0.145.0' — the exact core version this PR pins in the Dockerfile — yet the new scripts/check_mod… ⚠️ uncertain
🟠 major check_model_fallback.py:208 _save_state() is called after the only try/except in main() has already exited scope, so an OSError from path.write_text() during state persistence propagates … confirmed
🟡 minor scripts/vera-watchdog.sh:95 The fallback case comment still documents the abandoned native-OAuth rule — that any protoAgent-UA gateway traffic from Vera is by construction a fallback an… confirmed
findings JSON (machine-readable)
[
  {
    "file": "scripts/check_model_fallback.py",
    "line": 208,
    "severity": "major",
    "category": "correctness",
    "claim": "_save_state() is called after the only try/except in main() has already exited scope, so an OSError from path.write_text() during state persistence propagates uncaught instead of returning the documented exit code 2.",
    "evidence": "scripts/check_model_fallback.py:208 \u2014 carried forward from prior rounds; no verbatim diff hunk was available this pass to re-ground the quote.",
    "verdict": "uncertain",
    "note": "Standing major from rounds 1\u20135; the claimed fix could not be independently verified this pass (no repo access), so it remains open."
  },
  {
    "file": "README.md",
    "line": 100,
    "severity": "major",
    "category": "cross-file",
    "claim": "README states the silent-fallback middleware bug was 'FIXED in 0.145.0' \u2014 the exact core version this PR pins in the Dockerfile \u2014 yet the new scripts/check_model_fallback.py and the rest of that same README section are written on the premise the bug is still unfixed.",
    "evidence": "README.md:100 \u2014 prior-round claim quotes the README as saying 'FIXED in 0.145.0'; no verbatim README line was available this pass to re-ground the quote.",
    "verdict": "uncertain",
    "note": "Standing major from rounds 3\u20135; the claimed resolution could not be independently verified this pass (no repo access), so it remains open."
  },
  {
    "file": "scripts/vera-watchdog.sh",
    "line": 95,
    "severity": "minor",
    "category": "cross-file",
    "claim": "The `fallback` case comment still documents the abandoned native-OAuth rule \u2014 that any protoAgent-UA gateway traffic from Vera is by construction a fallback and that #2956 emits no event \u2014 which contradicts check_model_fallback.py's new live-config fallback-list rule and the README's statement that #2956 is FIXED in core 0.145.0; flagged by both cross-file and conventions review.",
    "evidence": "# Did Vera silently answer from her FALLBACK model? Her primary is a native-OAuth\n    # Claude subscription, which bypasses the gateway entirely (ADR 0097) \u2014 so\n    # protoAgent-UA traffic arriving AT the gateway from her container is, by\n    # construction, a fallback. protoAgent emits nothing when this happens (langchain's\n    # ModelFallbackMiddleware swallows the primary's exception without so much as a log\n    # line; filed as protoAgent#2956), so an inference from gateway metrics is the only\n    # signal there is until that issue lands a `model.fallback` event.",
    "verdict": "confirmed",
    "note": "The quoted comment exists verbatim in the new scripts/vera-watchdog.sh (PR diff) and is stale: README says the primary moved to gateway `protolabs/smart` on 2026-08-23 and check_model_fallback.py says the any-gateway-traffic shortcut 'silently inverted'; the comment also still frames #2956 as unfixed while README says it is FIXED in 0.145.0. Minor doc inconsistency."
  },
  {
    "file": "check_model_fallback.py",
    "line": 208,
    "severity": "major",
    "claim": "_save_state() is called after the only try/except in main() has already exited scope, so an OSError from path.write_text() during state persistence propagates uncaught instead of returning the documented exit code 2.",
    "verdict": "confirmed",
    "carried": true,
    "note": "Re-read PR #45's check_model_fallback.py body in the diff \u2014 try/except covers only _container_ip/_scrape/fallback_requests; _save_state() call is unconditionally after, no surrounding handler. Persists from round 1's identical request, unfixed. \u2014 carried from a prior round \u2014 a confirmed blocker/major this round neither fixed nor refuted (protoAgent#2283); it keeps gating until positively cleared \u2014 carried from a prior round \u2014 a confirmed blocker/major this round neither fixed nor refuted (protoAgent#2283); it keeps gating until positively cleared \u2014 carried from a prior round \u2014 a confirmed blocker/major this round neither fixed nor refuted (protoAgent#2283); it keeps gating until positively cleared \u2014 carried from a prior round \u2014 a confirmed blocker/major this round neither fixed nor refuted (protoAgent#2283); it keeps gating until positively cleared"
  }
]

Unaccounted prior finding(s). An earlier round of this panel confirmed the following, and this round neither reports them, nor says they were fixed, nor refutes them:

  • scripts/check_model_fallback.py:208 (major) — _save_state() is called after the only try/except in main() has already exited scope, so an OSError from the state-persistence write propagates uncaught instead of returning the documented exit code 2; the round's claim
  • README.md:100 (major) — README states the silent-fallback middleware bug was 'FIXED in 0.145.0' — the exact core version this PR pins in the Dockerfile — yet the new scripts/check_model_fallback.py and the rest of that same README section are w
  • check_model_fallback.py:208 (major) — _save_state() is called after the only try/except in main() has already exited scope, so an OSError from path.write_text() during state persistence propagates uncaught instead of returning the documented exit code 2.

A finding that disappears without a disposition is unproven, not resolved (issue #26). Any standing block stays up until the next round accounts for it — or an operator dismisses this review.

…ed rule

Vera's WARN on #45 (minor, confirmed). I rewrote check_model_fallback.py's rule to
read routing.fallback_models from the live config, and left the wrapper's case
comment asserting the old native-OAuth premise — that any protoAgent-UA gateway
traffic is by construction a fallback. That premise is exactly what the switch back
to a gateway primary invalidated, so the comment now documents the trap rather than
the rule.

Her two carried-forward majors on this head are stale, not open: both were fixed in
the previous commit and she marked them "⚠️ uncertain — no repo access this pass",
which is the panel correctly refusing to re-confirm what it could not read.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FnbQVnHNvTsvbnJ1pdUDdD
@protoreview

protoreview Bot commented Aug 23, 2026

Copy link
Copy Markdown

🛑 Review cap reached — this PR has had 6 automated review round(s). Further pushes will not trigger new reviews.

The cap resets when:

  • The PR is marked ready-for-review
  • An operator runs @vera review (manual summon)
  • 2 hour(s) elapse since the cap was hit

@mabry1985
mabry1985 merged commit 72d93be into main Aug 23, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant