Skip to content

feat(server): DELETE /v1/hosts/{host_id} to deregister a retired host (#2038) - #4360

Open
abhay-codes07 wants to merge 4 commits into
omnigent-ai:mainfrom
abhay-codes07:feat/deregister-host-route
Open

feat(server): DELETE /v1/hosts/{host_id} to deregister a retired host (#2038)#4360
abhay-codes07 wants to merge 4 commits into
omnigent-ai:mainfrom
abhay-codes07:feat/deregister-host-route

Conversation

@abhay-codes07

Copy link
Copy Markdown
Contributor

Related issue

Closes #2038

Summary

Continues #3238, which auto-closed after 7 days of inactivity (my fault — I did not reply in time) and could not be reopened (GitHub 422 on a closed PR whose branch was later rebased). Same branch, rebased onto current main, with @SabhyaC26's follow-up commits preserved.

External hosts self-register via the daemon and become rows in the host store, but server/routes/hosts.py exposed only GET/POST, so a permanently decommissioned machine lingered as an offline entry in every session-creation host picker forever, with no supported way to remove it.

This adds DELETE /v1/hosts/{host_id} and hardens the teardown:

  • Route (mine): mirrors get_host's auth (404 unknown → 403 non-owner), refuses an online host (409) and a server-managed sandbox host (409), else deletes and returns 204.
  • Fully unbind on delete (@SabhyaC26): deleting a host now nulls runner_id / workspace / git_branch alongside host_id, not just host_id. A surviving runner_id wedged the session (rebind is gated on runner_id IS NULL); clearing host_id + workspace together respects ck_conversation_metadata_workspace_required_for_host.
  • Refuse deregistration while scheduled tasks pin the host (@SabhyaC26): a scheduled task stores its target host_id as a soft reference (no FK). Deleting a pinned host would leave each task failing at fire time with host_not_found, which the user cannot recover from. The route now returns 409 and names the pinning tasks (paused tasks count — they break the same way once resumed), via a new ScheduledTaskStore.list_by_host_id.

Who calls this (per the earlier review question)

Host-lifecycle / IaC tooling and self-hosters who rotate runner machines (the #2038 use case: register is automatic, deregister was impossible). The scheduled-task-pin guard above is the concrete safety case that motivated finishing it: without a supported delete, a retired-then-recreated host leaves pinned tasks silently broken. A future web-UI "remove host" affordance can build on the same route.

Test Plan

OMNIGENT_SKIP_WEB_UI=true uv run pytest \
  tests/server/integration/test_hosts_api.py \
  tests/stores/test_host_store.py \
  tests/stores/test_scheduled_task_store.py \
  tests/server/test_openapi_drift.py -q
# 123 passed

Covers: offline delete (row + picker gone), 404 unknown, 409 online, 409 managed sandbox, 403 wrong owner, full session unbind (row reusable afterward), 409 when a scheduled task pins the host (incl. paused), tasks pinned elsewhere ignored, delete succeeds after a task is repointed, and list_by_host_id at the store level. ruff check + ruff format --check clean; openapi.json regenerated (drift test green).

Demo

N/A. Backend REST route + store changes with no visual surface. Before: no way to remove a retired host (DELETE 405); a pinned task on a deleted host fails forever with host_not_found. After: DELETE /v1/hosts/{id} returns 204 (409 if online / server-managed / pinned by tasks, 403/404 for auth), and a deleted host fully unbinds its sessions.

Type of change

  • Bug fix
  • Feature
  • UI / frontend change
  • Refactor / chore
  • Docs
  • Test / CI
  • Breaking change

Test coverage

  • Unit tests added / updated
  • Integration tests added / updated
  • E2E tests added / updated
  • Manual verification completed
  • Existing tests cover this change
  • Not applicable

Changelog

Added DELETE /v1/hosts/{host_id} to deregister a retired host: it fully unbinds any sessions still pointing at it and refuses (409) to delete a host that is online, server-managed, or still pinned by scheduled tasks.

abhay-codes07 and others added 3 commits August 7, 2026 18:58
…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>
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>
Copilot AI lite review requested due to automatic review settings August 7, 2026 14:32

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@github-actions github-actions Bot added the P2-medium Priority: bug with workaround, important feature request label Aug 7, 2026
@github-actions
github-actions Bot requested a review from dbczumar August 7, 2026 14:33
@github-actions github-actions Bot added the size/XL Pull request size: XL label Aug 7, 2026
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>
@abhay-codes07
abhay-codes07 force-pushed the feat/deregister-host-route branch from 1525ae8 to ed750dc Compare August 7, 2026 14:43
@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

@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:

  • A screenshot or screen recording of the change, or
  • A link to a hosted video or GIF showing the new behaviour.

Use N/A only when the change has no user-visible effect whatsoever (e.g. a pure refactor or test-only change). If that's the case, uncheck the relevant type box and check Refactor / chore or Test / CI instead.

@github-actions github-actions Bot added the needs-demo PR needs a demo screenshot or recording label Aug 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-demo PR needs a demo screenshot or recording P2-medium Priority: bug with workaround, important feature request size/XL Pull request size: XL waiting-for-review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature] No way to deregister/delete an external self-registered host — retired hosts linger in the host picker forever

4 participants