fix(control-channel): allow web:setup over the ZMQ control channel - #17
Open
rakhimovv wants to merge 1 commit into
Open
fix(control-channel): allow web:setup over the ZMQ control channel#17rakhimovv wants to merge 1 commit into
rakhimovv wants to merge 1 commit into
Conversation
`setup` is dispatched in run.py and routed by the console as `/api/setup`, but it was never listed in `_COMMANDS`, so it is absent from `WEB_COMMAND_VERBS` — the allow-list the ZMQ control channel checks. The browser is unaffected because `_enqueue` puts commands on the queue directly, so this only refused the ZMQ path, with `command not allowed: 'setup'`. Without it a non-eval (deploy) config cannot be driven to a running state over the channel: `web:run` is accepted but `_dispatch_run` returns early on `not session.is_setup_done`, with no log line. The sequence in the `eva_ctl.py` docstring fails at `web:setup` for this reason. The existing consistency test only asserts `WEB_COMMAND_VERBS <= branches`, which a verb missing from the catalog satisfies trivially, so add the other direction plus a channel-level test. Both fail before this change.
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
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.
Thanks for putting this out publicly — being able to run the whole stack against the fake nodes with no robot attached is what made it possible to find this.
The question behind the PR: is
setupdeliberately left out of_COMMANDS, or is it an omission? I could not find a reason for it in the code, so I have assumed the latter. Happy to close this if the exclusion is intentional.What I ran into
run.pydispatches asetupverb, and the HTTP console reaches it:src/core/app/run.py:729—if verb == "setup":src/core/app/console/server.py:3081—"/api/setup": "web:setup"But
setupis not in_COMMANDS, so it is not inWEB_COMMAND_VERBS, which the ZMQ control channel uses as its allow-list (control_channel.py:29, checked at:73-74). The browser is unaffected —_enqueueatserver.py:1525puts the command on the queue directly, without consulting the allow-list — so this only shows up on the ZMQ path.On
mainat0e0e5f8, using your own test helpers:warmupis there as a control, so the channel itself works.setupspecifically is refused, and nothing is queued.The practical effect: a non-eval (deploy) config cannot be driven to a running state over the control channel.
web:runis accepted, but_dispatch_runreturns early atrun.py:1117onnot session.is_setup_done. There is no log line, so it reads as a silent no-op. The sequence in theeva_ctl.pymodule docstring (select_mode→setup→run) stops at the second step for this reason. Eval configs are unaffected, sincebootstrapandwarmupare both in the catalog.Where it seems to have come from
control_channel.pyoriginally carried its own inline_ALLOWED_VERBS, and that set did include"setup"(git show 8a4fc87:src/core/app/control_channel.py, line 52). When the list was centralised intocommand_catalog.pyin77fd2dc, the new_COMMANDSdid not carrysetupacross.git log -p --follow -- src/core/app/command_catalog.py | grep -c '"setup"'returns0.The change
One catalog entry, plus two tests.
The existing consistency test at
tests/app/test_control_channel.py:12-19asserts onlyWEB_COMMAND_VERBS <= branches | {"select_collect_task"}. A verb that exists as arun.pybranch but is missing from_COMMANDSsatisfies that subset, so nothing flagged it. This PR adds the other direction, plus a channel-level test forweb:setup. Both fail before the catalog entry:and pass with it.
ruff check .andruff format --check .are both clean.The control ids on the new entry are the two bound to
/api/setupinconsole/static/js/main.js:404,407;run.js:370also fires it automatically with no button involved.End-to-end check with the fix applied
To confirm the catalog entry is sufficient rather than just necessary, I drove a full offline stack —
examples/fake_policyplusexamples/hardware/agibot_g2/run_fake_node.shplus headlessevaon a deploy config withcontrol_channel.enabled=True— entirely over ZMQ viaeva_ctl:Before the change the same script stops at
command not allowed: 'setup',session_statusstaysunset, andstep_indexnever leaves 0. So this one entry is what unblocks the whole headless deploy path, not just the single rejected command.Possibly relevant to #14
Flagging this only in case it's useful for #14. Its MCP
policy_runsends, in order,web:select_mode:real,web:switch_task:…,web:setup,web:runthroughEvaControlClient.require_ok, which raisesRuntimeErroronok: false— so against a live EVA the third of those would hit this. As far as I can tell the stub client intests/tools/test_mcp.pyreturns{"ok": True, ...}for anycmd, so I don't think that branch's own tests would surface it. I have not run that branch, so the runtime behaviour is inferred rather than observed. What I did check is its head (d6f40c0):_COMMANDSthere still holds 47 verbs withwarmuppresent and nosetupentry, so the rejection path is unchanged on that branch.Tested on Ubuntu 22.04, Python 3.11, fully offline (fake node plus
examples/fake_policy, no hardware).