Финальное ревью: пробельные поля ensure_fields, версия агента, чтение файлов, форма issues (#129, I-1, I-2, I-3, M-2, T2.1)

I-1 (R-8): ensure_fields с полем из одних пробелов даёт 422, очистка только "" или null.
I-2: agent_view не падает на нецифровой версии агента (GET /agent, /agents).
I-3 (R-9): files.size через fstat у открытого файла, _intact ловит OSError, put переписывает нечитаемый файл.
M-2: /agent/state отвергает пустые не-списки в issues.
T2.1, T3.1, T4.1-T4.3, T5.2, T6.1, T1.5: тесты и мелкие правки.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Andrey Epifancev 2026-10-26 03:16:39 +04:00
parent 98faabad67
commit e20977ef49
12 changed files with 86 additions and 18 deletions

View File

@ -2,8 +2,8 @@
команда модуля (bug-and-attachments, hub-api «Хранилище файлов вложений»). Модуль не знает HTTP и окружения: команда модуля (bug-and-attachments, hub-api «Хранилище файлов вложений»). Модуль не знает HTTP и окружения:
каталог передаётся параметром (Hub.files_dir, решение 10).""" каталог передаётся параметром (Hub.files_dir, решение 10)."""
import hashlib import hashlib
import os
import logging import logging
import os
import re import re
import shutil import shutil
import stat as stat_mod import stat as stat_mod
@ -50,7 +50,8 @@ def size(root, sha) -> int | None:
return None return None
target = path(root, sha) target = path(root, sha)
try: try:
st = target.stat() with open(target, "rb") as f: # fstat у дескриптора: Path.stat() не требует права на чтение (R-9)
st = os.fstat(f.fileno())
except FileNotFoundError: except FileNotFoundError:
return None return None
except OSError as e: except OSError as e:
@ -77,7 +78,7 @@ def _intact(target, sha) -> bool:
"""Копия на диске совпадает по sha256: испорченную того же размера повторная загрузка переписывает (M-2 ревью).""" """Копия на диске совпадает по sha256: испорченную того же размера повторная загрузка переписывает (M-2 ревью)."""
try: try:
return hashlib.sha256(target.read_bytes()).hexdigest() == sha return hashlib.sha256(target.read_bytes()).hexdigest() == sha
except FileNotFoundError: except OSError: # нет файла, нечитаем или каталог — не целая копия, put перепишет
return False return False

View File

@ -10,6 +10,7 @@ from datetime import timedelta
from . import backup as _backup # noqa: F401 (имя модуля занято параметром backup в claim) from . import backup as _backup # noqa: F401 (имя модуля занято параметром backup в claim)
from . import db, registry, settings from . import db, registry, settings
from .errors import CommandError from .errors import CommandError
from .http import is_digits
log = logging.getLogger(__name__) log = logging.getLogger(__name__)
@ -427,7 +428,8 @@ def agent_view(conn, agent: str) -> dict:
d["online"] = bool(d["last_seen_at"]) and now - db.parse(d["last_seen_at"]) < timedelta(minutes=ONLINE_MINUTES) d["online"] = bool(d["last_seen_at"]) and now - db.parse(d["last_seen_at"]) < timedelta(minutes=ONLINE_MINUTES)
m = registry.module(agent) m = registry.module(agent)
min_version = int(settings.get(conn, m.agent_min_version_key) or 0) if m and m.agent_min_version_key else 0 min_version = int(settings.get(conn, m.agent_min_version_key) or 0) if m and m.agent_min_version_key else 0
d["warning"] = "обновите агента" if d["agent_version"] and int(d["agent_version"]) < min_version else None known = is_digits(str(d["agent_version"])) if d["agent_version"] else False # «²», «10-dev» — версия неизвестна
d["warning"] = "обновите агента" if known and int(d["agent_version"]) < min_version else None
d["rate_limited_24h"] = conn.execute( d["rate_limited_24h"] = conn.execute(
f"SELECT COALESCE(SUM(rate_limited), 0) AS n FROM command WHERE finished_at >= ? AND type IN ({_in(types)})", f"SELECT COALESCE(SUM(rate_limited), 0) AS n FROM command WHERE finished_at >= ? AND type IN ({_in(types)})",
((now - timedelta(days=1)).isoformat(), *types)).fetchone()["n"] ((now - timedelta(days=1)).isoformat(), *types)).fetchone()["n"]

View File

@ -21,7 +21,8 @@ _OFFSET = re.compile(r"(Z|[+-]\d{2}:?\d{2})$")
def h_agent_state(hub, conn, m, q, body): def h_agent_state(hub, conn, m, q, body):
issues = body.get("issues") or [] issues = body.get("issues")
issues = [] if issues is None else issues # {} "" 0 false — не список: check_issues отвергнет
boards = body.get("boards") boards = body.get("boards")
try: try:
mirror.check_issues(issues) mirror.check_issues(issues)

View File

@ -234,6 +234,11 @@ def validate(commands) -> list[dict]:
bad = [e for e in (_type_error(ctype, n, v) for n, v in params.items()) if e] bad = [e for e in (_type_error(ctype, n, v) for n, v in params.items()) if e]
if bad: if bad:
raise CommandError(f"команда {i} ({ctype}): " + "; ".join(bad)) raise CommandError(f"команда {i} ({ctype}): " + "; ".join(bad))
if ctype == "ensure_fields": # « » стало бы «» = «очистить поле»; очистка — только явная (R-8)
for name in FIELD_PARAMS:
v = params.get(name)
if name not in TEXT_PARAMS and isinstance(v, str) and v and not v.strip():
raise CommandError(f'команда {i} ({ctype}): {name} — из одних пробелов; для очистки — "" или null')
params = normalize(ctype, params) params = normalize(ctype, params)
missing = [f for f in spec["required"] if params.get(f) in (None, "", [])] missing = [f for f in spec["required"] if params.get(f) in (None, "", [])]
if missing: if missing:

View File

@ -351,7 +351,8 @@ async def attachment(key: str, attachment: str, wait_seconds: int = 120) -> str:
prefix = "" prefix = ""
if not found: if not found:
want = unicodedata.normalize("NFC", attachment.strip()) want = unicodedata.normalize("NFC", attachment.strip())
found = [a for a in atts if unicodedata.normalize("NFC", a["filename"]) == want] # хаб отдаёт вложения новыми сверху # хаб отдаёт вложения новыми сверху
found = [a for a in atts if unicodedata.normalize("NFC", a["filename"]) == want]
if len(found) > 1: if len(found) > 1:
prefix = f"у {key} {len(found)} вложений «{found[0]['filename']}», взято самое новое #{found[0]['attachment_id']}\n" prefix = f"у {key} {len(found)} вложений «{found[0]['filename']}», взято самое новое #{found[0]['attachment_id']}\n"
if not found: if not found:

View File

@ -32,6 +32,7 @@ def check_comments(issues) -> None:
if c.get(f) is not None and not isinstance(c[f], str): if c.get(f) is not None and not isinstance(c[f], str):
raise ValueError(f"{where}.{f} — строка или null") raise ValueError(f"{where}.{f} — строка или null")
def check_issues(issues) -> None: def check_issues(issues) -> None:
"""Форма списка задач состояния: список объектов с непустым строковым key. Нарушение — ValueError.""" """Форма списка задач состояния: список объектов с непустым строковым key. Нарушение — ValueError."""
if not isinstance(issues, list): if not isinstance(issues, list):

View File

@ -1209,6 +1209,8 @@ def test_batches_post_fetch_attachment_id_not_ascii_digits_422(hub, bad):
hub["client"].post("/batches", {"title": "x", "commands": [ hub["client"].post("/batches", {"title": "x", "commands": [
{"type": "fetch_attachment", "params": {"key": "A-1", "attachment_id": bad}}]}) {"type": "fetch_attachment", "params": {"key": "A-1", "attachment_id": bad}}]})
assert e.value.status == 422 assert e.value.status == 422
want = "attachment_id — строка" if bad == 7 else "attachment_id — цифры id вложения из issue"
assert want in e.value.detail
assert hub["client"].get("/batches")["batches"] == [] assert hub["client"].get("/batches")["batches"] == []
@ -1326,3 +1328,24 @@ def test_agent_state_bad_issue_shape_422(hub, issues, detail):
with pytest.raises(HubError) as e: with pytest.raises(HubError) as e:
hub["agent"].post("/agent/state", {"issues": issues, "links": []}) hub["agent"].post("/agent/state", {"issues": issues, "links": []})
assert e.value.status == 422 and e.value.error == "bad_state" and detail in e.value.detail assert e.value.status == 422 and e.value.error == "bad_state" and detail in e.value.detail
@pytest.mark.parametrize("bad", [{}, "", 0, False])
def test_agent_state_falsy_non_list_issues_422(hub, bad):
"""M-2: пустые не-списки не маскируются под «нет задач»."""
with pytest.raises(HubError) as e:
hub["agent"].post("/agent/state", {"issues": bad, "links": []})
assert e.value.status == 422 and "issues — список задач" in e.value.detail
@pytest.mark.parametrize("body", [{"links": []}, {"issues": None, "links": []}])
def test_agent_state_missing_or_null_issues_ok(hub, body):
hub["agent"].post("/agent/state", body)
@pytest.mark.parametrize("version", ["²", "10-dev"])
def test_agent_endpoints_survive_non_digit_agent_version(hub, version):
"""I-2: GET /agent и /agents отвечают 200 при нецифровой версии агента."""
hub["agent"].post("/agent/claim", {"agent_version": version, "settings_version": 1})
assert hub["client"].get("/agent")["agent_version"] == version
assert any(a["agent_version"] == version for a in hub["client"].get("/agents")["agents"])

View File

@ -385,3 +385,23 @@ def test_is_digits_ascii_only():
assert is_digits("123") is True assert is_digits("123") is True
for bad in ("", "²", "١٢", "12a", 7, None): for bad in ("", "²", "١٢", "12a", 7, None):
assert is_digits(bad) is False assert is_digits(bad) is False
@pytest.mark.parametrize("field", ["assignee", "epic_key", "priority", "duedate", "planned_start"])
def test_ensure_fields_whitespace_only_rejected(field):
"""I-1 (R-8): « » после обрезки стало бы «» («очистить поле») — отвергается."""
with pytest.raises(CommandError) as e:
commands.validate([{"type": "ensure_fields", "params": {"key": "A-1", field: " "}}])
assert f'{field} — из одних пробелов; для очистки — "" или null' in str(e.value)
assert "команда 1 (ensure_fields)" in str(e.value)
@pytest.mark.parametrize("value", ["", None])
def test_ensure_fields_explicit_clear_accepted(value):
cmds = commands.validate([{"type": "ensure_fields", "params": {"key": "A-1", "assignee": value}}])
assert cmds[0]["params"]["assignee"] == value
def test_rank_before_after_whitespace_still_means_not_given():
cmds = commands.validate([{"type": "ensure_rank", "params": {"key": "A-1", "before": "A-2", "after": " "}}])
assert cmds[0]["params"]["after"] == ""

View File

@ -191,17 +191,21 @@ def test_sweep_downloads(tmp_path):
assert files.sweep_downloads(tmp_path / "нет") == [] assert files.sweep_downloads(tmp_path / "нет") == []
@pytest.mark.parametrize("func,attr", [(files.size, "stat"), (files.read, "read_bytes")]) @pytest.mark.skipif(os.geteuid() == 0, reason="root читает любой файл")
def test_unreadable_file_is_none_with_warning(root, monkeypatch, caplog, func, attr): def test_unreadable_file_is_none_with_warning_and_put_rewrites(root, caplog):
from pathlib import Path """I-3 (R-9): настоящий chmod 0 — size и read дают None с предупреждением, put пишет файл заново."""
sha = files.put(root, b"abc")[0] sha = files.put(root, b"abc")[0]
target = root / sha
def denied(self, *args, **kwargs): os.chmod(target, 0)
raise PermissionError(13, "Permission denied") try:
monkeypatch.setattr(Path, attr, denied) with caplog.at_level("WARNING"):
with caplog.at_level("WARNING"): assert files.size(root, sha) is None
assert func(root, sha) is None assert files.read(root, sha) is None
assert sha in caplog.text and "не читается" in caplog.text assert str(target) in caplog.text and "не читается" in caplog.text
assert files.put(root, b"abc") == (sha, 3)
assert target.read_bytes() == b"abc"
finally:
os.chmod(target, 0o600)
@pytest.mark.parametrize("func", [files.size, files.read]) @pytest.mark.parametrize("func", [files.size, files.read])
@ -209,6 +213,7 @@ def test_directory_instead_of_file_is_none(root, caplog, func):
(root / ABC).mkdir(parents=True) (root / ABC).mkdir(parents=True)
with caplog.at_level("WARNING"): with caplog.at_level("WARNING"):
assert func(root, ABC) is None assert func(root, ABC) is None
assert str(root / ABC) in caplog.text and "не читается" in caplog.text
@pytest.mark.parametrize("func", [files.size, files.read]) @pytest.mark.parametrize("func", [files.size, files.read])

View File

@ -1373,3 +1373,11 @@ def test_retry_sprint_meta_gets_jql_tail(conn, add_project):
cmds = queue.batch_view(conn, r["id"])["commands"] cmds = queue.batch_view(conn, r["id"])["commands"]
assert [c["type"] for c in cmds] == ["ensure_sprint_meta", "refresh"] assert [c["type"] for c in cmds] == ["ensure_sprint_meta", "refresh"]
assert cmds[-1]["params"] == {"jql": 'project = AURORA AND labels in ("AWPCA")'} assert cmds[-1]["params"] == {"jql": 'project = AURORA AND labels in ("AWPCA")'}
@pytest.mark.parametrize("version", ["²", "10-dev", "١٢"])
def test_agent_view_non_digit_version_is_unknown(conn, version):
"""I-2: нецифровая версия агента — неизвестная: agent_view не бросает ValueError и не предупреждает."""
queue.claim(conn, "jira", agent_version=version, settings_version=1)
v = queue.agent_view(conn, "jira")
assert v["agent_version"] == version and v["warning"] is None

View File

@ -335,7 +335,8 @@ def test_recover_partial_agent_keeps_good_prev(tmp_path, monkeypatch):
release.publish(rd, agent_file(tmp_path, 6, name="new.py")) release.publish(rd, agent_file(tmp_path, 6, name="new.py"))
monkeypatch.undo() monkeypatch.undo()
assert (rd / "agent.py").read_bytes() == v5 assert (rd / "agent.py").read_bytes() == v5
assert not (rd / "agent.py.prev").exists() or (rd / "agent.py.prev").read_bytes() == v5 assert not (rd / "agent.py.prev").exists() # _undo вернул .prev на место agent.py
assert sorted(p.name for p in rd.iterdir()) == ["agent.py", "manifest.json"]
assert release.load(rd)["version"] == 5 assert release.load(rd)["version"] == 5

View File

@ -174,7 +174,7 @@ def test_v3_keys_removed_on_hub_start_api(tmp_path):
try: try:
client = HubClient(f"http://127.0.0.1:{srv.server_port}", token) client = HubClient(f"http://127.0.0.1:{srv.server_port}", token)
got = client.get("/settings") got = client.get("/settings")
assert "board_id" not in got and got["base_jql"] != "project = AURORA" # base_jql вычисляется (без проектов — None) assert "board_id" not in got and got["base_jql"] is None # base_jql вычисляется (без проектов — None)
for key in ("board_id", "base_jql"): for key in ("board_id", "base_jql"):
with pytest.raises(HubError) as e: with pytest.raises(HubError) as e:
client.put("/settings", {key: 1}) client.put("/settings", {key: 1})