Skip to content

Commit 058e35b

Browse files
UN-3479 [FIX] Address Greptile: HITL auth errors, lookup staging, agentic re-run idempotency
- client.get_review_settings: only suppress the backend's DoesNotExist (500); re-raise 401/403/429 so an auth error can't silently drop a configured HITLSettings row. - lookups._replay_one_version: return after a template/adapter staging failure instead of publishing a version with stale content. - agentic _clone_prompt_versions / _clone_schemas: adopt children already on target (keyed by version) so a re-run against the same pair doesn't re-create duplicates — mirrors the lookups version-replay guard. Tests added for each. The draft_cache TOCTOU (P2) is already mitigated: the `is None` write guard guarantees a valid draft id is never overwritten by a peer's failure; only a benign extra GET can occur. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: 4f5ab436-eabb-4464-9a96-f024724bc818
1 parent e3604d7 commit 058e35b

7 files changed

Lines changed: 611 additions & 5 deletions

File tree

src/unstract/clone/DESIGN_cloud_entity_support.md

Lines changed: 442 additions & 0 deletions
Large diffs are not rendered by default.

src/unstract/clone/client.py

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -775,15 +775,18 @@ def get_review_settings(self, workflow_id: str) -> dict[str, Any] | None:
775775
"""Fetch a workflow's HITLSettings row, or ``None`` if absent.
776776
777777
The backend's ``settings_using_workflow`` raises ``DoesNotExist``
778-
(→ 500) when no row exists rather than 404, so any error here is
779-
treated as "no settings to clone".
778+
(→ 500) when no row exists rather than 404, so only a 500 is treated
779+
as "no settings to clone". Other errors (401/403/429) must surface —
780+
suppressing them would silently drop a configured HITLSettings row.
780781
"""
781782
try:
782783
body = self._request(
783784
"GET", f"manual_review/settings/workflow/{workflow_id}/"
784785
)
785-
except PlatformAPIError:
786-
return None
786+
except PlatformAPIError as e:
787+
if e.status_code == 500:
788+
return None
789+
raise
787790
return (body or {}).get("data")
788791

789792
def create_review_settings(self, payload: dict[str, Any]) -> dict[str, Any]:

src/unstract/clone/phases/agentic_studio.py

