feat(server): add DELETE /v1/hosts/{host_id} to deregister a retired host (#2038) - #3238
feat(server): add DELETE /v1/hosts/{host_id} to deregister a retired host (#2038)#3238abhay-codes07 wants to merge 4 commits into
Conversation
…host (omnigent-ai#2038) External hosts self-register via the daemon and become rows in the host store, but `server/routes/hosts.py` exposed only GET/POST, so there was no supported way to remove one. A permanently decommissioned machine lingered as an offline entry in every session-creation host picker, and the only workaround was hand-deleting the postgres row. The persistent teardown already exists: `HostStore.delete_host` unbinds any sessions still pointing at the host (nulls their `host_id`) and deletes the row (which also revokes its launch token). This wires it to a route so retiring a host has a first-class API and a clean IaC story. `DELETE /v1/hosts/{host_id}` mirrors `get_host`'s auth: 404 for an unknown host, 403 when the caller is not the owner. It refuses an online host (409) so an active machine is not pulled out from under running work, and refuses a server-managed sandbox host (409) whose lifecycle belongs to the server that created it. On success it returns 204 and the host is gone from the store and the pickers. Tests: removes an offline host (row and picker), 404 unknown, 409 online, 409 managed sandbox, 403 wrong owner, and that a bound session is unbound rather than left dangling at a deleted row. Each DELETE-path test fails against `main` with 405 (no route) and passes with it. Signed-off-by: abhay-codes07 <abhaysingh0293@gmail.com>
Signed-off-by: abhay-codes07 <abhaysingh0293@gmail.com>
github-actions
Bot
added
size/L
|
@abhay-codes07 This PR is a Bug fix, Feature, or UI / frontend change but the Demo section is missing or only contains a placeholder. These change types require a screenshot or screen recording so reviewers can see the new behaviour without checking out the branch. Please update the Demo section with:
Use |
Deleting a host nulled only omnigent_conversation_metadata.host_id and left runner_id, workspace, and git_branch pointing at a machine that no longer exists. A session in that state is wedged in every direction: it cannot auto-relaunch (that path is gated on host_id), cannot rebind to a new host (the atomic bind matches only runner_id IS NULL, so it returns "session already has a runner bound" forever), and cannot be stopped (Stop requires both host_id and runner_id). There is no API that repairs it, and no foreign key fails loudly to signal the inconsistency. Null all four columns in the same statement instead, matching the full unbind ConversationStore.clear_host_binding already performs for a failed per-session bind. The change lands in the shared primitive rather than behind a flag for the new deregistration route, because it is correct for the only other caller too: managed-sandbox teardown runs after the sandbox has been terminated, so a surviving runner_id there is just as stale. Signed-off-by: SabhyaC26 <sabhyachhabria@gmail.com>
A scheduled task pinned to a connected host stores that host_id as a soft reference with no foreign key. Once the host row is gone the task fails on every single fire with host_not_found, and the user cannot repair it in place: the PATCH validator rejects a null host_id, so the only way out is to delete the task and build it again from scratch. Managed hosts were never pinnable, so deregistering a connected host is the first path that can create this orphan. Refuse the delete with 409 while any task still pins the host, naming the tasks so the caller knows what to fix. Silently unpinning would be worse than the 409: an unpinned task resolves the owner's first online host at fire time, so clearing the pin would quietly relocate the work to a different machine. Re-pointing a task at another host is already a single PATCH, which makes the refusal recoverable with the surface that exists. Paused tasks count too — they break identically once resumed. Reading the tasks needs a host-scoped query rather than a filter over the owner's tasks, because single-user deployments store tasks with a null user_id while their hosts are owned by the reserved "local" user, so an owner filter would miss every pinned task there. Also corrects the route docstring, which is published to the public API reference through openapi.json: it promised the host would stay out of the pickers "forever" and claimed the delete revokes a launch token. Neither holds for the hosts this route can delete. Only managed hosts ever carry a token and those are refused, and an external host keeps its host_id locally, so its daemon recreates the row on the next reconnect. Signed-off-by: SabhyaC26 <sabhyachhabria@gmail.com>
github-actions
Bot
added
size/XL
|
/review |
|
|
@abhay-codes07 who is the call of this API? not sure if we should add it for the sake of adidng it |
|
Closing this PR because it has been labeled The label was last applied on 2026-07-27T23:39:43Z. If you are ready to continue, please reopen this PR or open a new one. |
|
This auto-closed on my inactivity and GitHub would not let me reopen it (422 after the branch was rebased), so I continued it in #4360: same branch rebased onto current main, with your two follow-up commits preserved (@SabhyaC26). It also answers your earlier "who calls this?" question in the description. Thanks for the fully-unbind fix and the scheduled-task-pin guard. |
Related issue
Closes #2038
Summary
External hosts self-register via the daemon (
POST /hosts) and become rows in the host store, butserver/routes/hosts.pyexposed only GET/POST. There was no supported way to remove one, so a permanently decommissioned machine (a retired runner VM, a torn-down IaC host) lingered as an offline entry in every session-creation host picker forever. The only workaround was hand-deleting the postgres row.The persistent teardown already exists:
HostStore.delete_hostunbinds any sessions still pointing at the host (nulls theirhost_id) and deletes the row, which also revokes its launch token. This PR wires it to a route so retiring a host has a first-class API and a clean IaC lifecycle story (register is automatic, now deregister is too).DELETE /v1/hosts/{host_id}:get_host's auth:require_user, 404 for an unknown host, 403 when the caller is not the owner.Scope: this adds the missing server primitive the issue asks for. SDK-client / CLI / web-picker affordances can build on it as follow-ups.
Test Plan
Added to
tests/server/integration/test_hosts_api.py:test_delete_host_removes_offline_host— 204, row gone from store and pickertest_delete_host_404_unknowntest_delete_host_409_when_online— refuses a live host, leaves it in placetest_delete_host_409_managed_sandbox— refuses a server-managed hosttest_delete_host_unbinds_bound_sessions— a bound session'shost_idis nulled, not left danglingtest_delete_host_403_wrong_owner— one user cannot deregister another's hostEach DELETE-path test fails against
mainwith 405 (no route) and passes with the change (verified by stashing the route).ruff check+ruff format --checkclean.Demo
N/A. Backend REST route with no visual surface of its own. Before: no way to remove a retired host;
DELETE /v1/hosts/{id}returns 405 and the host lingers in every picker. After:DELETE /v1/hosts/{id}returns 204 and the host is gone (409 if it is still online or server-managed, 403/404 for the auth cases).Type of change
Test coverage
Coverage notes
The route's own logic (auth, the online / managed / owner guards, the 204 path) is covered directly; the unbind-on-delete behavior is asserted end to end through the route. The underlying
HostStore.delete_hostwas already exercised by its managed-host teardown callers.Changelog
Added
DELETE /v1/hosts/{host_id}so a retired self-registered host can be deregistered instead of lingering in the session-creation host picker forever.