fix(#4771): local approvals no longer 409 as gateway run unavailable - #4950
2 commits merged into
Conversation
#4771 added an explicit relay-failure 409 (code: gateway_run_unavailable) to /api/approval/respond so a gateway approval whose run is gone keeps its card actionable instead of silently failing. But the 409 was emitted regardless of backend. On the DEFAULT local in-process backend, every guarded command parks an entry in tools.approval._gateway_queues (via _await_gateway_decision), and the local streaming notify callback mirrors it into _pending tagged with _GATEWAY_MIRROR_FLAG. There is no gateway run and no _STREAM_RUN_IDS entry by design. _gateway_pending_approval_without_run_id() therefore returned True for that purely local approval, so the handler 409'd ("active run unavailable") and refused to resolve an approval that resolves perfectly well locally. Local-backend users hit this on every approval click (reported by b3nw on Discord; independently filed as #4948 by claw-io, which also notes the local-runtime trigger). Fix: gate the 409 on the WebUI actually running the gateway chat backend (webui_gateway_chat_enabled). Local approvals fall through to the local resolution path; the legitimate #4771 gateway behaviour is unchanged. Tests: - New tests/test_issue4771_local_approval_regression.py drives the real _handle_approval_respond handler and asserts: local backend -> 200 + agent thread unblocked; gateway backend -> 409 preserved. These fail before the fix (409) and pass after. - The two existing #4771 tests that asserted the 409 are updated to pin HERMES_WEBUI_CHAT_BACKEND=gateway at respond time, since the 409 is now gateway-deployment behaviour. Reported-by: b3nw Reported-by: claw-io (#4948)
|
| Filename | Overview |
|---|---|
| api/routes.py | Adds the backend-mode guard before returning the gateway unavailable-run response. |
| tests/test_issue4771_local_approval_regression.py | Adds tests for local approval resolution and preserved gateway unavailable-run behavior. |
| tests/test_approval_unblock.py | Pins gateway backend mode for the unavailable-run approval test. |
| tests/test_gateway_approval_legacy_path.py | Pins gateway backend mode during the legacy approval response assertion. |
| CHANGELOG.md | Documents the local approval regression fix. |
Reviews (2): Last reviewed commit: "docs(changelog): add [Unreleased] entry ..." | Re-trigger Greptile
AGENTS.md requires a CHANGELOG entry for user-visible behavior changes, and this is one (local-backend command approvals went from 409-on-every-click to working). The PR shipped the routes.py gate fix without a changelog note; this adds the [Unreleased] -> Fixed entry crediting b3nw / claw-io (#4948). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
nesquena
left a comment
There was a problem hiding this comment.
Review — end-to-end ✅ (approved after fix; pushed the missing CHANGELOG entry)
Independent review of the #4771 regression fix: on the default local in-process backend, every command-approval click returned HTTP 409 gateway_run_unavailable and refused to resolve an approval that resolves perfectly well locally. Reported by b3nw (Discord) and filed as #4948 (claw-io).
What this ships
api/routes.py (+23/-2), tests/test_issue4771_local_approval_regression.py (+222, new), and two existing #4771 tests updated. Agent-authored. I added the missing CHANGELOG entry (commit below).
Traced against upstream hermes-agent
Fresh tarball: this is WebUI-side approval-relay routing. On local, a guarded command parks an _ApprovalEntry in tools.approval._gateway_queues (via the agent's _await_gateway_decision) and the WebUI mirrors it into _pending with _GATEWAY_MIRROR_FLAG; there's no gateway run / _STREAM_RUN_IDS entry by design. The fix only changes WebUI routing — the agent-side approval plumbing and the gateway relay (respond_approval(run_id,…)) are untouched. ✓
Root cause + fix — traced
#4771 added a relay-failure 409 to /api/approval/respond so a gateway approval whose run is gone keeps its card actionable. But the 409 fired regardless of backend: _gateway_pending_approval_without_run_id() is always True on local (the mirror is always populated, the run never exists), so local users 409'd on every click.
The fix AND-gates the 409 on the backend being gateway:
if webui_gateway_chat_enabled(_get_config()) and _gateway_pending_approval_without_run_id(sid, approval_id):
return ... 409 ...
I verified the control flow in _handle_approval_respond:
- Local backend:
webui_gateway_chat_enabledis False → the 409ifis skipped; the gateway-relaytryblock has no_run_id(none on local) so it returns nothing and ends; control falls through to the local resolution (runtime_adapter_enabled()→LegacyJournalRuntimeAdapter.respond_approvalelse_resolve_approval_legacy) → resolves the parked entry, unblocks the agent thread, returns{"ok": ok, "choice": choice}(200). ✓ (the new tests assert the parked thread unblocks with the chosen result) - Gateway, run present: relays to the runs API (200/502) — unchanged. ✓
- Gateway, run gone:
webui_gateway_chat_enabledTrue + predicate True → 409 (#4771behaviour preserved). ✓
The gate is precise: webui_gateway_chat_enabled(cfg) = webui_chat_backend_mode(cfg) == "gateway", and webui_chat_backend_mode defaults to the in-process runtime, returning "gateway" only for an explicit HERMES_WEBUI_CHAT_BACKEND/webui_chat_backend gateway value (generic truthy strings are deliberately ignored so a deployment can't change execution ownership by accident). So the only behavioural change is for the local backend, where the 409 was wrong; gateway deployments are byte-for-byte unchanged.
What I caught + pushed — CHANGELOG entry (commit bc12bb4)
The PR shipped a user-visible behaviour change (local approvals went from "409 on every click" to "works") with no CHANGELOG entry; AGENTS.md requires one for release-note-ready behaviour changes. Added an [Unreleased] → Fixed entry crediting b3nw / #4948.
Other audit — things that are correct already
- Auth/CSRF:
/api/approval/respondsits behind the existing CSRF gate + (when enabled) password/passkey auth — unchanged by this PR. - No double-resolution: on local the gateway-relay path returns nothing (no
_run_id); the single local resolution runs once. - The
except Exception: passfall-through around the gateway block is pre-existing and intended (any gateway-import/relay hiccup falls through to local resolution) — the fix sits inside that guarded block, so a config-read failure can't 500 the approval.
Edge-case matrix
| Scenario | Behavior |
|---|---|
| Local backend, approve/deny | resolves locally → 200, parked thread unblocked ✅ (the fix) |
| Gateway, active run present | relayed to runs API (200/502) ✅ unchanged |
| Gateway, run gone + pending mirror | 409 gateway_run_unavailable ✅ (#4771 preserved) |
HERMES_WEBUI_CHAT_BACKEND unset (default) |
treated as local → no 409 ✅ |
| Generic-truthy backend value | ignored → local ✅ (no accidental ownership change) |
| Gateway config-read raises | caught → falls through to local resolution ✅ |
Tests
tests/test_issue4771_local_approval_regression.py(4) — local→200+unblock (once/deny), gateway→409 preserved. Agent-dependent: they skip locally without hermes-agent (consistent with the repo's baseline) and run in CI; the PR reports verified fail-before/pass-after and 778 passed across approval/gateway/streaming/respond suites.- The two existing #4771 tests updated to pin
HERMES_WEBUI_CHAT_BACKEND=gateway(the 409 is now gateway-deployment behaviour) — correct. - Locally: 12 approval/gateway tests pass, 27 agent-dependent skipped;
webui_chat_backend_modedefault-local/explicit-gateway verified. - Full suite: 10485 passed / 138 skipped / 0 failures in 258s (deselected the pre-existing darwin CRLF flake; agent-dependent approval tests skip locally, run in CI).
py_compileclean on routes.py; CI lint + browser-smoke green (test shards finishing).
Minor observations (non-blocking)
- The broader stale-mirror lifecycle hardening from #4948 (expire the mirror on stream teardown so a cancel/fork doesn't leave a pending card) is correctly scoped OUT — this PR closes the headline local-backend 409; #4948's cancel/fork mirror expiry remains a tracked follow-up.
Recommendation
✅ Approved after fix. Parked at approval — ready for the release agent's merge/tag pipeline.
A minimal, correctly-scoped regression fix: the #4771 relay-failure 409 is only meaningful on a gateway deployment, so gating it on webui_gateway_chat_enabled lets local approvals fall through to the local resolution path (resolving the parked entry and unblocking the agent thread) while preserving the gateway 409 exactly. I traced the full control flow in both modes, confirmed the backend-mode detection is precise (default local, explicit-only gateway), and pushed the missing CHANGELOG entry. Ship.
|
Shipped in v0.51.666 (Release XV, just deployed). Command approvals resolve again on the default local backend — the #4771 409 is now gated on gateway mode. Independent review @nesquena APPROVED; deep gate Codex SAFE + Opus SHIP IT (verified the local fall-through genuinely unblocks the parked agent thread + #4771 gateway behaviour intact + tests re-pinned not weakened); suite 10618. Verified on prod (gate live). Thanks @b3nw + @claw-io for the reports. |
…unavailable (nesquena#4950, nesquena#4771 regression, closes nesquena#4948)
v0.51.666 — Release XV: local approvals no longer 409 as gateway run unavailable (nesquena#4950, nesquena#4771 regression, closes nesquena#4948) # Conflicts: # CHANGELOG.md
Summary
Fixes the
Gateway approval could not be relayed because the active run is unavailable(HTTP 409,code: gateway_run_unavailable) error that fires on every approval click on the default local in-process backend. Reported by b3nw on Discord and independently filed as #4948 by claw-io.This is a regression introduced by #4771 (v0.51.653), not a revert of it — the #4771 behaviour is preserved for gateway deployments.
Root cause
#4771 added an explicit relay-failure 409 to
/api/approval/respondso a gateway approval whose run is gone keeps its card actionable instead of silently failing. But the 409 was emitted regardless of backend.On the default local in-process backend, every guarded command parks an
_ApprovalEntryintools.approval._gateway_queues(via_await_gateway_decision), and the local streaming notify callback (_run_agent_streaming) mirrors it into_pendingtagged with_GATEWAY_MIRROR_FLAG. There is no gateway run and no_STREAM_RUN_IDSentry — by design. So_gateway_pending_approval_without_run_id()returnedTruefor that purely local approval, and_handle_approval_respondreturned the 409, refusing to resolve an approval that resolves perfectly well locally.The 409 fires exactly when
_gateway_queues[sid]still has a live parked entry — i.e. exactly when local resolution would succeed._pending+ gateway mirror (_GATEWAY_MIRROR_FLAG)/api/approval/pendingsession.active_stream_id+_STREAM_RUN_IDSrespond_approval(run_id, …)On a local deployment the second row never exists, yet the first row is always populated — so the predicate was always True and the 409 always fired.
Fix
Gate the 409 on the WebUI actually running the gateway chat backend (
webui_gateway_chat_enabled). Local approvals fall through to the existing local resolution path; gateway behaviour is unchanged.Tests
tests/test_issue4771_local_approval_regression.py(4 tests) drives the real_handle_approval_respondhandler:gateway_run_unavailable409; post-fix all four pass.test_gateway_approval_legacy_path.py,test_approval_unblock.py) to pinHERMES_WEBUI_CHAT_BACKEND=gatewayat respond time, since the 409 is now gateway-deployment behaviour.Relationship to #4948
#4948 (claw-io) reports the same 409 and its environment line explicitly notes it triggers on the local in-process WebUI chat runtime (not gateway-exclusive) — independent confirmation of this root cause. Its suggested fix direction #2 ("prefer local resolve before returning 409 for mirrored-only state") is exactly this fix's mechanism. This PR closes the headline local-backend case; #4948's broader cancel/fork mirror-lifecycle hardening (stale-mirror expiry on stream teardown) remains a follow-up.
Reported-by: b3nw
Reported-by: claw-io (#4948)