Skip to content

Commit 55a9084

Browse files
ryanmcmillanmarty-clawed-botclaude
authored
fix: reject non-zero delegation_depth on POST/PUT (parity with delega-api#28) (#22)
Self-hosted backend's task handlers were coherent by construction — TaskCreate / TaskUpdate Pydantic schemas don't include delegation_depth, parent_task_id, or root_task_id, so Pydantic v2 silently drops them. POST /tasks always sets root_task_id atomically (either from parent or self), and chain rows are only ever built via POST /tasks/:id/delegate. But silent-drop is indistinguishable from "accepted and honored" from a client's perspective. Hosted API (delega-api#28) landed explicit 400s for the same shapes, making the contract visible. This change mirrors that behavior so clients see identical 400s across deployments. Added: - reject_chain_fields_on_create / _on_update: FastAPI Depends that inspect the raw request body and raise HTTPException(400) when: * POST /tasks has delegation_depth != 0 or any root_task_id * PUT /tasks/:id has any delegation_depth, parent_task_id, or root_task_id (chain fields are immutable once set) Error messages match hosted's wording, pointing callers to POST /tasks/:id/delegate for chain creation. - migrations/006_fix_orphan_chains.py: idempotent sqlite3 script that zeroes delegation_depth + re-opens status='delegated' for any orphan rows (parent/root NULL but depth > 0). No-op if the DB is already coherent, which is the expected case. - tests/test_task_coherence.py: 10 pytest cases covering bare create, non-zero depth reject, root_task_id reject on POST and PUT, parent_task_id still OK on POST but rejected on PUT, regular content updates, and /delegate atomicity. Parent_task_id on POST /tasks is intentionally still accepted — that's the documented path for chain creation via bare POST, and the handler already sets parent + root + depth atomically in that branch. Verified: pytest backend/tests/ passes 47/47 (10 new + 37 existing permissions tests unchanged). Co-authored-by: Marty (Clawdbot) <marty@mcmillan.io> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent adda255 commit 55a9084

3 files changed

Lines changed: 332 additions & 0 deletions

File tree

backend/main.py

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -341,6 +341,52 @@ def get_task_for_agent(
341341
return task
342342

343343

344+
async def _reject_chain_fields(request: Request, *, mode: str) -> None:
345+
# Chain membership (parent_task_id + root_task_id + delegation_depth) is
346+
# set atomically by the server — via POST /tasks/:id/delegate for chain
347+
# tasks, or implicitly on bare POST /tasks where root_task_id = self.id.
348+
# Clients that set these fields directly can create orphan rows
349+
# (delegation_depth > 0 with NULL parent/root), which this guard prevents.
350+
# Parity with delega-api's coherence fix.
351+
try:
352+
raw = await request.json()
353+
except Exception:
354+
return # Let FastAPI's own body parsing raise the user-facing error.
355+
if not isinstance(raw, dict):
356+
return
357+
if "root_task_id" in raw:
358+
raise HTTPException(
359+
status_code=400,
360+
detail="root_task_id cannot be set via this endpoint. Use POST /tasks/:id/delegate to create a chain task with parent_task_id and root_task_id set atomically.",
361+
)
362+
dd = raw.get("delegation_depth")
363+
if mode == "create":
364+
if dd is not None and dd != 0:
365+
raise HTTPException(
366+
status_code=400,
367+
detail="delegation_depth cannot be set via POST /tasks. Use POST /tasks/:id/delegate to create a chain task with parent_task_id and root_task_id set atomically.",
368+
)
369+
elif mode == "update":
370+
if "delegation_depth" in raw:
371+
raise HTTPException(
372+
status_code=400,
373+
detail="delegation_depth is immutable once set. Chain membership is only established via POST /tasks/:id/delegate.",
374+
)
375+
if "parent_task_id" in raw:
376+
raise HTTPException(
377+
status_code=400,
378+
detail="parent_task_id is immutable once set. Chain membership cannot be re-parented via PUT /tasks/:id.",
379+
)
380+
381+
382+
async def reject_chain_fields_on_create(request: Request) -> None:
383+
await _reject_chain_fields(request, mode="create")
384+
385+
386+
async def reject_chain_fields_on_update(request: Request) -> None:
387+
await _reject_chain_fields(request, mode="update")
388+
389+
344390
@app.middleware("http")
345391
async def rate_limit_middleware(request: Request, call_next):
346392
path = request.url.path
@@ -1070,6 +1116,7 @@ def create_task(
10701116
request: Request,
10711117
db: Session = Depends(get_db),
10721118
agent: Optional[models.Agent] = Depends(get_current_agent),
1119+
_coherence: None = Depends(reject_chain_fields_on_create),
10731120
):
10741121
# Optional dedup check via header
10751122
dedup_header = request.headers.get("X-Dedup-Check", "").lower()
@@ -1144,6 +1191,7 @@ def update_task(
11441191
task: schemas.TaskUpdate,
11451192
db: Session = Depends(get_db),
11461193
agent: Optional[models.Agent] = Depends(get_current_agent),
1194+
_coherence: None = Depends(reject_chain_fields_on_update),
11471195
):
11481196
db_task = get_task_for_agent(db, task_id, agent, require_mutation=True)
11491197

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
"""
2+
Migration 006: Fix orphan chain rows.
3+
4+
Parity with the delega-api coherence fix (hosted: delega-dev/delega-api#28).
5+
Zeroes delegation_depth and re-opens status for any rows where
6+
delegation_depth > 0 but parent_task_id / root_task_id are NULL — those
7+
rows could not have been created via the server's /tasks or
8+
/tasks/:id/delegate handlers in their current form, but may exist from
9+
direct SQL writes, prior buggy API versions, or imports.
10+
11+
Idempotent: reports "no orphans" and exits cleanly if the DB is already
12+
coherent, which is the expected case for installs minted after the
13+
handler-level guards landed in the same PR as this migration.
14+
15+
Usage:
16+
python backend/migrations/006_fix_orphan_chains.py
17+
DELEGA_DB_PATH=/path/to/custom.db python backend/migrations/006_fix_orphan_chains.py
18+
"""
19+
import sqlite3
20+
import os
21+
22+
23+
def migrate(db_path: str):
24+
conn = sqlite3.connect(db_path)
25+
cur = conn.cursor()
26+
27+
cur.execute(
28+
"""
29+
SELECT COUNT(*) FROM tasks
30+
WHERE parent_task_id IS NULL
31+
AND root_task_id IS NULL
32+
AND delegation_depth > 0
33+
"""
34+
)
35+
orphans = cur.fetchone()[0]
36+
37+
if orphans == 0:
38+
print(" No orphan rows found. DB is coherent; nothing to do.")
39+
conn.close()
40+
return
41+
42+
cur.execute(
43+
"""
44+
UPDATE tasks
45+
SET delegation_depth = 0,
46+
status = CASE
47+
WHEN status = 'delegated' THEN 'open'
48+
ELSE status
49+
END,
50+
updated_at = datetime('now')
51+
WHERE parent_task_id IS NULL
52+
AND root_task_id IS NULL
53+
AND delegation_depth > 0
54+
"""
55+
)
56+
conn.commit()
57+
conn.close()
58+
print(f" Normalized {orphans} orphan row(s): delegation_depth → 0, status 'delegated' → 'open'.")
59+
print("Migration 006 complete.")
60+
61+
62+
if __name__ == "__main__":
63+
default_path = os.path.join(
64+
os.path.dirname(os.path.dirname(os.path.abspath(__file__))),
65+
"data", "delega.db",
66+
)
67+
db_path = os.environ.get("DELEGA_DB_PATH", default_path)
68+
print(f"Running migration on: {db_path}")
69+
migrate(db_path)
Lines changed: 215 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,215 @@
1+
"""Tests for delegation_depth coherence guards on the tasks endpoints.
2+
3+
Parity with the hosted API's coherence fix (delega-dev/delega-api#28):
4+
chain fields (parent_task_id, root_task_id, delegation_depth) must be
5+
managed atomically by the server. Clients cannot set them directly on
6+
POST /tasks or mutate them via PUT /tasks/:id. Chains are built only
7+
via POST /tasks/:id/delegate, which sets all three atomically.
8+
"""
9+
10+
import os
11+
import sys
12+
import pytest
13+
from pathlib import Path
14+
15+
sys.path.insert(0, str(Path(__file__).resolve().parent.parent))
16+
17+
os.environ["DELEGA_REQUIRE_AUTH"] = "true"
18+
os.environ["DELEGA_DB_PATH"] = ":memory:"
19+
20+
from sqlalchemy import create_engine
21+
from sqlalchemy.orm import sessionmaker
22+
from fastapi.testclient import TestClient
23+
24+
import main
25+
import models
26+
from main import app, get_db, derive_key_lookup, derive_key_hash
27+
28+
29+
# ---------- Fixtures (mirrors test_permissions.py pattern) ----------
30+
31+
@pytest.fixture(autouse=True)
32+
def fresh_db(tmp_path):
33+
db_path = str(tmp_path / "test.db")
34+
test_engine = create_engine(
35+
f"sqlite:///{db_path}",
36+
connect_args={"check_same_thread": False},
37+
)
38+
models.Base.metadata.create_all(bind=test_engine)
39+
TestSession = sessionmaker(autocommit=False, autoflush=False, bind=test_engine)
40+
41+
def override_get_db():
42+
db = TestSession()
43+
try:
44+
yield db
45+
finally:
46+
db.close()
47+
48+
app.dependency_overrides[get_db] = override_get_db
49+
_orig_session_local = main.SessionLocal
50+
main.SessionLocal = TestSession
51+
main._rate_limiter._hits.clear()
52+
53+
yield test_engine
54+
55+
main.SessionLocal = _orig_session_local
56+
app.dependency_overrides.clear()
57+
58+
59+
@pytest.fixture()
60+
def client():
61+
return TestClient(app, base_url="http://localhost")
62+
63+
64+
def make_agent(engine, name: str, *, is_admin: bool = False):
65+
import secrets as _secrets
66+
raw_key = "dlg_" + _secrets.token_urlsafe(32)
67+
salt = _secrets.token_hex(16)
68+
key_hash = derive_key_hash(raw_key, salt)
69+
key_lookup = derive_key_lookup(raw_key)
70+
71+
Session = sessionmaker(bind=engine)
72+
db = Session()
73+
agent = models.Agent(
74+
name=name,
75+
api_key=raw_key,
76+
key_hash=key_hash,
77+
key_salt=salt,
78+
key_lookup=key_lookup,
79+
is_admin=is_admin,
80+
permissions=[],
81+
active=True,
82+
)
83+
db.add(agent)
84+
db.commit()
85+
db.refresh(agent)
86+
agent_id = agent.id
87+
db.close()
88+
return agent_id, raw_key
89+
90+
91+
def auth(key: str):
92+
return {"X-Agent-Key": key}
93+
94+
95+
# ---------- POST /tasks coherence ----------
96+
97+
class TestCreateTaskCoherence:
98+
def test_bare_create_defaults_to_depth_zero_root_self(self, fresh_db, client):
99+
_, key = make_agent(fresh_db, "a", is_admin=True)
100+
r = client.post("/api/tasks", json={"content": "hello"}, headers=auth(key))
101+
assert r.status_code == 200
102+
body = r.json()
103+
assert body["delegation_depth"] == 0
104+
assert body["parent_task_id"] is None
105+
assert body["root_task_id"] == body["id"]
106+
107+
def test_explicit_zero_depth_accepted(self, fresh_db, client):
108+
_, key = make_agent(fresh_db, "a", is_admin=True)
109+
r = client.post(
110+
"/api/tasks",
111+
json={"content": "explicit zero", "delegation_depth": 0},
112+
headers=auth(key),
113+
)
114+
assert r.status_code == 200
115+
assert r.json()["delegation_depth"] == 0
116+
117+
def test_non_zero_depth_rejected_400(self, fresh_db, client):
118+
_, key = make_agent(fresh_db, "a", is_admin=True)
119+
r = client.post(
120+
"/api/tasks",
121+
json={"content": "orphan attempt", "delegation_depth": 3},
122+
headers=auth(key),
123+
)
124+
assert r.status_code == 400
125+
assert "delegation_depth" in r.json()["detail"]
126+
127+
def test_root_task_id_in_body_rejected_400(self, fresh_db, client):
128+
_, key = make_agent(fresh_db, "a", is_admin=True)
129+
r = client.post(
130+
"/api/tasks",
131+
json={"content": "x", "root_task_id": 99},
132+
headers=auth(key),
133+
)
134+
assert r.status_code == 400
135+
assert "root_task_id" in r.json()["detail"]
136+
137+
def test_parent_task_id_still_accepted(self, fresh_db, client):
138+
_, key = make_agent(fresh_db, "a", is_admin=True)
139+
parent = client.post("/api/tasks", json={"content": "parent"}, headers=auth(key)).json()
140+
r = client.post(
141+
"/api/tasks",
142+
json={"content": "child", "parent_task_id": parent["id"]},
143+
headers=auth(key),
144+
)
145+
assert r.status_code == 200
146+
child = r.json()
147+
assert child["parent_task_id"] == parent["id"]
148+
assert child["root_task_id"] == parent["id"]
149+
assert child["delegation_depth"] == 1
150+
151+
152+
# ---------- PUT /tasks/:id coherence ----------
153+
154+
class TestUpdateTaskCoherence:
155+
def test_delegation_depth_in_put_rejected_400(self, fresh_db, client):
156+
_, key = make_agent(fresh_db, "a", is_admin=True)
157+
task = client.post("/api/tasks", json={"content": "x"}, headers=auth(key)).json()
158+
r = client.put(
159+
f"/api/tasks/{task['id']}",
160+
json={"delegation_depth": 9},
161+
headers=auth(key),
162+
)
163+
assert r.status_code == 400
164+
assert "delegation_depth" in r.json()["detail"]
165+
166+
def test_parent_task_id_in_put_rejected_400(self, fresh_db, client):
167+
_, key = make_agent(fresh_db, "a", is_admin=True)
168+
task = client.post("/api/tasks", json={"content": "x"}, headers=auth(key)).json()
169+
r = client.put(
170+
f"/api/tasks/{task['id']}",
171+
json={"parent_task_id": 42},
172+
headers=auth(key),
173+
)
174+
assert r.status_code == 400
175+
assert "parent_task_id" in r.json()["detail"]
176+
177+
def test_root_task_id_in_put_rejected_400(self, fresh_db, client):
178+
_, key = make_agent(fresh_db, "a", is_admin=True)
179+
task = client.post("/api/tasks", json={"content": "x"}, headers=auth(key)).json()
180+
r = client.put(
181+
f"/api/tasks/{task['id']}",
182+
json={"root_task_id": 7},
183+
headers=auth(key),
184+
)
185+
assert r.status_code == 400
186+
assert "root_task_id" in r.json()["detail"]
187+
188+
def test_regular_content_update_still_works(self, fresh_db, client):
189+
_, key = make_agent(fresh_db, "a", is_admin=True)
190+
task = client.post("/api/tasks", json={"content": "old"}, headers=auth(key)).json()
191+
r = client.put(
192+
f"/api/tasks/{task['id']}",
193+
json={"content": "new"},
194+
headers=auth(key),
195+
)
196+
assert r.status_code == 200
197+
assert r.json()["content"] == "new"
198+
199+
200+
# ---------- POST /tasks/:id/delegate atomicity ----------
201+
202+
class TestDelegateAtomicity:
203+
def test_delegate_sets_chain_fields_atomically(self, fresh_db, client):
204+
_, key = make_agent(fresh_db, "a", is_admin=True)
205+
parent = client.post("/api/tasks", json={"content": "root"}, headers=auth(key)).json()
206+
r = client.post(
207+
f"/api/tasks/{parent['id']}/delegate",
208+
json={"content": "delegated"},
209+
headers=auth(key),
210+
)
211+
assert r.status_code == 200
212+
child = r.json()
213+
assert child["parent_task_id"] == parent["id"]
214+
assert child["root_task_id"] == parent["id"]
215+
assert child["delegation_depth"] == 1

0 commit comments

Comments
 (0)