Lines changed: 47 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -251,6 +251,22 @@ def _clone_prompt_versions(
251251
result.errors.append(f"agentic {name} list prompt-versions: {e}")
252252
return
253253

254+
# Adopt versions already on target (keyed by version number) so a
255+
# re-run against the same pair doesn't re-create duplicates.
256+
try:
257+
tgt_versions = self.ctx.target.list_agentic_prompt_versions(
258+
project_id=tgt_project_id
259+
)
260+
except Exception as e:
261+
logger.warning(
262+
"agentic '%s': target prompt-version listing failed "
263+
"(re-run may duplicate): %s",
264+
name,
265+
e,
266+
)
267+
tgt_versions = []
268+
tgt_by_version = {v.get("version"): v for v in tgt_versions}
269+
254270
# parent_version is a self-FK: clone roots (no parent) first so a child's
255271
# parent already resolves. Sort by version ascending as a stable order.
256272
ordered = sorted(
@@ -259,18 +275,28 @@ def _clone_prompt_versions(
259275
)
260276
for src in ordered:
261277
self._clone_one_prompt_version(
262-
name, src, tgt_project_id, result, lock
278+
name, src, tgt_project_id, tgt_by_version, result, lock
263279
)
264280

265281
def _clone_one_prompt_version(
266282
self,
267283
name: str,
268284
src: dict[str, Any],
269285
tgt_project_id: str,
286+
tgt_by_version: dict[Any, dict[str, Any]],
270287
result: PhaseResult,
271288
lock: threading.Lock,
272289
) -> None:
273290
src_vid = src["id"]
291+
292+
existing = tgt_by_version.get(src.get("version"))
293+
if existing is not None:
294+
with lock:
295+
result.adopted += 1
296+
self.ctx.remap.record(
297+
"agentic_prompt_version", src_vid, existing["id"]
298+
)
299+
return
274300
payload = {
275301
k: v
276302
for k, v in src.items()
@@ -349,7 +375,27 @@ def _clone_schemas(
349375
result.errors.append(f"agentic {name} list schemas: {e}")
350376
return
351377

378+
# Adopt schemas already on target (keyed by version) so a re-run
379+
# doesn't re-create duplicates.
380+
try:
381+
tgt_schemas = self.ctx.target.list_agentic_schemas(
382+
project_id=tgt_project_id
383+
)
384+
except Exception as e:
385+
logger.warning(
386+
"agentic '%s': target schema listing failed (re-run may "
387+
"duplicate): %s",
388+
name,
389+
e,
390+
)
391+
tgt_schemas = []
392+
existing_versions = {s.get("version") for s in tgt_schemas}
393+
352394
for src in src_schemas:
395+
if src.get("version") in existing_versions:
396+
with lock:
397+
result.adopted += 1
398+
continue
353399
payload = {
354400
k: v
355401
for k, v in src.items()

src/unstract/clone/phases/lookups.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -403,6 +403,9 @@ def _replay_one_version(
403403
result.errors.append(
404404
f"lookup {name} version {version_name} template: {e}"
405405
)
406+
# Don't publish — a stale draft would freeze wrong content
407+
# into the named version.
408+
return
406409
tgt_adapters = self._remap_adapters(
407410
name, detail.get("adapters") or {}, result, lock
408411
)
@@ -423,6 +426,8 @@ def _replay_one_version(
423426
result.errors.append(
424427
f"lookup {name} version {version_name} adapters: {e}"
425428
)
429+
# Don't publish a version with unmapped adapters.
430+
return
426431

427432
self._replay_version_files(
428433
name, src_lookup_id, tgt_lookup_id, src_version_id, detail, result, lock

tests/clone/test_agentic_studio_phase.py

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -299,6 +299,58 @@ def test_schema_clone_bound_to_target_project():
299299
assert schema["json_schema"] == '{"type":"object"}'
300300

301301

302+
def test_rerun_adopts_existing_prompt_versions_and_schemas():
303+
# Re-run against a pair whose project + children already exist on target
304+
# must adopt them (no duplicate child creates).
305+
src = FakeClient(
306+
projects=[_src_project("src-p", "Receipts")],
307+
versions={
308+
"src-p": [
309+
{
310+
"id": "src-v1",
311+
"project": "src-p",
312+
"version": 1,
313+
"prompt_text": "v1",
314+
"parent_version": None,
315+
}
316+
]
317+
},
318+
schemas={
319+
"src-p": [
320+
{
321+
"id": "src-s1",
322+
"project": "src-p",
323+
"json_schema": '{"type":"object"}',
324+
"version": 1,
325+
}
326+
]
327+
},
328+
)
329+
tgt = FakeClient(
330+
projects=[{"id": "tgt-existing", "name": "Receipts"}],
331+
versions={
332+
"tgt-existing": [
333+
{"id": "tgt-v1", "project": "tgt-existing", "version": 1}
334+
]
335+
},
336+
schemas={
337+
"tgt-existing": [
338+
{"id": "tgt-s1", "project": "tgt-existing", "version": 1}
339+
]
340+
},
341+
)
342+
ctx = _ctx(src, tgt, on_name_conflict="adopt")
343+
344+
result = AgenticStudioPhase(ctx).run(CloneReport())
345+
346+
# No duplicate children created on the re-run.
347+
assert tgt.created_versions == []
348+
assert tgt.created_schemas == []
349+
assert result.created == 0
350+
# Existing version adopted + remap recorded so child parents still resolve.
351+
assert ctx.remap.resolve("agentic_prompt_version", "src-v1") == "tgt-v1"
352+
353+
302354
def test_registry_republished_and_remapped():
303355
src = FakeClient(
304356
projects=[_src_project("src-p", "Receipts")],

tests/clone/test_client.py

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,3 +143,17 @@ def test_options_response_with_null_body_still_yields_empty_schema():
143143
# Some deployments return 200 with no body on OPTIONS.
144144
client, _ = _client_with_mock(payload=None, text="")
145145
assert client.get_post_schema("pipeline/") == frozenset()
146+
147+
148+
def test_get_review_settings_500_treated_as_absent():
149+
# Backend raises DoesNotExist (-> 500) when no HITLSettings row exists.
150+
client, _ = _client_with_mock(status=500, text="DoesNotExist")
151+
assert client.get_review_settings("wf-1") is None
152+
153+
154+
def test_get_review_settings_reraises_non_500():
155+
# Auth / rate-limit errors must surface, not masquerade as "no settings".
156+
client, _ = _client_with_mock(status=403, text="forbidden")
157+
with pytest.raises(PlatformAPIError) as exc_info:
158+
client.get_review_settings("wf-1")
159+
assert exc_info.value.status_code == 403

tests/clone/test_lookups_phase.py

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -460,6 +460,50 @@ def test_published_version_replayed_publishes_in_order_and_records_remap():
460460
assert remap.resolve("lookup_version", "src-v2") is not None
461461

462462

463+
def test_staging_template_failure_skips_publish():
464+
# If staging a version's template onto the draft fails, the version must
465+
# NOT be published — else stale content freezes into a named version.
466+
src = FakeClient(
467+
lookups=[_src_lookup("src-lk", "Vendors")],
468+
details={"src-lk": _src_detail("Current draft", llm="src-llm")},
469+
versions={
470+
"src-lk": [
471+
{
472+
"version_id": "src-v1",
473+
"is_draft": False,
474+
"version_name": "v1",
475+
"version_number": 1,
476+
},
477+
]
478+
},
479+
version_details={
480+
"src-v1": {
481+
"version_id": "src-v1",
482+
"is_draft": False,
483+
"version_name": "v1",
484+
"version_number": 1,
485+
"prompt_template": "Frozen v1",
486+
"adapters": {"llm": "src-llm", "x2text": None},
487+
"files": [],
488+
},
489+
},
490+
)
491+
tgt = FakeClient()
492+
493+
def _boom(lookup_id, prompt_template):
494+
raise RuntimeError("template too long")
495+
496+
tgt.update_lookup_draft_template = _boom
497+
remap = RemapTable()
498+
remap.record("adapter", "src-llm", "tgt-llm")
499+
ctx = _ctx(src, tgt, remap=remap)
500+
501+
result = LookupsPhase(ctx).run(CloneReport())
502+
503+
assert tgt.published_versions == [] # staging failed -> no publish
504+
assert result.failed >= 1
505+
506+
463507
def test_published_version_adopted_on_rerun_no_republish():
464508
# Target already has a same-name definition with the same published version
465509
# names: re-run must adopt them (no re-publish) and still record remaps.

0 commit comments

Comments
 (0)