fix(store): rescan promotes non-current submission when guardrails off (Codex follow-up to #330) (#331)
* fix(store): rescan promotes non-current submission when guardrails off Codex adversarial-review follow-up on PR #330: admin rescan with `guardrails.enabled: false` flipped submission status to `approved` and entity visibility to `approved` but never called `promote_to_version`. A rescan that re-approved a non-current v2+ left the entity stuck at the prior version even though the operator's intent in clicking rescan was to publish the rescanned bytes. Mirrors the inline-promote pattern in create / update / restore. The guardrails-on path is unchanged — it schedules an LLM review and promotion lands via `runner.run_llm_review` on approval. Adds tests for the byte-identical edge cases Codex flagged as under-covered by PR #330: - TestPromoteLookupByByteIdenticalBundles::test_byte_identical_v3_after_different_v2 - TestOverrideForwardOnly::test_override_byte_identical_v2_blocked_promotes_correctly - TestRescanPromotesNonCurrent::test_rescan_promotes_non_current_v2_when_guardrails_disabled * release: 0.54.23 — rescan promotes non-current submission when guardrails off (Codex follow-up to #330) --------- Co-authored-by: ZdenekSrotyr <zdenek.srotyr@keboola.com>
This commit is contained in:
parent
78cd243e65
commit
9eaa1dc53c
5 changed files with 317 additions and 1 deletions
13
CHANGELOG.md
13
CHANGELOG.md
|
|
@ -10,6 +10,19 @@ CalVer image tags (`stable-YYYY.MM.N`, `dev-YYYY.MM.N`) are produced for every C
|
||||||
|
|
||||||
## [Unreleased]
|
## [Unreleased]
|
||||||
|
|
||||||
|
## [0.54.23] — 2026-05-16
|
||||||
|
|
||||||
|
### Fixed
|
||||||
|
- Flea-market admin **Rescan** of a non-current v2+ submission with
|
||||||
|
`guardrails.enabled: false` now promotes the entity forward
|
||||||
|
(mirrors the inline-promote in create / update / restore). Pre-fix
|
||||||
|
the branch flipped submission status to `approved` and entity
|
||||||
|
visibility to `approved` but never called `promote_to_version` —
|
||||||
|
the rescan re-approved the version without making it current.
|
||||||
|
Codex adversarial-review follow-up on PR #330. The guardrails-on
|
||||||
|
path is unchanged (rescan schedules an LLM review; promotion lands
|
||||||
|
when the verdict approves through `runner.run_llm_review`).
|
||||||
|
|
||||||
## [0.54.22] — 2026-05-15
|
## [0.54.22] — 2026-05-15
|
||||||
|
|
||||||
### Fixed
|
### Fixed
|
||||||
|
|
|
||||||
|
|
@ -3924,6 +3924,17 @@ async def admin_rescan_store_submission(
|
||||||
ents.set_visibility(entity_id, "pending")
|
ents.set_visibility(entity_id, "pending")
|
||||||
else:
|
else:
|
||||||
ents.set_visibility(entity_id, "approved")
|
ents.set_visibility(entity_id, "approved")
|
||||||
|
# Guardrails explicitly disabled — immediately live. Promote
|
||||||
|
# the rescanned submission's version forward (same atomic
|
||||||
|
# helper the create / update / restore inline-promote paths
|
||||||
|
# use). Pre-fix this branch flipped visibility but never
|
||||||
|
# called promote_to_version, so a rescan that re-approved a
|
||||||
|
# non-current v2+ left the entity stuck at the prior version.
|
||||||
|
# Surfaced by adversarial review of PR #330.
|
||||||
|
from app.api.store import promote_to_version
|
||||||
|
entity_row = ents.get(entity_id) or {}
|
||||||
|
if target_n is not None and target_n > int(entity_row.get("version_no") or 0):
|
||||||
|
promote_to_version(entity_id, target_n, ents)
|
||||||
# v46: attribution lookup is live — no explicit refresh needed.
|
# v46: attribution lookup is live — no explicit refresh needed.
|
||||||
AuditRepository(conn).log(
|
AuditRepository(conn).log(
|
||||||
user_id=user["id"],
|
user_id=user["id"],
|
||||||
|
|
|
||||||
|
|
@ -1,6 +1,6 @@
|
||||||
[project]
|
[project]
|
||||||
name = "agnes-the-ai-analyst"
|
name = "agnes-the-ai-analyst"
|
||||||
version = "0.54.22"
|
version = "0.54.23"
|
||||||
description = "Agnes — AI Data Analyst platform for AI analytical systems"
|
description = "Agnes — AI Data Analyst platform for AI analytical systems"
|
||||||
requires-python = ">=3.11,<3.14"
|
requires-python = ">=3.11,<3.14"
|
||||||
license = "MIT"
|
license = "MIT"
|
||||||
|
|
|
||||||
|
|
@ -1303,6 +1303,112 @@ class TestOverrideForwardOnly:
|
||||||
# row itself is preserved; only the on-disk roll-back is gated).
|
# row itself is preserved; only the on-disk roll-back is gated).
|
||||||
assert v2_sub_after["status"] == "overridden"
|
assert v2_sub_after["status"] == "overridden"
|
||||||
|
|
||||||
|
def test_override_byte_identical_v2_blocked_promotes_correctly(
|
||||||
|
self, web_client, monkeypatch,
|
||||||
|
):
|
||||||
|
"""Codex adversarial-review follow-up on PR #330: confirm
|
||||||
|
override's submission_id lookup resolves v2 correctly when
|
||||||
|
its hash collides with v1's. Pre-PR-330 the override loop
|
||||||
|
did hash-match-first-wins → stuck on v1's n=1; forward-only
|
||||||
|
`1 > 1` skipped promote."""
|
||||||
|
from pathlib import Path
|
||||||
|
from app.utils import get_store_dir
|
||||||
|
from src.repositories.store_entities import StoreEntitiesRepository
|
||||||
|
from src.repositories.store_submissions import StoreSubmissionsRepository
|
||||||
|
from src.store_guardrails.runner import run_llm_review
|
||||||
|
import io as _io
|
||||||
|
import zipfile as _zip
|
||||||
|
|
||||||
|
monkeypatch.setenv("ANTHROPIC_API_KEY", "sk-ant-fake-for-override-identical")
|
||||||
|
user_id, user_cookies = _create_user(web_client, "override-id@x.com")
|
||||||
|
|
||||||
|
identical_body = (
|
||||||
|
"Identical body content long enough to clear the inline "
|
||||||
|
"content-quality threshold for skill bodies. " * 4
|
||||||
|
)
|
||||||
|
|
||||||
|
def _identical_zip():
|
||||||
|
buf = _io.BytesIO()
|
||||||
|
with _zip.ZipFile(buf, "w") as zf:
|
||||||
|
zf.writestr(
|
||||||
|
"overrideid/SKILL.md",
|
||||||
|
"---\nname: overrideid\ndescription: "
|
||||||
|
"Use when verifying override resolves v2 by "
|
||||||
|
"submission_id even when v2 hash matches v1 hash\n---\n\n"
|
||||||
|
+ identical_body,
|
||||||
|
)
|
||||||
|
return buf.getvalue()
|
||||||
|
|
||||||
|
r = web_client.post(
|
||||||
|
"/api/store/entities",
|
||||||
|
files={"file": ("s.zip", _identical_zip(), "application/zip")},
|
||||||
|
data={"type": "skill",
|
||||||
|
"description": (
|
||||||
|
"Use when verifying override resolves v2 by "
|
||||||
|
"submission_id even when v2 hash matches v1 hash"
|
||||||
|
)},
|
||||||
|
cookies=user_cookies,
|
||||||
|
)
|
||||||
|
assert r.status_code == 201, r.text
|
||||||
|
eid = r.json()["id"]
|
||||||
|
|
||||||
|
monkeypatch.setattr(
|
||||||
|
"app.api.store.get_guardrails_enabled", lambda: True,
|
||||||
|
)
|
||||||
|
monkeypatch.setattr(
|
||||||
|
"app.api.store.get_guardrails_llm_provider_ready", lambda: True,
|
||||||
|
)
|
||||||
|
|
||||||
|
def mock_block(*a, **kw):
|
||||||
|
return {
|
||||||
|
"risk_level": "high", "summary": "mock block",
|
||||||
|
"findings": [{"severity": "high", "category": "test",
|
||||||
|
"file": "x", "explanation": "mock"}],
|
||||||
|
"template_placeholders_found": 0,
|
||||||
|
"reviewed_by_model": "mock", "error": None,
|
||||||
|
}
|
||||||
|
monkeypatch.setattr(
|
||||||
|
"src.store_guardrails.llm_review.review_bundle", mock_block,
|
||||||
|
)
|
||||||
|
r = web_client.put(
|
||||||
|
f"/api/store/entities/{eid}",
|
||||||
|
files={"file": ("v2.zip", _identical_zip(), "application/zip")},
|
||||||
|
cookies=user_cookies,
|
||||||
|
)
|
||||||
|
assert r.status_code == 200, r.text
|
||||||
|
conn = get_system_db()
|
||||||
|
v2_sub_id = StoreSubmissionsRepository(conn).latest_for_entity(eid)["id"]
|
||||||
|
conn.close()
|
||||||
|
run_llm_review(
|
||||||
|
v2_sub_id,
|
||||||
|
plugin_dir=Path(get_store_dir()) / eid / "versions" / "v2" / "plugin",
|
||||||
|
conn_factory=get_system_db,
|
||||||
|
api_key_loader=lambda: "sk", model_loader=lambda: "mock",
|
||||||
|
)
|
||||||
|
|
||||||
|
conn = get_system_db()
|
||||||
|
ent_before = StoreEntitiesRepository(conn).get(eid)
|
||||||
|
conn.close()
|
||||||
|
assert ent_before["version_no"] == 1
|
||||||
|
v1_hash = ent_before["version"]
|
||||||
|
|
||||||
|
_, admin_cookies = _create_admin(web_client)
|
||||||
|
r = web_client.post(
|
||||||
|
f"/api/admin/store/submissions/{v2_sub_id}/override",
|
||||||
|
json={"reason": "false positive — cleared in offline review"},
|
||||||
|
cookies=admin_cookies,
|
||||||
|
)
|
||||||
|
assert r.status_code == 200, r.text
|
||||||
|
conn = get_system_db()
|
||||||
|
ent_after = StoreEntitiesRepository(conn).get(eid)
|
||||||
|
conn.close()
|
||||||
|
assert ent_after["version_no"] == 2, (
|
||||||
|
f"override must promote to v2 even when v2 hash matches v1's; "
|
||||||
|
f"got version_no={ent_after['version_no']}"
|
||||||
|
)
|
||||||
|
# Hash unchanged (identical bundle), but version_no DID move.
|
||||||
|
assert ent_after["version"] == v1_hash
|
||||||
|
|
||||||
|
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
# v30: Download bundle, Sort by size, Quota
|
# v30: Download bundle, Sort by size, Quota
|
||||||
|
|
|
||||||
|
|
@ -1806,3 +1806,189 @@ class TestPromoteLookupByByteIdenticalBundles:
|
||||||
assert ent_after["version"] == v1_hash, (
|
assert ent_after["version"] == v1_hash, (
|
||||||
"hash unchanged (bundle is byte-identical) but version_no DID move"
|
"hash unchanged (bundle is byte-identical) but version_no DID move"
|
||||||
)
|
)
|
||||||
|
|
||||||
|
def test_byte_identical_v3_after_different_v2(
|
||||||
|
self, web_client, monkeypatch,
|
||||||
|
):
|
||||||
|
"""v1 + v2 (different hash) + v3 byte-identical to v1.
|
||||||
|
Lookup must resolve v3 to n=3, not v1 (same hash) or v2 (the
|
||||||
|
most-recent approved). With current=2 and target=3 the
|
||||||
|
forward-only guard fires correctly only if target_n=3."""
|
||||||
|
from pathlib import Path
|
||||||
|
from app.utils import get_store_dir
|
||||||
|
from src.repositories.store_submissions import StoreSubmissionsRepository
|
||||||
|
from src.store_guardrails.runner import run_llm_review
|
||||||
|
|
||||||
|
monkeypatch.setenv("ANTHROPIC_API_KEY", "sk-ant-fake-for-v3-test")
|
||||||
|
owner_id, owner_cookies = _create_user(web_client, "v3hash@x.com")
|
||||||
|
|
||||||
|
body_a = (
|
||||||
|
"Body A line that is intentionally long enough to clear the "
|
||||||
|
"content threshold for skill bodies. " * 4
|
||||||
|
)
|
||||||
|
body_b = (
|
||||||
|
"Body B line that is intentionally DIFFERENT and also long "
|
||||||
|
"enough to clear the content threshold for skill bodies. " * 4
|
||||||
|
)
|
||||||
|
|
||||||
|
r = web_client.post(
|
||||||
|
"/api/store/entities",
|
||||||
|
files={"file": ("v1.zip", _make_skill_zip("v3hash", body=body_a),
|
||||||
|
"application/zip")},
|
||||||
|
data={"type": "skill", "description": _OK_DESC},
|
||||||
|
cookies=owner_cookies,
|
||||||
|
)
|
||||||
|
assert r.status_code == 201
|
||||||
|
eid = r.json()["id"]
|
||||||
|
|
||||||
|
monkeypatch.setattr(
|
||||||
|
"app.api.store.get_guardrails_enabled", lambda: True,
|
||||||
|
)
|
||||||
|
monkeypatch.setattr(
|
||||||
|
"app.api.store.get_guardrails_llm_provider_ready", lambda: True,
|
||||||
|
)
|
||||||
|
|
||||||
|
def mock_approve(*a, **kw):
|
||||||
|
return {
|
||||||
|
"risk_level": "safe", "summary": "ok",
|
||||||
|
"findings": [], "template_placeholders_found": 0,
|
||||||
|
"reviewed_by_model": "mock", "error": None,
|
||||||
|
"content_quality": {"verdict": "pass", "issues": []},
|
||||||
|
}
|
||||||
|
monkeypatch.setattr(
|
||||||
|
"src.store_guardrails.llm_review.review_bundle", mock_approve,
|
||||||
|
)
|
||||||
|
|
||||||
|
v2_zip = _make_skill_zip("v3hash", body=body_b)
|
||||||
|
r = web_client.put(
|
||||||
|
f"/api/store/entities/{eid}",
|
||||||
|
files={"file": ("v2.zip", v2_zip, "application/zip")},
|
||||||
|
cookies=owner_cookies,
|
||||||
|
)
|
||||||
|
assert r.status_code == 200
|
||||||
|
conn = get_system_db()
|
||||||
|
v2_sub_id = StoreSubmissionsRepository(conn).latest_for_entity(eid)["id"]
|
||||||
|
conn.close()
|
||||||
|
run_llm_review(
|
||||||
|
v2_sub_id,
|
||||||
|
plugin_dir=Path(get_store_dir()) / eid / "versions" / "v2" / "plugin",
|
||||||
|
conn_factory=get_system_db,
|
||||||
|
api_key_loader=lambda: "sk", model_loader=lambda: "mock",
|
||||||
|
)
|
||||||
|
conn = get_system_db()
|
||||||
|
ent_at_v2 = StoreEntitiesRepository(conn).get(eid)
|
||||||
|
conn.close()
|
||||||
|
assert ent_at_v2["version_no"] == 2
|
||||||
|
|
||||||
|
v3_zip = _make_skill_zip("v3hash", body=body_a)
|
||||||
|
r = web_client.put(
|
||||||
|
f"/api/store/entities/{eid}",
|
||||||
|
files={"file": ("v3.zip", v3_zip, "application/zip")},
|
||||||
|
cookies=owner_cookies,
|
||||||
|
)
|
||||||
|
assert r.status_code == 200
|
||||||
|
conn = get_system_db()
|
||||||
|
v3_sub_id = StoreSubmissionsRepository(conn).latest_for_entity(eid)["id"]
|
||||||
|
conn.close()
|
||||||
|
run_llm_review(
|
||||||
|
v3_sub_id,
|
||||||
|
plugin_dir=Path(get_store_dir()) / eid / "versions" / "v3" / "plugin",
|
||||||
|
conn_factory=get_system_db,
|
||||||
|
api_key_loader=lambda: "sk", model_loader=lambda: "mock",
|
||||||
|
)
|
||||||
|
conn = get_system_db()
|
||||||
|
ent_at_v3 = StoreEntitiesRepository(conn).get(eid)
|
||||||
|
conn.close()
|
||||||
|
assert ent_at_v3["version_no"] == 3, (
|
||||||
|
f"v3 must promote despite hash collision with v1; "
|
||||||
|
f"got version_no={ent_at_v3['version_no']}"
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
class TestRescanPromotesNonCurrent:
|
||||||
|
"""Codex adversarial-review follow-up on PR #330: admin rescan
|
||||||
|
with `guardrails.enabled: false` flipped status='approved' +
|
||||||
|
visibility but never called `promote_to_version`. A rescan that
|
||||||
|
re-approved a non-current v2+ left the entity stuck at the prior
|
||||||
|
version. Fix mirrors the inline-promote in create/update/restore."""
|
||||||
|
|
||||||
|
def test_rescan_promotes_non_current_v2_when_guardrails_disabled(
|
||||||
|
self, web_client, monkeypatch,
|
||||||
|
):
|
||||||
|
from pathlib import Path
|
||||||
|
from app.utils import get_store_dir
|
||||||
|
from src.repositories.store_submissions import StoreSubmissionsRepository
|
||||||
|
from src.store_guardrails.runner import run_llm_review
|
||||||
|
|
||||||
|
owner_id, owner_cookies = _create_user(web_client, "rescanpromote@x.com")
|
||||||
|
eid = _upload_clean(web_client, owner_cookies, name="rescanpromote")
|
||||||
|
|
||||||
|
monkeypatch.setattr(
|
||||||
|
"app.api.store.get_guardrails_enabled", lambda: True,
|
||||||
|
)
|
||||||
|
monkeypatch.setattr(
|
||||||
|
"app.api.store.get_guardrails_llm_provider_ready", lambda: True,
|
||||||
|
)
|
||||||
|
monkeypatch.setenv("ANTHROPIC_API_KEY", "sk-ant-fake-for-rescan-promote")
|
||||||
|
|
||||||
|
def mock_block(*a, **kw):
|
||||||
|
return {
|
||||||
|
"risk_level": "high", "summary": "mock block",
|
||||||
|
"findings": [{"severity": "high", "category": "test",
|
||||||
|
"file": "x", "explanation": "mock"}],
|
||||||
|
"template_placeholders_found": 0,
|
||||||
|
"reviewed_by_model": "mock", "error": None,
|
||||||
|
}
|
||||||
|
monkeypatch.setattr(
|
||||||
|
"src.store_guardrails.llm_review.review_bundle", mock_block,
|
||||||
|
)
|
||||||
|
|
||||||
|
v2 = _make_skill_zip("rescanpromote", body="V2 body content " * 30)
|
||||||
|
r = web_client.put(
|
||||||
|
f"/api/store/entities/{eid}",
|
||||||
|
files={"file": ("v2.zip", v2, "application/zip")},
|
||||||
|
cookies=owner_cookies,
|
||||||
|
)
|
||||||
|
assert r.status_code == 200, r.text
|
||||||
|
conn = get_system_db()
|
||||||
|
v2_sub_id = StoreSubmissionsRepository(conn).latest_for_entity(eid)["id"]
|
||||||
|
conn.close()
|
||||||
|
run_llm_review(
|
||||||
|
v2_sub_id,
|
||||||
|
plugin_dir=Path(get_store_dir()) / eid / "versions" / "v2" / "plugin",
|
||||||
|
conn_factory=get_system_db,
|
||||||
|
api_key_loader=lambda: "sk", model_loader=lambda: "mock",
|
||||||
|
)
|
||||||
|
|
||||||
|
conn = get_system_db()
|
||||||
|
ent_before = StoreEntitiesRepository(conn).get(eid)
|
||||||
|
conn.close()
|
||||||
|
assert ent_before["version_no"] == 1
|
||||||
|
v1_hash = ent_before["version"]
|
||||||
|
|
||||||
|
# Rescan with guardrails OFF — branch under test. Patch both
|
||||||
|
# bound symbols (admin imports function-locally).
|
||||||
|
monkeypatch.setattr(
|
||||||
|
"app.api.store.get_guardrails_enabled", lambda: False,
|
||||||
|
)
|
||||||
|
monkeypatch.setattr(
|
||||||
|
"app.instance_config.get_guardrails_enabled", lambda: False,
|
||||||
|
)
|
||||||
|
_, admin_cookies = _create_admin(web_client)
|
||||||
|
r = web_client.post(
|
||||||
|
f"/api/admin/store/submissions/{v2_sub_id}/rescan",
|
||||||
|
cookies=admin_cookies,
|
||||||
|
)
|
||||||
|
assert r.status_code == 200, r.text
|
||||||
|
assert r.json()["status"] == "approved"
|
||||||
|
|
||||||
|
conn = get_system_db()
|
||||||
|
ent_after = StoreEntitiesRepository(conn).get(eid)
|
||||||
|
conn.close()
|
||||||
|
assert ent_after["version_no"] == 2, (
|
||||||
|
f"rescan-approve of v2 must promote entity to v2 when "
|
||||||
|
f"guardrails are disabled; got version_no={ent_after['version_no']}"
|
||||||
|
)
|
||||||
|
assert ent_after["version"] != v1_hash, (
|
||||||
|
"entity.version hash must move to v2 after rescan promote"
|
||||||
|
)
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue