feat: add remote-only and stateless serverless controller modes - #163
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7b8dd12fd7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
fwerkor
changed the title
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5767c65eae
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 28e4a00aef
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 90aebaa117
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| path.parent.mkdir(parents=True, exist_ok=True) | ||
| data = { | ||
| "version": 1, | ||
| "workers": [ |
There was a problem hiding this comment.
When state_backend=redis and the controller cold-starts after invite generation but before enrollment, the invite is rejected because create_invite() calls _save_registry_unlocked(), but this payload contains only workers and the loader likewise reconstructs only workers, leaving self.invites empty. This breaks remote enrollment across exactly the serverless cold starts Redis is intended to survive; serialize and restore unexpired pending invites as well.
Useful? React with 👍 / 👎.
| if get_settings().state_backend != "file": | ||
| get_state_store().write_bytes("dynamic-mcp.json", (payload + "\n").encode("utf-8")) |
There was a problem hiding this comment.
When two Redis-backed controller instances concurrently mutate different dynamic MCP servers, each process's asyncio.Lock protects only that instance: both can load the same snapshot and this unconditional full-registry write then causes the later writer to discard the other registration or configuration update. Hold the state store's distributed lock across each complete load/modify/save transaction, including refresh updates.
Useful? React with 👍 / 👎.
| with state_lock("audit.jsonl"): | ||
| existing = store.read_bytes("audit.jsonl") or b"" | ||
| store.write_bytes("audit.jsonl", existing + encoded.encode("utf-8")) | ||
| _enforce_state_audit_storage_limit(settings.max_audit_log_bytes) |
There was a problem hiding this comment.
When two Redis-backed replicas emit audit values above the inline threshold concurrently, _serialize_audit_value() writes each external payload before this distributed lock is acquired. The replica entering the lock first calls _enforce_state_audit_storage_limit(), which immediately deletes every currently unreferenced audit-payloads/* key, so it can delete the other replica's payload before that replica appends its reference; full audit queries then report the payload as unavailable. Acquire the shared lock before externalizing the record or retain fresh unreferenced payloads.
Useful? React with 👍 / 👎.
| redis = [ | ||
| "redis>=5.0.0,<7", | ||
| ] |
There was a problem hiding this comment.
In the documented Docker Compose serverless setup, setting LOCAL_SHELL_MCP_STATE_BACKEND=redis cannot work with the published image: Redis is defined only in this optional extra, while the Dockerfile installs .[dev] and requirements-agent.txt contains no Redis client. The first state-store access therefore raises the missing-redis runtime error and breaks OAuth and other control-plane operations; install the redis extra in the official runtime image that now exposes this configuration.
Useful? React with 👍 / 👎.
| "--max-time", | ||
| str(_worker_curl_timeout(timeout_s)), |
There was a problem hiding this comment.
When remote_peer_transfer_timeout_s is configured above remote_job_timeout_s, direct transfers still terminate at the shorter job timeout because this new upload path passes the peer timeout through _worker_curl_timeout(), which clamps it to remote_job_timeout_s. The controller and destination receiver can remain valid for the longer configured interval while the source curl exits early, so large transfers fail or unnecessarily fall back; either avoid this clamp for peer transfers or validate the two settings' relationship.
Useful? React with 👍 / 👎.
1 participant
Summary
disable_local/LOCAL_SHELL_MCP_DISABLE_LOCAL(default:false) and tighten remote-only behavior across MCP tools and WebUIstateless_controllerplus pluggablefile,memory, and optional Redis control-state backends so the controller does not require a persistent volumeautotries direct → object store → relayServerless scope
stateless_controller=trueremoves the persistent-volume requirement. The default memory backend is intentionally ephemeral: cold starts invalidate pending remote invites and worker identities as well as other control state. Usestate_backend=rediswhen that state must survive cold starts. Active remote RPC queues/futures still live in one controller process, so remote-worker deployments should currently run a single active controller replica rather than horizontally load-balanced replicas.Validation
PYTHONPATH=src pytest -q— 697 passed, 1 skippedPYTHONPATH=src ruff check .PYTHONPATH=src python scripts/export-tools-json.pypython scripts/check-doc-i18n.pymkdocs build --strictgit diff --checksecret_scan— only pre-existing/test fixture generic-assignment matches