mirror of
https://github.com/0xWheatyz/handler.git
synced 2026-08-30 04:36:24 +00:00
fix(projects): cascade dependents when deleting a project or agent
Removing a repo returned 500. `delete_project` deleted only the projects row, but `commands`, `agents`, `approvals`, and `schedules` all carry a foreign key to `projects.id` with no ON DELETE CASCADE — and every real project has at least the `sync` command queued at registration referencing it, so Postgres rejected the delete with a ForeignKeyViolation (commands_project_id_fkey). The existing tests only deleted dependent-free projects, so it went unnoticed (and SQLite, though it has FK enforcement on here, was never exercised with a referencing row). delete_project now clears dependents in FK-safe order: agent-owned rows (checkmarks before log_entries per the use_alter cycle, approvals authored by those agents, and shared_context attribution nulled since it's a global table), then schedules (which reference commands via last_command_id), then the project-scoped approvals/commands/agents, then the project. delete_agent had the same latent bug — a spawned agent always accrues a checkmark + log entries via the hooks, which the log_entries/checkmarks FKs would block — so it shares the same _purge_agent_dependents helper. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BxKY28XKCM6o4ag3nmaVsZ
This commit is contained in:
@@ -337,12 +337,59 @@ def update_project(conn: Connection, project_id: str, **fields: Any) -> dict | N
|
||||
return get_project(conn, project_id)
|
||||
|
||||
|
||||
def _purge_agent_dependents(conn: Connection, agent_ids: list[int]) -> None:
|
||||
"""Delete (or detach) everything that references the given agents, so the agent rows
|
||||
themselves can be removed without tripping a foreign key.
|
||||
|
||||
``shared_context`` is a *global* table keyed by ``key`` — its rows outlive any one
|
||||
agent, so we only null the ``set_by_agent_id`` attribution rather than delete them.
|
||||
``checkmarks`` must go before ``log_entries`` (it carries an FK to ``log_entries`` via
|
||||
the ``use_alter`` cycle). Approvals authored by these agents go with them.
|
||||
"""
|
||||
if not agent_ids:
|
||||
return
|
||||
conn.execute(
|
||||
shared_context.update()
|
||||
.where(shared_context.c.set_by_agent_id.in_(agent_ids))
|
||||
.values(set_by_agent_id=None)
|
||||
)
|
||||
conn.execute(approvals.delete().where(approvals.c.approved_by_agent_id.in_(agent_ids)))
|
||||
conn.execute(checkmarks.delete().where(checkmarks.c.agent_id.in_(agent_ids)))
|
||||
conn.execute(log_entries.delete().where(log_entries.c.agent_id.in_(agent_ids)))
|
||||
|
||||
|
||||
def delete_project(conn: Connection, project_id: str) -> bool:
|
||||
"""Remove a project and everything scoped to it.
|
||||
|
||||
Every project accumulates FK-referencing rows (at minimum the ``sync`` command queued
|
||||
at registration, plus each agent's log/checkmark history), so the bare project delete
|
||||
would violate ``commands``/``agents``/``approvals``/``schedules`` foreign keys. Clear
|
||||
the dependents in FK-safe order first: agent-owned rows, then ``schedules`` (which
|
||||
reference ``commands`` via ``last_command_id``), then the remaining project-scoped
|
||||
rows, then the agents, then the project itself.
|
||||
"""
|
||||
agent_ids = [
|
||||
row[0]
|
||||
for row in conn.execute(
|
||||
select(agents.c.id).where(agents.c.project_id == project_id)
|
||||
).all()
|
||||
]
|
||||
_purge_agent_dependents(conn, agent_ids)
|
||||
conn.execute(schedules.delete().where(schedules.c.project_id == project_id))
|
||||
conn.execute(approvals.delete().where(approvals.c.project_id == project_id))
|
||||
conn.execute(commands.delete().where(commands.c.project_id == project_id))
|
||||
conn.execute(agents.delete().where(agents.c.project_id == project_id))
|
||||
result = conn.execute(projects.delete().where(projects.c.id == project_id))
|
||||
return result.rowcount > 0
|
||||
|
||||
|
||||
def delete_agent(conn: Connection, project_id: str, name: str) -> bool:
|
||||
"""Remove a single agent row, first clearing its log/checkmark/approval history so the
|
||||
``log_entries``/``checkmarks``/``approvals`` foreign keys don't block the delete."""
|
||||
agent = get_agent_by_name(conn, project_id, name)
|
||||
if agent is None:
|
||||
return False
|
||||
_purge_agent_dependents(conn, [agent["id"]])
|
||||
result = conn.execute(
|
||||
agents.delete().where(agents.c.project_id == project_id, agents.c.name == name)
|
||||
)
|
||||
|
||||
@@ -24,6 +24,51 @@ def test_delete_agent_row(conn):
|
||||
assert repo.get_agent_by_name(conn, "p", "api") is None
|
||||
|
||||
|
||||
def test_delete_project_cascades_all_dependents(conn):
|
||||
"""A real project always has FK-referencing rows (the sync command, agent history,
|
||||
approvals, schedules). Deleting it must clear them, not raise a ForeignKeyViolation."""
|
||||
from datetime import UTC, datetime
|
||||
|
||||
repo.create_project(conn, "p", "/tmp/p", git_remote="https://github.com/me/p.git")
|
||||
# The sync command queued at registration — the exact row that blocked the delete.
|
||||
repo.enqueue_command(conn, "sync", project_id="p", requested_by="operator:web")
|
||||
agent = repo.create_agent(conn, "p", "api", "/tmp/p/api")
|
||||
repo.insert_log_entry(conn, agent_id=agent["id"], status="working", summary="did work")
|
||||
repo.upsert_checkmark_row(conn, agent_id=agent["id"], status="working")
|
||||
repo.set_shared_context(conn, "db", "postgres", agent["id"])
|
||||
repo.record_approval(conn, "p", "feat/x", "approved", approved_by_agent_id=agent["id"])
|
||||
repo.create_schedule(
|
||||
conn, "p", "nightly", "run the thing", 3600, datetime.now(UTC)
|
||||
)
|
||||
|
||||
assert repo.delete_project(conn, "p") is True
|
||||
|
||||
# Project and everything scoped to it are gone.
|
||||
assert repo.get_project(conn, "p") is None
|
||||
assert repo.get_agent_by_name(conn, "p", "api") is None
|
||||
assert repo.list_commands(conn, project_id="p") == []
|
||||
assert repo.list_approvals(conn, "p") == []
|
||||
assert repo.list_schedules(conn) == []
|
||||
# The global shared-context row survives, with its agent attribution cleared.
|
||||
ctx = repo.get_shared_context_key(conn, "db")
|
||||
assert ctx is not None and ctx["value"] == "postgres"
|
||||
assert ctx["set_by_agent_id"] is None
|
||||
|
||||
|
||||
def test_delete_agent_cascades_log_and_checkmark(conn):
|
||||
"""Every spawned agent accrues a checkmark + log entries via the hooks; removing the
|
||||
agent must clear them rather than trip the log_entries/checkmarks foreign keys."""
|
||||
repo.create_project(conn, "p", "/tmp/p")
|
||||
agent = repo.create_agent(conn, "p", "api", "/tmp/p/api")
|
||||
repo.insert_log_entry(conn, agent_id=agent["id"], status="working", summary="x")
|
||||
repo.upsert_checkmark_row(conn, agent_id=agent["id"], status="working")
|
||||
|
||||
assert repo.delete_agent(conn, "p", "api") is True
|
||||
assert repo.get_agent_by_name(conn, "p", "api") is None
|
||||
assert repo.get_checkmark(conn, agent["id"]) is None
|
||||
assert repo.get_log(conn, agent["id"]) == []
|
||||
|
||||
|
||||
def test_enqueue_get_and_list_command(conn):
|
||||
repo.create_project(conn, "p", "/tmp/p")
|
||||
cmd = repo.enqueue_command(
|
||||
|
||||
Reference in New Issue
Block a user