fix: ответ PUT /settings, «создание задачи отменено», CRLF в подтверждении, реле по resolve() (O-11, O-10, O-9, minor-1, T3.1 #16) (#32)
PUT /settings отвечает всеми настройками (R-3); пропуск при cancelled-создателе — «создание задачи new:N отменено» (R-4); ensure_fields сравнивает строки без различия CRLF/LF; _cleaned по resolve(); _type_error по веткам (решения 9–12, 15). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Andrey Epifancev
parent
8a319ea595
commit
e180bb4e5f
@ -144,7 +144,7 @@ uv run jira-hub agent publish ../jira-agent/src/agent.py
|
||||
| `POST /commands/{id}/retry`, `POST /commands/{id}/cancel` | клиент | повтор (новый пакет) / отмена (`force` для взятой) |
|
||||
| `POST /refresh` | клиент | `{keys}`, `{jql}` или `{full: true}` |
|
||||
| `GET /agent`, `GET /sync-runs` | клиент | состояние агента (с `release`, `update_state`, `launcher_version`, `agent_reported`, `update_hint`), журнал загрузок |
|
||||
| `GET /settings`, `PUT /settings` | клиент | настройки (PUT частично, поднимает `settings_version`) |
|
||||
| `GET /settings`, `PUT /settings` | клиент | настройки (PUT частично, поднимает `settings_version`; ответ — все настройки после записи, как у GET) |
|
||||
|
||||
```bash
|
||||
TOKEN=$(uv run jira-hub tokens | sed -n 's/^клиентский токен: //p')
|
||||
|
||||
@ -306,7 +306,8 @@ def h_settings_get(hub, conn, m, q, body):
|
||||
|
||||
def h_settings_put(hub, conn, m, q, body):
|
||||
"""Записать настройки пакетом (всё или ничего). До записи проверяются защищённые ключи и значения
|
||||
(settings.validate): ошибка — 422, версия настроек не меняется."""
|
||||
(settings.validate): ошибка — 422, версия настроек не меняется. Ответ — все настройки после записи, как у
|
||||
GET /settings (hub-hygiene-d, решение 9)."""
|
||||
bad = [k for k in body if k in settings.PROTECTED]
|
||||
if bad:
|
||||
raise ApiError(422, "protected", ", ".join(bad))
|
||||
@ -315,11 +316,10 @@ def h_settings_put(hub, conn, m, q, body):
|
||||
settings.validate(key, value)
|
||||
except ValueError as e:
|
||||
raise ApiError(422, "bad_setting", str(e))
|
||||
version = settings.get(conn, "settings_version")
|
||||
with db.transaction(conn):
|
||||
for key, value in body.items():
|
||||
version = settings.set_(conn, key, value)
|
||||
return 200, {"settings_version": version}
|
||||
settings.set_(conn, key, value)
|
||||
return 200, settings.get_all(conn)
|
||||
|
||||
|
||||
ROUTES = [
|
||||
|
||||
@ -57,15 +57,16 @@ def _type_error(ctype, name, value) -> str | None:
|
||||
if value is None:
|
||||
return None
|
||||
if name in LIST_PARAMS:
|
||||
ok, want = isinstance(value, list) and all(isinstance(v, str) for v in value), "список строк"
|
||||
elif name in BOOL_PARAMS:
|
||||
ok, want = isinstance(value, bool), "true или false"
|
||||
elif ctype == "ensure_status" and name == "target":
|
||||
ok = isinstance(value, str) or (isinstance(value, list) and all(isinstance(v, str) for v in value))
|
||||
want = "строка или список строк"
|
||||
else:
|
||||
ok, want = isinstance(value, str), "строка"
|
||||
return None if ok else f"{name} — {want}"
|
||||
return None if _is_str_list(value) else f"{name} — список строк"
|
||||
if name in BOOL_PARAMS:
|
||||
return None if isinstance(value, bool) else f"{name} — true или false"
|
||||
if ctype == "ensure_status" and name == "target":
|
||||
return None if isinstance(value, str) or _is_str_list(value) else f"{name} — строка или список строк"
|
||||
return None if isinstance(value, str) else f"{name} — строка"
|
||||
|
||||
|
||||
def _is_str_list(value) -> bool:
|
||||
return isinstance(value, list) and all(isinstance(v, str) for v in value)
|
||||
|
||||
|
||||
def validate(commands) -> list[dict]:
|
||||
@ -133,6 +134,14 @@ def estimate(types, pacing: dict) -> int:
|
||||
return sum(int(pacing.get(t, 0)) for t in types)
|
||||
|
||||
|
||||
def _norm_text(value):
|
||||
"""Значение поля для сравнения с зеркалом: переводы строки CRLF и LF не различаются (описание, правленное в UI Jira,
|
||||
приходит с CRLF, O-9 #16); null и пустая строка равны (решение 11)."""
|
||||
if isinstance(value, str):
|
||||
value = value.replace("\r\n", "\n")
|
||||
return value or None
|
||||
|
||||
|
||||
def check_confirmed(conn, cmd, link_types: dict) -> tuple[bool, dict | None]:
|
||||
"""Виден ли желаемый результат в зеркале. ensure_issue, ensure_comment и refresh по состоянию не подтверждаются:
|
||||
потерянный отчёт ensure_issue закрывается довыдачей — агент находит свою задачу поиском дубля (bridge-reliability,
|
||||
@ -145,7 +154,7 @@ def check_confirmed(conn, cmd, link_types: dict) -> tuple[bool, dict | None]:
|
||||
return False, None
|
||||
if ctype == "ensure_fields":
|
||||
for field in FIELD_PARAMS:
|
||||
if field in p and (issue.get(field) or None) != (p[field] or None):
|
||||
if field in p and _norm_text(issue.get(field)) != _norm_text(p[field]):
|
||||
return False, None
|
||||
if any(l not in issue["labels"] for l in p.get("labels_add", [])):
|
||||
return False, None
|
||||
|
||||
@ -114,6 +114,12 @@ def _definer_status(conn, batch_id, temp_key):
|
||||
return r["status"] if r else "failed"
|
||||
|
||||
|
||||
def _skip_reason(conn, batch_id, unresolved) -> str:
|
||||
"""Текст skipped для зависимой команды: отменённое создание называется отменой (O-10 #16, решение 10)."""
|
||||
cancelled = [k for k in unresolved if _definer_status(conn, batch_id, k) == "cancelled"]
|
||||
return f"создание задачи {cancelled[0]} отменено" if cancelled else f"задача {unresolved[0]} не создана"
|
||||
|
||||
|
||||
def _next(conn) -> dict | None:
|
||||
rows = conn.execute("SELECT c.* FROM command c JOIN batch b ON b.id = c.batch_id "
|
||||
"WHERE c.status = 'queued' AND b.cancelled_at IS NULL ORDER BY b.created_at, b.id, c.seq").fetchall()
|
||||
@ -131,7 +137,7 @@ def _next(conn) -> dict | None:
|
||||
unresolved = [] if cmd["params"]["keys"] else unresolved
|
||||
if unresolved:
|
||||
conn.execute("UPDATE command SET status = 'skipped', finished_at = ?, error = ? WHERE id = ?",
|
||||
(db.now(), f"задача {unresolved[0]} не создана", cmd["id"]))
|
||||
(db.now(), _skip_reason(conn, cmd["batch_id"], unresolved), cmd["id"]))
|
||||
continue
|
||||
return cmd
|
||||
return None
|
||||
|
||||
@ -61,7 +61,7 @@ class Relay:
|
||||
self._cleanup_at = 0.0
|
||||
self._ready = False
|
||||
self._warned: str | None = None
|
||||
self._cleaned: set[Path] = set() # папки, чей work/ вычищен после старта реле
|
||||
self._cleaned: set[Path] = set() # папки (по resolve()), чей work/ вычищен после старта реле
|
||||
self.workers: list[threading.Thread] = []
|
||||
|
||||
# --- цикл ---------------------------------------------------------------------
|
||||
@ -124,12 +124,13 @@ class Relay:
|
||||
def _prepare(self) -> None:
|
||||
for sub in ("req", "resp", "work"):
|
||||
(self.root / sub).mkdir(exist_ok=True)
|
||||
if self.root not in self._cleaned:
|
||||
key = _same_dir_key(self.root)
|
||||
if key not in self._cleaned:
|
||||
# первая подготовка этой папки после старта: взятые прошлым процессом запросы уже никто не обработает
|
||||
for stale in (self.root / "work").iterdir():
|
||||
stale.unlink(missing_ok=True)
|
||||
say(f"relay: удалён незавершённый запрос {stale.name} (старт реле)")
|
||||
self._cleaned.add(self.root)
|
||||
self._cleaned.add(key)
|
||||
else:
|
||||
# пропал подкаталог или не было доступа: взятые запросы ещё обрабатываются своими потоками — work/ не трогаем
|
||||
say(f"relay: папка {self.root} подготовлена заново")
|
||||
@ -225,6 +226,15 @@ class Relay:
|
||||
p.unlink(missing_ok=True)
|
||||
|
||||
|
||||
def _same_dir_key(path: Path) -> Path:
|
||||
"""Ключ папки для «уже вычищена»: resolve() — симлинк и другая запись той же папки совпадают (minor-1 #16,
|
||||
решение 12); ошибка resolve — путь как записан."""
|
||||
try:
|
||||
return path.resolve()
|
||||
except (OSError, RuntimeError):
|
||||
return path
|
||||
|
||||
|
||||
def _error(error: str, detail: str) -> str:
|
||||
return json.dumps({"error": error, "detail": detail}, ensure_ascii=False)
|
||||
|
||||
|
||||
@ -509,3 +509,20 @@ def test_agent_reported_even_with_bad_update_fields(hub, tmp_path):
|
||||
v = hub["client"].get("/agent")
|
||||
assert v["agent_reported"] is True and v["launcher_version"] is None
|
||||
assert v["update_hint"] == "агент запущен без run.py — обновится только вручную"
|
||||
|
||||
|
||||
def test_settings_put_returns_all_settings(hub):
|
||||
"""O-11 #16, решение 9, спека «Ответ на запись настроек»: все настройки после записи, как GET /settings, без токенов."""
|
||||
c = hub["client"]
|
||||
v0 = c.get("/settings")["settings_version"]
|
||||
out = c.put("/settings", {"relay_dir": "/Volumes/externalSSD/utm/rshb/jira-bridge"})
|
||||
assert out["relay_dir"] == "/Volumes/externalSSD/utm/rshb/jira-bridge" and out["settings_version"] == v0 + 1
|
||||
assert "tokens" not in out and out["base_jql"] == "project = AURORA AND labels = AWPCA"
|
||||
assert out == c.get("/settings")
|
||||
|
||||
|
||||
def test_settings_put_empty_body_returns_settings_unchanged(hub):
|
||||
"""Пустое тело — 200, все настройки, версия не меняется."""
|
||||
c = hub["client"]
|
||||
before = c.get("/settings")
|
||||
assert c.put("/settings", {}) == before
|
||||
|
||||
@ -147,3 +147,33 @@ def test_validate_accepts_null_for_clearable_and_path_target():
|
||||
{"type": "ensure_link", "params": {"source": "A-1", "target": "A-2", "link_type": "связанные", "replace": True}},
|
||||
])
|
||||
assert len(out) == 3
|
||||
|
||||
|
||||
@pytest.mark.parametrize("mirror_value, param, expected", [
|
||||
("a\r\nb", "a\nb", True), ("a\nb", "a\r\nb", True), ("a\r\nb\r\n", "a\nb\n", True),
|
||||
("a\r\nb", "a b", False), (None, "", True), ("", None, True), ("a\rb", "a\nb", False),
|
||||
])
|
||||
def test_ensure_fields_text_compare_ignores_crlf(conn, mirror_value, param, expected):
|
||||
"""O-9 #16, решение 11: CRLF и LF не различаются, null и пустая строка равны; одиночный CR — другой текст.
|
||||
summary с CRLF в зеркале и LF в команде — в каждом случае: сравнение одно для всех строковых полей."""
|
||||
conn.execute("UPDATE issue SET description = ?, summary = ? WHERE key = 'A-1'", (mirror_value, "x\r\ny"))
|
||||
ok, _ = commands.check_confirmed(conn, {"type": "ensure_fields",
|
||||
"params": {"key": "A-1", "description": param, "summary": "x\ny"}}, LT)
|
||||
assert ok is expected
|
||||
|
||||
|
||||
@pytest.mark.parametrize("ctype, name, value, msg", [
|
||||
("ensure_fields", "duedate", True, "duedate — строка"),
|
||||
("ensure_fields", "labels_add", "AWPCA", "labels_add — список строк"),
|
||||
("ensure_fields", "labels_add", ["x", 1], "labels_add — список строк"),
|
||||
("ensure_link", "replace", "да", "replace — true или false"),
|
||||
("ensure_status", "target", 5, "target — строка или список строк"),
|
||||
("ensure_status", "target", ["Разработка", "Готово"], None),
|
||||
("ensure_status", "target", "Готово", None),
|
||||
("ensure_fields", "labels_add", [], None),
|
||||
("ensure_fields", "assignee", None, None),
|
||||
("ensure_link", "replace", False, None),
|
||||
])
|
||||
def test_type_error_messages(ctype, name, value, msg):
|
||||
"""T3.1 #16, решение 15: _type_error по веткам — текст ошибки для каждого вида параметра (до и после правки один)."""
|
||||
assert commands._type_error(ctype, name, value) == msg
|
||||
|
||||
@ -462,3 +462,40 @@ def test_cancel_batch_reports_still_claimed(conn):
|
||||
view = queue.cancel_batch(conn, b["id"])
|
||||
assert view["still_claimed"] == [{"id": c["id"], "type": "ensure_fields"}]
|
||||
assert queue.get_command(conn, c["id"])["status"] == "claimed"
|
||||
|
||||
|
||||
def test_dependent_skipped_when_creation_cancelled(conn):
|
||||
"""O-10 #16, решение 10, спека «Создание отменено»: зависимая команда — «создание задачи new:N отменено»."""
|
||||
b = queue.create_batch(conn, "x", "claude", [
|
||||
{"type": "ensure_issue", "params": {"temp_key": "new:901", "issue_type": "task", "summary": "Отменённая"}},
|
||||
{"type": "ensure_comment", "params": {"key": "new:901", "marker": "m", "body": "b"}}])
|
||||
c = queue.claim(conn)
|
||||
queue.cancel_command(conn, c["id"], force=True)
|
||||
assert queue.claim(conn) is None # комментарий и хвостовой refresh пропущены
|
||||
cmds = queue.batch_view(conn, b["id"])["commands"]
|
||||
assert [x["status"] for x in cmds] == ["cancelled", "skipped", "skipped"]
|
||||
assert cmds[1]["error"] == "создание задачи new:901 отменено"
|
||||
|
||||
|
||||
def test_dependent_on_failed_and_cancelled_names_cancelled(conn):
|
||||
"""Review Focus 4: два временных ключа — создатель первого failed, второго cancelled: текст по отменённому."""
|
||||
b = queue.create_batch(conn, "x", "claude", [
|
||||
{"type": "ensure_issue", "params": {"temp_key": "new:1", "issue_type": "task", "summary": "Упавшая"}},
|
||||
{"type": "ensure_issue", "params": {"temp_key": "new:2", "issue_type": "task", "summary": "Отменённая"}},
|
||||
{"type": "ensure_link", "params": {"source": "new:1", "target": "new:2", "link_type": "связанные"}}])
|
||||
first = queue.claim(conn)
|
||||
queue.report_result(conn, first["id"], "failed", error="HTTP 400")
|
||||
second = queue.claim(conn)
|
||||
queue.cancel_command(conn, second["id"], force=True)
|
||||
queue.claim(conn)
|
||||
link = queue.batch_view(conn, b["id"])["commands"][2]
|
||||
assert link["status"] == "skipped" and link["error"] == "создание задачи new:2 отменено"
|
||||
|
||||
|
||||
def test_reconcile_confirms_fields_with_crlf(conn):
|
||||
"""O-9 #16, решение 11, спека «Описание с CRLF в зеркале»: потерянный отчёт закрывается по состоянию."""
|
||||
queue.create_batch(conn, "x", "claude", [{"type": "ensure_fields", "params": {"key": "A-1", "description": "a\nb"}}])
|
||||
c = queue.claim(conn)
|
||||
mirror.apply_state(conn, [{"key": "A-1", "summary": "т", "description": "a\r\nb", "labels": []}], [])
|
||||
assert queue.reconcile(conn) == [c["id"]]
|
||||
assert queue.get_command(conn, c["id"])["status"] == "confirmed_by_state"
|
||||
|
||||
@ -402,3 +402,21 @@ def test_relay_targets_serve_host(tmp_path, monkeypatch):
|
||||
stop.set()
|
||||
srv.server_close()
|
||||
assert seen == ["127.0.0.1", "localhost"] # конкретный адрес serve — туда же ходит реле (M-1 #8)
|
||||
|
||||
|
||||
def test_symlink_to_same_dir_not_cleaned_again(env, tmp_path, capsys):
|
||||
"""minor-1 #16, решение 12: relay_dir сменили на симлинк к той же папке — work/ не чистится второй раз."""
|
||||
r, root, h = env["relay"], env["root"], env["hub"]
|
||||
r.step() # root вычищен
|
||||
link = tmp_path / "bridge-link"
|
||||
link.symlink_to(root, target_is_directory=True)
|
||||
(root / "work" / "busy.json").write_text("{}", encoding="utf-8") # взятый запрос ещё обрабатывается
|
||||
conn = h.conn()
|
||||
settings.set_(conn, "relay_dir", str(link))
|
||||
conn.close()
|
||||
r._settings_at = None
|
||||
capsys.readouterr()
|
||||
r.step()
|
||||
out = capsys.readouterr().out
|
||||
assert (root / "work" / "busy.json").exists()
|
||||
assert "удалён незавершённый запрос" not in out and "подготовлена заново" in out
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user