Правки финального ревью: ссылки на вложения, размер, 413 в пакетах, имя копии, документация (#83)
I-1 I-2 M-1 M-2 M-3 M-5 M-6 M-7 T3.4 T4.3 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Andrey Epifancev
parent
1e4457d710
commit
62057606c0
@ -48,7 +48,7 @@
|
||||
- Вложение: файл → `POST /files` → `files/<sha256>` → `ensure_attachment {key, file, filename}` в пакете (`queue._check_setup` проверяет, что файл есть) → `claim` (хаб дописывает `size`, в базе `params` без него) → агент `GET /agent/files/{sha256}` → Jira; уборка `files/` — из `tick` раз в сутки.
|
||||
- Создание проекта: `POST /projects` (веб или `project_create`) → `projects.create` и пакет «Новый проект <метка>» (`refresh` по общей JQL, `batch.project = NULL`) в одной транзакции → `claim` (хаб дописывает в `refresh` доски проектов, `boards`) → агент определяет тип доски → отчёт `refresh` пишет `board_type` → `ensure_sprint` проекта получает `board_id`.
|
||||
- Чтение Jira: команда `refresh` → агент → `POST /agent/state` → `mirror.apply_state`.
|
||||
- Скачивание вложения: `queue.request_fetch` (одна транзакция: состояние `attachments.state` + пакет `fetch_attachment {key, attachment_id}`) → `claim` (без копии базы) → агент → `POST /agent/files` → результат `{file, size}` → `report_result` проверяет файл в `files/` и пишет `attachment_file`; нет файла или чужой размер — команда `failed`.
|
||||
- Скачивание вложения: вход — `POST /attachments/{id}/fetch` (из веб-приложения и из MCP `attachment`); `queue.request_fetch` (одна транзакция: состояние `attachments.state` + пакет `fetch_attachment {key, attachment_id}`) → `claim` (без копии базы) → агент → `POST /agent/files` → результат `{file, size}` → `report_result` проверяет файл в `files/` и пишет `attachment_file`; нет файла или чужой размер — команда `failed`. Выход — `GET /attachments/{id}/content`: в вебе Blob и сохранение браузером, в MCP копия `downloads/<id>/<имя>`.
|
||||
- Журнал изменений: выдача `refresh` (`queue.deliver_params` дописывает `history` — `since` и/или стартовую `jql`) →
|
||||
агент v13 после состояния читает changelog Jira → `POST /agent/history` → `history.record_changes` (повторы
|
||||
отбрасываются по `(history_id, seq)`) → отчёт `refresh` с `result.history` → `sync_run.history_at` у полного прогона
|
||||
|
||||
@ -9,7 +9,7 @@
|
||||
- Хаб не знает пароль Jira и не ходит в Jira: учётные данные и сеть до Jira только у агента.
|
||||
- Зеркало задач пишет только `mirror.apply_state` по загрузке состояния от агента (`POST /agent/state`); правки идут командами через очередь.
|
||||
- Хаб не создаёт команды сам: повтор после failed — новая команда по запросу человека/интерфейса. Команду, выданную агенту и оставшуюся без отчёта, хаб довыдаёт при следующем claim (до 3 выдач, затем failed).
|
||||
- Данные: `~/.local/share/jira-hub/` (`JIRA_HUB_HOME`): `hub.sqlite` (зеркало, очередь, журнал изменений `issue_change`), `backups/`, `release/` (опубликованная версия агента), `files/` (файлы вложений по sha256, уборка через 30 дней), `downloads/` (копии вложений для MCP `<id>/<имя>`, уборка 30 дней); каталог данных, `backups/`, `release/` и `files/` — права 0700. Порт по умолчанию 8790.
|
||||
- Данные: `~/.local/share/jira-hub/` (`JIRA_HUB_HOME`): `hub.sqlite` (зеркало, очередь, журнал изменений `issue_change`), `backups/`, `release/` (опубликованная версия агента), `files/` (файлы вложений по sha256, уборка через 30 дней), `downloads/` (копии вложений для MCP `<id>/<имя>`, уборка 30 дней); каталог данных, `backups/`, `release/`, `files/` и `downloads/` — права 0700. Порт по умолчанию 8790.
|
||||
- Два статических токена (в таблице `setting`): агентский открывает только `/agent/*`, клиентский — остальное.
|
||||
|
||||
## Архитектура
|
||||
|
||||
@ -344,6 +344,8 @@ def h_batches_post(hub, conn, m, q, body):
|
||||
try:
|
||||
return 201, queue.create_batch(conn, body["title"], body.get("author") or "api", body.get("commands"),
|
||||
files_dir=hub.files_dir, project=body.get("project"))
|
||||
except queue.TooLarge as e: # раньше CommandError: TooLarge — его подкласс (M-2)
|
||||
raise ApiError(413, "too_large", str(e)) from None
|
||||
except CommandError as e:
|
||||
raise ApiError(422, "bad_batch", str(e))
|
||||
|
||||
@ -421,8 +423,10 @@ def h_attachment_fetch(hub, conn, m, q, body):
|
||||
def h_attachment_content(hub, conn, m, q, body):
|
||||
"""Содержимое скачанного вложения; mime — из вложения, негодный → octet-stream; имя — filename* (RFC 5987)."""
|
||||
att = _attachment(hub, conn, m)
|
||||
row = conn.execute("SELECT file FROM attachment_file WHERE attachment_id = ?", (m.group(1),)).fetchone()
|
||||
data = files.read(hub.files_dir, row["file"]) if att["state"] == "ready" and row else None
|
||||
# state == ready означает, что запись attachment_file есть и файл в хранилище на месте (attachments.state)
|
||||
row = conn.execute("SELECT file FROM attachment_file WHERE attachment_id = ?", (m.group(1),)).fetchone() \
|
||||
if att["state"] == "ready" else None
|
||||
data = files.read(hub.files_dir, row["file"]) if row else None
|
||||
if data is None:
|
||||
raise ApiError(404, "not_fetched", "вложение не скачано — запросите скачивание")
|
||||
files.touch(hub.files_dir, row["file"])
|
||||
@ -443,6 +447,8 @@ def h_command_retry(hub, conn, m, q, body):
|
||||
project=body.get("project"))
|
||||
except KeyError:
|
||||
raise ApiError(404, "not_found", "command")
|
||||
except queue.TooLarge as e: # раньше CommandError: TooLarge — его подкласс (M-2)
|
||||
raise ApiError(413, "too_large", str(e)) from None
|
||||
except CommandError as e:
|
||||
raise ApiError(422, "bad_batch", str(e))
|
||||
|
||||
|
||||
@ -5,7 +5,9 @@ import re
|
||||
|
||||
from . import files
|
||||
|
||||
_IMAGE = re.compile(r"!([^!|\n]+)(?:\|[^!\n]*)?!")
|
||||
# lookahead: совпадения перекрываются, поэтому одиночный «!» ранее в тексте не съедает открывающий «!» настоящей ссылки;
|
||||
# после открывающего «!» пробела нет (как в Jira), иначе закрывающий «!» одной картинки породил бы ложное имя
|
||||
_IMAGE = re.compile(r"(?=!(?!\s)([^!|\n]+)(?:\|[^!\n]*)?!)")
|
||||
_LINK = re.compile(r"\[(?:[^\]|\n]*\|)?\^([^\]\n]+)\]")
|
||||
|
||||
|
||||
@ -14,9 +16,13 @@ def fmt_size(size: int | None) -> str:
|
||||
return "размер неизвестен"
|
||||
if size < 1024:
|
||||
return f"{size} Б"
|
||||
if size < 1024 ** 2:
|
||||
return f"{size / 1024:.1f} КБ".replace(".", ",")
|
||||
return f"{size / 1024 ** 2:.1f} МБ".replace(".", ",")
|
||||
# десятые — в целых, округление half up (как в вебе, fmtSize); единица — по округлённому значению:
|
||||
# 1048575 Б — «1,0 МБ», а не «1024,0 КБ»
|
||||
kb = (size * 10 + 512) // 1024
|
||||
if kb < 10240:
|
||||
return f"{kb // 10},{kb % 10} КБ"
|
||||
mb = (size * 10 + 524288) // 1048576
|
||||
return f"{mb // 10},{mb % 10} МБ"
|
||||
|
||||
|
||||
def ref_names(text: str | None) -> list[str]:
|
||||
|
||||
@ -158,8 +158,16 @@ def _files_line(c: dict, atts: list) -> str:
|
||||
|
||||
|
||||
def safe_name(filename: str, attachment_id: str) -> str:
|
||||
"""Имя файла копии: «/ \\ :» и управляющие символы (< 32, 127) → «_»; пусто, «.», «..» → attachment-<id>."""
|
||||
"""Имя файла копии: «/ \\ :» и управляющие символы (< 32, 127) → «_»; не длиннее 255 байт UTF-8; пусто, «.», «..» → attachment-<id>."""
|
||||
name = "".join("_" if ch in "/\\:" or ord(ch) < 32 or ord(ch) == 127 else ch for ch in filename or "")
|
||||
if len(name.encode()) > 255: # предел имени в ФС — 255 байт: режем основу, расширение сохраняем
|
||||
stem, dot, ext = name.rpartition(".")
|
||||
if not dot or len(ext.encode()) > 32: # «расширение» из одного длинного хвоста — просто режем всё имя
|
||||
stem, ext = name, ""
|
||||
else:
|
||||
ext = "." + ext
|
||||
stem = stem.encode()[:255 - len(ext.encode())].decode("utf-8", "ignore")
|
||||
name = stem + ext
|
||||
return name if name.strip() and name.strip() not in (".", "..") else f"attachment-{attachment_id}"
|
||||
|
||||
|
||||
@ -345,6 +353,7 @@ def _store_copy(attachment_id: str, name: str, data: bytes) -> Path:
|
||||
d.chmod(0o700)
|
||||
target = folder / name
|
||||
if target.is_file() and target.stat().st_size == len(data):
|
||||
os.utime(folder) # копия снова нужна — уборка по mtime папки её не заденет
|
||||
return target
|
||||
fd, tmp = tempfile.mkstemp(dir=folder, prefix=".part-")
|
||||
try:
|
||||
|
||||
@ -40,8 +40,8 @@ def check_attachments(issues) -> None:
|
||||
where = f"{key}: attachments[{n}]"
|
||||
if not isinstance(a, dict):
|
||||
raise ValueError(f"{where} — объект вложения")
|
||||
if not isinstance(a.get("id"), str) or not a["id"]:
|
||||
raise ValueError(f"{where}.id — непустая строка")
|
||||
if not isinstance(a.get("id"), str) or not a["id"].isdigit():
|
||||
raise ValueError(f"{where}.id — строка из цифр")
|
||||
if not isinstance(a.get("filename"), str) or not a["filename"]:
|
||||
raise ValueError(f"{where}.filename — непустая строка")
|
||||
size = a.get("size")
|
||||
|
||||
@ -1153,6 +1153,26 @@ def test_attachment_fetch_too_large_413(hub):
|
||||
assert hub["client"].get("/batches")["batches"] == []
|
||||
|
||||
|
||||
def test_batches_post_fetch_too_large_413(hub):
|
||||
"""M-2: POST /batches с fetch_attachment > 10 МБ — 413 too_large, не 422."""
|
||||
_seed_attachment(hub, size=25_000_000)
|
||||
with pytest.raises(HubError) as e:
|
||||
hub["client"].post("/batches", {"title": "x", "commands": [
|
||||
{"type": "fetch_attachment", "params": {"key": "A-1", "attachment_id": "31337"}}]})
|
||||
assert e.value.status == 413 and e.value.error == "too_large"
|
||||
assert hub["client"].get("/batches")["batches"] == []
|
||||
|
||||
|
||||
def test_command_retry_fetch_too_large_413(hub):
|
||||
"""M-2: повтор fetch_attachment, когда размер в зеркале вырос выше предела, — 413 too_large."""
|
||||
_seed_attachment(hub)
|
||||
st = hub["client"].post("/attachments/31337/fetch", {})
|
||||
_seed_attachment(hub, size=25_000_000)
|
||||
with pytest.raises(HubError) as e:
|
||||
hub["client"].post(f"/commands/{st['command_id']}/retry", {"author": "claude"})
|
||||
assert e.value.status == 413 and e.value.error == "too_large"
|
||||
|
||||
|
||||
def test_attachment_fetch_agent_too_old_409(hub):
|
||||
_seed_attachment(hub)
|
||||
hub["agent"].post("/agent/claim", {"agent_version": 13})
|
||||
|
||||
@ -10,6 +10,10 @@ def test_fmt_size():
|
||||
assert attachments.fmt_size(240000) == "234,4 КБ"
|
||||
assert attachments.fmt_size(25_000_000) == "23,8 МБ"
|
||||
assert attachments.fmt_size(None) == "размер неизвестен"
|
||||
# M-1: десятые считаются в целых, округление half up; единица — по округлённому значению
|
||||
assert attachments.fmt_size(1280) == "1,3 КБ"
|
||||
assert attachments.fmt_size(1048575) == "1,0 МБ"
|
||||
assert attachments.fmt_size(1310720) == "1,3 МБ"
|
||||
|
||||
|
||||
def test_ref_names():
|
||||
@ -20,6 +24,15 @@ def test_ref_names():
|
||||
assert attachments.ref_names(None) == []
|
||||
|
||||
|
||||
def test_ref_names_lone_exclamation():
|
||||
"""I-1: одиночный «!» перед настоящей ссылкой не съедает её открывающий «!»."""
|
||||
assert "скрин.png" in attachments.ref_names("Внимание! см. !скрин.png!")
|
||||
names = attachments.ref_names("Важно! [^b.xlsx] и !скрин.png!")
|
||||
assert names.index("b.xlsx") < names.index("скрин.png")
|
||||
assert attachments.ref_names("{code}!a.png!{code} !b.png!")[-1] == "b.png"
|
||||
assert attachments.ref_names("!http://x/y.png! !c.png!")[-1] == "c.png"
|
||||
|
||||
|
||||
def test_resolve_newest():
|
||||
items = [{"attachment_id": "10", "filename": "скрин.png", "created_at": "2026-10-14T10:00:00+00:00"},
|
||||
{"attachment_id": "20", "filename": "скрин.png", "created_at": "2026-10-20T10:00:00+00:00"}]
|
||||
|
||||
@ -4,6 +4,7 @@ import json
|
||||
import os
|
||||
import re
|
||||
import threading
|
||||
from pathlib import Path
|
||||
from types import SimpleNamespace
|
||||
|
||||
import pytest
|
||||
@ -728,6 +729,27 @@ def test_comment_attachments():
|
||||
assert "вложения" not in mcp_server.fmt_comment(ATT_KEY, {**d["comments"][0], "attachment_ids": []}, d["attachments"])
|
||||
|
||||
|
||||
def test_safe_name_limits_255_bytes():
|
||||
"""M-7: имя ≤ 255 байт UTF-8, расширение сохраняется, многобайтовый символ не режется."""
|
||||
for stem in ("я" * 200, "ы" * 127 + "a"):
|
||||
name = mcp_server.safe_name(stem + ".xlsx", "7")
|
||||
assert len(name.encode()) <= 255 and name.endswith(".xlsx")
|
||||
name.encode("utf-8")
|
||||
assert name.startswith("я") or name.startswith("ы")
|
||||
assert len(mcp_server.safe_name("ф" * 300, "7").encode()) <= 255
|
||||
assert mcp_server.safe_name("короткое.png", "7") == "короткое.png"
|
||||
|
||||
|
||||
def test_attachment_long_cyrillic_name_written(downloads, monkeypatch):
|
||||
name = "я" * 200 + ".png"
|
||||
att = _att("31337", name, 3, state="ready")
|
||||
hub = FakeHub(_detail(key=ATT_KEY, attachments=[att]), fetch={"state": "ready", "command_id": 5}, polls=[att], data=b"abc")
|
||||
_use(monkeypatch, hub)
|
||||
out = asyncio.run(mcp_server.attachment(ATT_KEY, "31337"))
|
||||
path = Path(out.splitlines()[0])
|
||||
assert path.is_file() and path.read_bytes() == b"abc" and len(path.name.encode()) <= 255
|
||||
|
||||
|
||||
def test_safe_name():
|
||||
assert mcp_server.safe_name("a/b\\c:d\x01.png", "7") == "a_b_c_d_.png"
|
||||
assert mcp_server.safe_name(" ", "7") == "attachment-7"
|
||||
@ -812,5 +834,7 @@ def test_attachment_existing_copy_same_size(downloads, monkeypatch):
|
||||
path.parent.mkdir(parents=True)
|
||||
path.write_bytes(b"OLD" * 3)
|
||||
os.utime(path, (1_000_000, 1_000_000))
|
||||
os.utime(path.parent, (1_000_000, 1_000_000))
|
||||
asyncio.run(mcp_server.attachment(ATT_KEY, "31337"))
|
||||
assert path.read_bytes() == b"OLD" * 3 and path.stat().st_mtime == 1_000_000
|
||||
assert path.read_bytes() == b"OLD" * 3
|
||||
assert path.parent.stat().st_mtime > 1_000_000 + 100 # M-6: папка «освежена» — уборка её не заденет
|
||||
|
||||
@ -363,7 +363,8 @@ def test_attachment_moves_to_other_key(conn):
|
||||
|
||||
@pytest.mark.parametrize("bad, field", [
|
||||
("x", "attachments"), (["x"], "attachments[0]"),
|
||||
([{"id": "", "filename": "a"}], "attachments[0].id"), ([{"id": 5, "filename": "a"}], "attachments[0].id"),
|
||||
([{"id": "", "filename": "a"}], "attachments[0].id"), ([{"id": "1a", "filename": "a"}], "attachments[0].id"),
|
||||
([{"id": "-5", "filename": "a"}], "attachments[0].id"), ([{"id": 5, "filename": "a"}], "attachments[0].id"),
|
||||
([{"id": "5", "filename": 1}], "attachments[0].filename"), ([{"id": "5", "filename": ""}], "attachments[0].filename"),
|
||||
([{"id": "5", "filename": "a", "size": -1}], "attachments[0].size"),
|
||||
([{"id": "5", "filename": "a", "size": "1"}], "attachments[0].size"),
|
||||
|
||||
@ -1070,6 +1070,25 @@ def test_fetch_checks(fconn):
|
||||
assert queue.create_batch(fconn, "x", "claude", [fetch("31339")])["count"] == 1 # size NULL — ставится
|
||||
|
||||
|
||||
def test_fetch_size_limit_boundary(fconn):
|
||||
"""T3.4: ровно MAX_SIZE принимается, MAX_SIZE+1 — TooLarge."""
|
||||
mirror.apply_state(fconn, [{"key": "A-1", "summary": "т", "labels": [], "attachments": [
|
||||
{"id": "41", "filename": "eq.bin", "size": files.MAX_SIZE, "created": "2026-10-20T10:00:00.000+0300"},
|
||||
{"id": "42", "filename": "over.bin", "size": files.MAX_SIZE + 1, "created": "2026-10-20T10:00:00.000+0300"}]}], [])
|
||||
assert queue.request_fetch(fconn, None, "41", "claude")["state"] == "fetching"
|
||||
with pytest.raises(queue.TooLarge):
|
||||
queue.request_fetch(fconn, None, "42", "claude")
|
||||
|
||||
|
||||
def test_fetch_report_done_non_dict_result_fails(fconn, tmp_path):
|
||||
"""T3.4: done с result не-словарём — failed, вложение не отмечено скачанным."""
|
||||
queue.create_batch(fconn, "x", "claude", [fetch()])
|
||||
c = queue.claim(fconn)
|
||||
r = queue.report_result(fconn, c["id"], "done", result="ok", files_dir=tmp_path)
|
||||
assert r["status"] == "failed"
|
||||
assert fconn.execute("SELECT COUNT(*) AS n FROM attachment_file").fetchone()["n"] == 0
|
||||
|
||||
|
||||
def test_fetch_no_backup(fconn):
|
||||
queue.create_batch(fconn, "x", "claude", [fetch()])
|
||||
calls = []
|
||||
|
||||
@ -10,6 +10,10 @@ describe("fmtSize", () => {
|
||||
expect(fmtSize(240000)).toBe("234,4 КБ");
|
||||
expect(fmtSize(1024 ** 2)).toBe("1,0 МБ");
|
||||
expect(fmtSize(25 * 1024 ** 2)).toBe("25,0 МБ");
|
||||
expect(fmtSize(1280)).toBe("1,3 КБ");
|
||||
expect(fmtSize(1048575)).toBe("1,0 МБ");
|
||||
expect(fmtSize(1310720)).toBe("1,3 МБ");
|
||||
expect(fmtSize(25_000_000)).toBe("23,8 МБ");
|
||||
expect(fmtSize(null)).toBe("размер неизвестен");
|
||||
});
|
||||
});
|
||||
|
||||
@ -8,8 +8,11 @@ export const TOO_BIG_HINT = "больше 10 МБ — только в Jira";
|
||||
export function fmtSize(size: number | null): string {
|
||||
if (size === null) return "размер неизвестен";
|
||||
if (size < 1024) return `${size} Б`;
|
||||
if (size < 1024 ** 2) return `${(size / 1024).toFixed(1).replace(".", ",")} КБ`;
|
||||
return `${(size / 1024 ** 2).toFixed(1).replace(".", ",")} МБ`;
|
||||
// десятые — в целых, округление half up (как в хабе); единица — по округлённому значению: 1048575 → «1,0 МБ»
|
||||
const kb = Math.floor((size * 10 + 512) / 1024);
|
||||
if (kb < 10240) return `${Math.floor(kb / 10)},${kb % 10} КБ`;
|
||||
const mb = Math.floor((size * 10 + 524288) / 1048576);
|
||||
return `${Math.floor(mb / 10)},${mb % 10} МБ`;
|
||||
}
|
||||
|
||||
export const isTooBig = (size: number | null): boolean => size !== null && size > MAX_SIZE;
|
||||
|
||||
@ -45,6 +45,8 @@ describe("useAttachmentDownload", () => {
|
||||
await act(async () => { await vi.advanceTimersByTimeAsync(2000); });
|
||||
expect(polls).toBe(2);
|
||||
expect(clicked).toEqual([{ download: "скрин 1.png", href: "blob:fake" }]);
|
||||
expect(URL.revokeObjectURL).not.toHaveBeenCalled(); // I-2: отзыв отложен
|
||||
await act(async () => { await vi.advanceTimersByTimeAsync(60_000); });
|
||||
expect(URL.revokeObjectURL).toHaveBeenCalledWith("blob:fake");
|
||||
expect(result.current.status("31337")).toEqual({ busy: false, error: null });
|
||||
await act(async () => { await vi.advanceTimersByTimeAsync(10000); });
|
||||
|
||||
@ -15,8 +15,12 @@ function save(blob: Blob, filename: string) {
|
||||
const a = document.createElement("a");
|
||||
a.href = url;
|
||||
a.download = filename;
|
||||
document.body.appendChild(a); // Firefox не кликает по ссылке вне документа
|
||||
a.click();
|
||||
URL.revokeObjectURL(url);
|
||||
a.remove();
|
||||
// ссылку отзываем не сразу: браузер может начать чтение Blob уже после click(); таймер не в `timers` —
|
||||
// размонтирование карточки не должно оставлять URL неотозванным
|
||||
setTimeout(() => URL.revokeObjectURL(url), 60_000);
|
||||
}
|
||||
|
||||
const errorText = (e: unknown): string => e instanceof ApiError ? (e.detail ?? e.message) : e instanceof Error ? e.message : String(e);
|
||||
|
||||
@ -101,6 +101,11 @@ describe("parseInline: ссылки на вложения", () => {
|
||||
expect(parseInline("!нет.png!", A)).toEqual([t("!нет.png!")]);
|
||||
expect(parseInline("[^нет.pdf] и [x|^нет.pdf]", A)).toEqual([t("[^нет.pdf] и [x|^нет.pdf]")]);
|
||||
});
|
||||
it("одиночный «!» перед ссылкой не мешает ей (I-1)", () => {
|
||||
expect(parseInline("Внимание! см. !a.pdf!", A)).toContainEqual(att("31338", t("a.pdf")));
|
||||
const r = parseInline("Важно! [^a.pdf] и !a.pdf!", A);
|
||||
expect(r.filter((n) => n.t === "attachment")).toEqual([att("31338", t("a.pdf")), att("31338", t("a.pdf"))]);
|
||||
});
|
||||
it("внешняя картинка не вложение", () => {
|
||||
expect(JSON.stringify(parseInline("!http://x/скрин 1.png!", { attachments: [{ id: "1", filename: "http://x/скрин 1.png", created: null }] }))).not.toContain("attachment");
|
||||
});
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user