From 4b6ec55a7df3126b37bd582f56976d1a21bbcc40 Mon Sep 17 00:00:00 2001 From: thewrz Date: Wed, 22 Jul 2026 17:13:17 -0700 Subject: [PATCH 01/10] feat(issue-65): add client-boundary support for description sync Adds the config/AgilePlace/GitHub client-boundary primitives that description sync (issue #65) will wire up in a later task: env_config()'s ap_description_max_length (safe-int-parse with WARN fallback, default 20000), agileplace.op_description/card_description (description-present path plus lazy get_card fallback, matching the API's list_cards()-never- returns-description shape), and ghkit.edit_issue_body plus list_issues()'s null-safe 'body' field, both following the existing dry-run/apply/ boundary-validation idiom. These additions are inert until description_sync.py and sync.py wire them in -- no existing call site is touched. Co-Authored-By: Claude Sonnet 5 --- .env.example | 7 ++ agileplace.py | 21 ++++ config.py | 28 +++++ ghkit.py | 30 ++++- tests/test_agileplace.py | 62 ++++++++++ tests/test_config_env.py | 72 +++++++++++ tests/test_ghkit_edit_issue_body.py | 186 ++++++++++++++++++++++++++++ 7 files changed, 405 insertions(+), 1 deletion(-) create mode 100644 tests/test_ghkit_edit_issue_body.py diff --git a/.env.example b/.env.example index 80ed38a..0820341 100644 --- a/.env.example +++ b/.env.example @@ -28,6 +28,13 @@ GH_PROJECT_TARGET_FIELD=Target # agent:in-progress, agent:in-review, and agent:ready are always excluded (they drive lane movement). LABEL_SYNC_IGNORE= +# Optional: max character length of an AgilePlace card description (rendered HTML) before the sync +# truncates it (appending a marker) rather than risking a write-time error. AgilePlace's own limit is +# undocumented, so this defaults to a conservative value below every documented LeanKit/AgilePlace +# field limit we could find. Must be a positive integer; anything else falls back to the default with +# a WARN. +AP_DESCRIPTION_MAX_LENGTH= + # Optional: map GitHub stages -> YOUR board's lanes when lane titles don't self-describe. Format: # ';'-separated "Stage=Lane" entries; '|' lists multiple lanes per stage (FIRST = move-to target, ALL = # "already in that stage", so the sync won't shuffle a card between equivalent lanes). Stages are: diff --git a/agileplace.py b/agileplace.py index eb1eab5..d219294 100644 --- a/agileplace.py +++ b/agileplace.py @@ -332,6 +332,19 @@ def get_card(cfg: dict, card_id: str) -> dict: return card +def card_description(cfg: dict, card: dict) -> str: + """The card's description text, normalized to "" for None/absent. list_cards() never returns + `description` (no field-selection params sent, confirmed against the live board), so a card + reaching this function via the board snapshot is almost always missing the key entirely -- that + (and ONLY that) triggers one lazy get_card refetch. A card that DOES carry the key -- including + description="" -- takes the zero-I/O path: an explicit empty string is a real, current + description, not "unknown", and must never be treated as a reason to hit the network.""" + if "description" in card: + return card["description"] or "" + fresh = get_card(cfg, card["id"]) + return fresh.get("description") or "" + + def _warn_card_field(card: dict, detail: str) -> None: print(f"WARN card {card.get('id', '')} {detail} -- skipping malformed value") @@ -480,6 +493,14 @@ def op_custom_id(custom_id: str) -> dict: return {"op": "replace", "path": "/customId", "value": custom_id} +def op_description(html: str) -> dict: + """A single unconditional replace of the card's description with `html` (already rendered by + richtext.markdown_to_leankit_html and, when oversized, truncated by + description_sync._truncate_for_agileplace). No validation here -- description_sync owns the + decision of *whether* to write; this is purely the op-builder, matching op_custom_id's shape.""" + return {"op": "replace", "path": "/description", "value": html} + + def op_lane(lane_id: str) -> dict: return {"op": "replace", "path": "/laneId", "value": lane_id} diff --git a/config.py b/config.py index a091ab9..a6566ea 100644 --- a/config.py +++ b/config.py @@ -24,6 +24,12 @@ # over-abstraction for no benefit. _ENV_LOADER_BLOCKLIST = frozenset({"GH_REPO", "GH_HOST"}) +# AgilePlace's `description` field length limit is undocumented (VALIDATE LIVE -- see +# API-VALIDATION.md); this is a conservative ceiling picked to stay well under every publicly +# documented LeanKit/AgilePlace card-field limit we could find. description_sync truncates (with +# TRUNCATION_MARKER appended) rather than risking a write-time 4xx from an oversized description. +DEFAULT_AP_DESCRIPTION_MAX_LENGTH = 20000 + def load_env_file() -> None: """Load KEY=VALUE lines from ./.env; real environment variables win over it. GH_REPO/GH_HOST are @@ -64,6 +70,26 @@ def parse_stage_lane_map(raw: str) -> dict[str, list[str]]: return mapping +def _parse_ap_description_max_length(raw: str | None) -> int: + """AP_DESCRIPTION_MAX_LENGTH from .env/environment, falling back to + DEFAULT_AP_DESCRIPTION_MAX_LENGTH (with one WARN) on anything that isn't a positive int -- a + malformed override must never silently disable truncation (e.g. becoming 0/negative) or crash + env_config() outright.""" + if raw is None or not raw.strip(): + return DEFAULT_AP_DESCRIPTION_MAX_LENGTH + try: + value = int(raw.strip()) + except ValueError: + print(f"WARN AP_DESCRIPTION_MAX_LENGTH={raw!r} is not an integer -- using default " + f"{DEFAULT_AP_DESCRIPTION_MAX_LENGTH}") + return DEFAULT_AP_DESCRIPTION_MAX_LENGTH + if value <= 0: + print(f"WARN AP_DESCRIPTION_MAX_LENGTH={raw!r} must be a positive integer -- using default " + f"{DEFAULT_AP_DESCRIPTION_MAX_LENGTH}") + return DEFAULT_AP_DESCRIPTION_MAX_LENGTH + return value + + def env_config() -> dict: """token/host/board_id are None when absent (offline dry run). target_repo_path is the local clone every `gh` call runs against.""" @@ -78,6 +104,8 @@ def env_config() -> dict: "target_repo_path": Path(target).expanduser() if target else None, "label_sync_ignore": frozenset(DEFAULT_IGNORE) | frozenset(extra), "stage_lane_map": parse_stage_lane_map(os.environ.get("STAGE_LANE_MAP", "")), + "ap_description_max_length": _parse_ap_description_max_length( + os.environ.get("AP_DESCRIPTION_MAX_LENGTH")), "gh_project": { "owner": os.environ.get("GH_PROJECT_OWNER") or None, "number": os.environ.get("GH_PROJECT_NUMBER") or None, diff --git a/ghkit.py b/ghkit.py index 43c224e..d629fb3 100644 --- a/ghkit.py +++ b/ghkit.py @@ -115,7 +115,7 @@ def list_issues(cfg: dict) -> list[dict]: the card-creation path. """ out = run(cfg, ["issue", "list", "--state", "all", "--limit", "1000", "--json", - "number,title,state,stateReason,labels,milestone,assignees,url"]) + "number,title,state,stateReason,labels,milestone,assignees,url,body"]) issues = json.loads(out.stdout or "[]") normalized = [] for i in issues: @@ -129,6 +129,7 @@ def list_issues(cfg: dict) -> list[dict]: "milestone": ms.get("title") or None, "assignees": [a.get("login") for a in i.get("assignees", [])], "url": i.get("url", ""), + "body": i.get("body") or "", # description_sync's GitHub-side canonicalization input "has_open_pr": False, # populated by open_pr_issue_numbers() }) return normalized @@ -329,6 +330,33 @@ def set_milestone(cfg: dict, apply: bool, number: int, title: str | None) -> Non print(f"DRY gh issue edit {number} --remove-milestone") +def edit_issue_body(cfg: dict, apply: bool, number: int, body: str) -> bool: + """Set an issue's body via `gh issue edit --body-file -`, through the dry-run gate. The body is + piped through run()'s `input=` stdin passthrough (same idiom as create_issue's --body-file -), + never interpolated into argv, so a description containing shell metacharacters or gh-flag-like + text can't be misparsed. + + Returns True only when the write actually happened (apply=True and gh succeeded) and False for a + dry run -- description_sync.sync_description gates its base-advance on this exact boolean + (gh_write_ok), so a dry run must never report success. Any CalledProcessError/TimeoutExpired from + run() propagates uncaught, matching create_issue/edit_label's own apply=True behavior -- a failed + write must not be swallowed into a false "it worked". + + Validates `number`/`body` at this boundary, before either the dry-run print or a live run() call + -- an invalid number would otherwise reach `gh issue edit ` and fail opaquely, and a + non-string body would crash run()'s stdin pipe with a confusing TypeError.""" + if not isinstance(number, int) or isinstance(number, bool) or number < 1: + raise ValueError(f"edit_issue_body: number must be a positive int, got {number!r}") + if not isinstance(body, str): + raise ValueError(f"edit_issue_body: body must be a str, got {type(body).__name__}") + if not apply: + print(f"DRY gh issue edit {number} --body-file -") + return False + run(cfg, ["issue", "edit", str(number), "--body-file", "-"], input=body) + print(f"gh issue {number} body updated") + return True + + def _issue_number_from_url(url: str) -> int: """The trailing /issues/{n} segment of a GitHub issue URL, as an int. `gh issue create`'s stdout contract is exactly one bare URL line; a malformed one is a genuine unrecovered failure, diff --git a/tests/test_agileplace.py b/tests/test_agileplace.py index cd8f011..f7efba8 100644 --- a/tests/test_agileplace.py +++ b/tests/test_agileplace.py @@ -12,6 +12,7 @@ from agileplace import ( # noqa: E402 _card_with_version, api, + card_description, card_external_urls, card_block_reason, card_is_blocked, @@ -22,6 +23,7 @@ get_card, list_cards, op_custom_id, + op_description, op_planned_date, op_tag, ops_blocked, @@ -789,6 +791,66 @@ def fake_api_double_miss(cfg, method, path, body=None, params=None, headers=None patch_card(CFG, True, card, ops) +# --- op_description / card_description (issue #65 Task 1/7) -------------- + +def test_op_description_returns_single_replace_op(): + """No validation here (description_sync owns write-vs-not) -- matches op_custom_id's shape.""" + op = op_description("

hello

") + assert op == {"op": "replace", "path": "/description", "value": "

hello

"} + + +def test_card_description_returns_present_description_without_network_call(): + card = {"id": "1", "description": "

hi

"} + with patch("agileplace.get_card") as get_card_mock: + result = card_description(CFG, card) + get_card_mock.assert_not_called() + assert result == "

hi

" + + +def test_card_description_present_but_empty_string_takes_zero_io_path(): + """A card literally carrying description="" is a real, current empty description -- NOT + 'unknown' -- and must not trigger the lazy get_card fallback (see struct #7 in the design: + every fixture reaching this must either carry the key or explicitly mock get_card).""" + card = {"id": "1", "description": ""} + with patch("agileplace.get_card") as get_card_mock: + result = card_description(CFG, card) + get_card_mock.assert_not_called() + assert result == "" + + +def test_card_description_present_but_none_normalizes_to_empty_string(): + card = {"id": "1", "description": None} + with patch("agileplace.get_card") as get_card_mock: + result = card_description(CFG, card) + get_card_mock.assert_not_called() + assert result == "" + + +def test_card_description_missing_key_falls_back_to_lazy_get_card(): + """list_cards() never returns description (no field-selection params sent) -- a card summary + missing the key entirely must trigger exactly one get_card refetch.""" + card = {"id": "77"} + with patch("agileplace.get_card", + return_value={"id": "77", "description": "

fresh

"}) as get_card_mock: + result = card_description(CFG, card) + get_card_mock.assert_called_once_with(CFG, "77") + assert result == "

fresh

" + + +def test_card_description_lazy_fallback_normalizes_missing_description_to_empty_string(): + card = {"id": "78"} + with patch("agileplace.get_card", return_value={"id": "78"}): + result = card_description(CFG, card) + assert result == "" + + +def test_card_description_never_mutates_input_card(): + card = {"id": "1", "description": "

hi

"} + before = dict(card) + card_description(CFG, card) + assert card == before + + def test_connect_children_disconnect_children_create_card_have_no_version_header_logic(): """Non-goal preserved: no version header logic is added to any non-card-PATCH endpoint.""" with patch("agileplace.api", return_value={}) as api_mock: diff --git a/tests/test_config_env.py b/tests/test_config_env.py index 893910b..29168c1 100644 --- a/tests/test_config_env.py +++ b/tests/test_config_env.py @@ -86,3 +86,75 @@ def spy_setdefault(key, value): assert "AGILEPLACE_TOKEN" in setdefault_calls # unrelated key still goes through setdefault assert os.environ.get("GH_REPO") == "correct/repo" # untouched, never overwritten assert os.environ.get("GH_HOST") == "correct.example.com" + + +# --- AP_DESCRIPTION_MAX_LENGTH: safe int parse with WARN fallback (issue #65 Task 1) ------------- +# +# description_sync's truncation boundary depends on env_config() always handing back a usable +# positive int here, whatever garbage a .env file or the real environment supplies -- these tests +# pin that env_config() itself never raises and never silently produces a 0/negative ceiling that +# would degrade _truncate_for_agileplace to a marker-only result on every single run. + +def test_default_ap_description_max_length_is_a_positive_int(): + assert isinstance(config.DEFAULT_AP_DESCRIPTION_MAX_LENGTH, int) + assert config.DEFAULT_AP_DESCRIPTION_MAX_LENGTH > 0 + + +def test_env_config_ap_description_max_length_defaults_when_unset(tmp_path, monkeypatch): + monkeypatch.setattr(config, "ENV_FILE", tmp_path / ".env") # no .env file present + monkeypatch.delenv("AP_DESCRIPTION_MAX_LENGTH", raising=False) + + cfg = config.env_config() + + assert cfg["ap_description_max_length"] == config.DEFAULT_AP_DESCRIPTION_MAX_LENGTH + + +def test_env_config_ap_description_max_length_reads_valid_override(tmp_path, monkeypatch): + monkeypatch.setattr(config, "ENV_FILE", tmp_path / ".env") + monkeypatch.setenv("AP_DESCRIPTION_MAX_LENGTH", "5000") + + cfg = config.env_config() + + assert cfg["ap_description_max_length"] == 5000 + + +def test_env_config_ap_description_max_length_falls_back_and_warns_on_non_int(tmp_path, monkeypatch, capsys): + monkeypatch.setattr(config, "ENV_FILE", tmp_path / ".env") + monkeypatch.setenv("AP_DESCRIPTION_MAX_LENGTH", "not-a-number") + + cfg = config.env_config() + + assert cfg["ap_description_max_length"] == config.DEFAULT_AP_DESCRIPTION_MAX_LENGTH + out = capsys.readouterr().out + assert "WARN" in out + assert "AP_DESCRIPTION_MAX_LENGTH" in out + + +def test_env_config_ap_description_max_length_falls_back_and_warns_on_zero(tmp_path, monkeypatch, capsys): + monkeypatch.setattr(config, "ENV_FILE", tmp_path / ".env") + monkeypatch.setenv("AP_DESCRIPTION_MAX_LENGTH", "0") + + cfg = config.env_config() + + assert cfg["ap_description_max_length"] == config.DEFAULT_AP_DESCRIPTION_MAX_LENGTH + assert "WARN" in capsys.readouterr().out + + +def test_env_config_ap_description_max_length_falls_back_and_warns_on_negative(tmp_path, monkeypatch, capsys): + monkeypatch.setattr(config, "ENV_FILE", tmp_path / ".env") + monkeypatch.setenv("AP_DESCRIPTION_MAX_LENGTH", "-1") + + cfg = config.env_config() + + assert cfg["ap_description_max_length"] == config.DEFAULT_AP_DESCRIPTION_MAX_LENGTH + assert "WARN" in capsys.readouterr().out + + +def test_env_config_ap_description_max_length_treats_blank_as_unset(tmp_path, monkeypatch, capsys): + monkeypatch.setattr(config, "ENV_FILE", tmp_path / ".env") + monkeypatch.setenv("AP_DESCRIPTION_MAX_LENGTH", " ") + + cfg = config.env_config() + + assert cfg["ap_description_max_length"] == config.DEFAULT_AP_DESCRIPTION_MAX_LENGTH + assert capsys.readouterr().out == "" # blank is "unset", not a malformed value -- no WARN noise diff --git a/tests/test_ghkit_edit_issue_body.py b/tests/test_ghkit_edit_issue_body.py new file mode 100644 index 0000000..2148c67 --- /dev/null +++ b/tests/test_ghkit_edit_issue_body.py @@ -0,0 +1,186 @@ +"""Unit tests for issue #65 Task 1/7: ghkit.edit_issue_body() (the GitHub-side client-boundary +write for description sync) and list_issues()'s null-safe 'body' field addition. + +edit_issue_body follows the exact dry-run/apply/boundary-validation idiom already established by +create_issue/edit_label: apply=False makes zero calls to run() and returns False; apply=True pipes +the body through run()'s stdin `input=` passthrough (never argv interpolation) and returns True on +success; a malformed `number`/`body` raises ValueError before any I/O. + +Run: pytest -q tests/test_ghkit_edit_issue_body.py +""" +from __future__ import annotations + +import json +import subprocess +import sys +from pathlib import Path +from unittest.mock import Mock + +import pytest + +sys.path.insert(0, str(Path(__file__).resolve().parent.parent)) + +import ghkit # noqa: E402 + + +# --- dry-run: zero calls to the transport boundary, returns False ------------ + +def test_edit_issue_body_dry_run_makes_zero_run_calls_and_returns_false(monkeypatch, capsys): + calls = [] + monkeypatch.setattr(ghkit, "run", lambda *a, **k: calls.append((a, k))) + + result = ghkit.edit_issue_body({}, False, 42, "new body") + + assert result is False + assert calls == [] + assert capsys.readouterr().out.startswith("DRY") + + +def test_edit_issue_body_dry_run_prints_issue_number(monkeypatch, capsys): + monkeypatch.setattr(ghkit, "run", + lambda *a, **k: (_ for _ in ()).throw(AssertionError("must not call run"))) + + ghkit.edit_issue_body({}, False, 42, "body") + + assert "42" in capsys.readouterr().out + + +# --- apply=True: exact argv shape + stdin body passthrough, returns True ----- + +def test_edit_issue_body_apply_calls_run_with_body_file_stdin_and_input_kwarg(monkeypatch): + captured = {} + + def fake_run(cfg, args, **kwargs): + captured["args"] = args + captured["kwargs"] = kwargs + return Mock(stdout="") + + monkeypatch.setattr(ghkit, "run", fake_run) + + result = ghkit.edit_issue_body({}, True, 42, "the new body") + + args = captured["args"] + assert args[:2] == ["issue", "edit"] + assert args[2] == "42" + assert "--body-file" in args and args[args.index("--body-file") + 1] == "-" + assert captured["kwargs"].get("input") == "the new body" + assert result is True + + +def test_edit_issue_body_calls_run_exactly_once(monkeypatch): + calls = [] + + def fake_run(cfg, args, **kwargs): + calls.append((args, kwargs)) + return Mock(stdout="") + + monkeypatch.setattr(ghkit, "run", fake_run) + + ghkit.edit_issue_body({}, True, 42, "body") + + assert len(calls) == 1 + + +def test_edit_issue_body_accepts_empty_string_body(monkeypatch): + """Clearing a description entirely is a legitimate write -- an empty-string body must not be + rejected by the boundary validation.""" + captured = {} + monkeypatch.setattr(ghkit, "run", + lambda cfg, args, **k: (captured.update(k), Mock(stdout=""))[1]) + + result = ghkit.edit_issue_body({}, True, 1, "") + + assert result is True + assert captured.get("input") == "" + + +# --- apply=True: transport failures propagate uncaught ----------------------- + +@pytest.mark.parametrize("exc", [ + subprocess.CalledProcessError(returncode=1, cmd=["gh"]), + subprocess.TimeoutExpired(cmd=["gh"], timeout=60), +]) +def test_edit_issue_body_propagates_run_failures_uncaught(monkeypatch, exc): + def fake_run(cfg, args, **k): + raise exc + + monkeypatch.setattr(ghkit, "run", fake_run) + + with pytest.raises(type(exc)): + ghkit.edit_issue_body({}, True, 42, "body") + + +# --- number/body validated at the boundary, before any I/O ------------------- + +@pytest.mark.parametrize("number", [None, "42", 0, -1, 4.2, True]) +@pytest.mark.parametrize("apply", [True, False]) +def test_edit_issue_body_rejects_an_unusable_number_before_reaching_run(monkeypatch, number, apply): + monkeypatch.setattr(ghkit, "run", lambda *a, **k: (_ for _ in ()).throw( + AssertionError("must not call run for an unusable number"))) + + with pytest.raises(ValueError, match="number"): + ghkit.edit_issue_body({}, apply, number, "body") + + +@pytest.mark.parametrize("body", [None, 42, ["not", "a", "string"]]) +@pytest.mark.parametrize("apply", [True, False]) +def test_edit_issue_body_rejects_a_non_string_body_before_reaching_run(monkeypatch, body, apply): + monkeypatch.setattr(ghkit, "run", lambda *a, **k: (_ for _ in ()).throw( + AssertionError("must not call run for a non-string body"))) + + with pytest.raises(ValueError, match="body"): + ghkit.edit_issue_body({}, apply, 1, body) + + +# --- list_issues(): null-safe 'body' field addition (issue #65 struct #5) ---- + +def _raw_issue(**overrides) -> dict: + base = { + "number": 1, "title": "T", "state": "OPEN", "stateReason": "", + "labels": [], "milestone": None, "assignees": [], + "url": "https://github.com/o/r/issues/1", + } + base.update(overrides) + return base + + +def test_list_issues_includes_body_when_present(monkeypatch): + monkeypatch.setattr(ghkit, "run", + lambda *a, **k: Mock(stdout=json.dumps([_raw_issue(body="the body text")]))) + + issues = ghkit.list_issues({}) + + assert issues[0]["body"] == "the body text" + + +def test_list_issues_normalizes_missing_body_to_empty_string(monkeypatch): + monkeypatch.setattr(ghkit, "run", lambda *a, **k: Mock(stdout=json.dumps([_raw_issue()]))) + + issues = ghkit.list_issues({}) + + assert issues[0]["body"] == "" + + +def test_list_issues_normalizes_null_body_to_empty_string(monkeypatch): + monkeypatch.setattr(ghkit, "run", + lambda *a, **k: Mock(stdout=json.dumps([_raw_issue(body=None)]))) + + issues = ghkit.list_issues({}) + + assert issues[0]["body"] == "" + + +def test_list_issues_requests_body_field_in_gh_json_flag(monkeypatch): + captured = {} + + def fake_run(cfg, args, **kwargs): + captured["args"] = args + return Mock(stdout="[]") + + monkeypatch.setattr(ghkit, "run", fake_run) + + ghkit.list_issues({}) + + json_flag_index = captured["args"].index("--json") + fields = captured["args"][json_flag_index + 1].split(",") + assert "body" in fields From 05b1283b9c181fc2d4b3a46f2cc60f3f9ae6fb35 Mon Sep 17 00:00:00 2001 From: thewrz Date: Wed, 22 Jul 2026 17:24:41 -0700 Subject: [PATCH 02/10] feat(description-sync): add resolve_description merge + canonicalization helpers New pure module description_sync.py implements the GH<->AgilePlace description merge core for issue #65: DescriptionResolution (NamedTuple) and resolve_description(), a 3-way merge over each side's own reference point (desc_base for GitHub, desc_ap_written for AgilePlace so a prior truncation never re-registers as an AP-side edit). Warn-and-skip conflict policy: both sides changing to genuinely different values writes nothing and surfaces a warning; both sides independently converging on the same value is not a conflict. Also adds the two canonicalization helpers with their CORRECTED idempotence invariants from the design spike: _canonicalize_gh_body (genuine md->html->md round trip, self-composition-idempotent) and _canonicalize_ap_description (one-directional html->md, idempotent only through a round trip back through HTML -- feeding its own Markdown output back in as HTML is a type mismatch, not a no-op). Pins all 9 hand-traced resolve_description states (3 seeding variants, steady state, truncated-steady-state, GH-only, AP-only, both-changed-conflict, both-changed-independently-converged) plus None/"" normalization and both canonicalization invariants. Co-Authored-By: Claude Sonnet 5 --- description_sync.py | 107 ++++++++++++++++++++++ tests/test_description_sync.py | 159 +++++++++++++++++++++++++++++++++ 2 files changed, 266 insertions(+) create mode 100644 description_sync.py create mode 100644 tests/test_description_sync.py diff --git a/description_sync.py b/description_sync.py new file mode 100644 index 0000000..f82f716 --- /dev/null +++ b/description_sync.py @@ -0,0 +1,107 @@ +"""Pure GH<->AgilePlace description merge for issue #65. No I/O -- exhaustively unit-tested. + +Two canonicalization helpers put each side's raw text into a stable, comparable "canonical +Markdown" form. richtext's HTML<->Markdown translation absorbs cosmetic round-trip variance (e.g. +`*bold*` vs `_bold_`-equivalent AgilePlace HTML) so it never registers as a spurious edit -- +echo prevention falls out of comparing canonicalized values, never raw ones. `resolve_description` +is the 3-way merge itself: `base`/`ap_written_base` are the last-synced canonical values persisted +in .sync-state.json (see sync.py's issues_state); `gh_canonical`/`ap_canonical` are this run's +freshly canonicalized GitHub issue body and AgilePlace card description. + +Conflict policy is warn-and-skip (issue #65's brainstormed decision, deliberately breaking the +AgilePlace-wins-dates precedent set by sync_dates): when BOTH sides changed since their own +reference point AND landed on genuinely different values, neither side is overwritten -- prose +edits are too costly to silently destroy. When both sides changed but converged on the identical +value (e.g. two people independently typing the same fix), that is not a conflict: there is +nothing to warn about and nothing left to write. +""" +from __future__ import annotations + +from typing import NamedTuple + +import richtext + + +class DescriptionResolution(NamedTuple): + """Result of one resolve_description() call. + + Invariant: `conflict` is True iff `write_gh` and `write_ap` are both False AND `warning` is not + None AND `merged == (base or "")`. A genuine conflict never touches either side and always + reports the OLD agreed value, never a partial or guessed merge. (Steady state also has both + write flags False and `merged == (base or "")` -- `warning is None` is what tells the two + apart.) + """ + merged: str + write_gh: bool + write_ap: bool + conflict: bool + warning: str | None + + +def resolve_description(base: str | None, ap_written_base: str | None, gh_canonical: str, + ap_canonical: str) -> DescriptionResolution: + """3-way merge of a GitHub issue body against an AgilePlace card description, both already + canonicalized (see _canonicalize_gh_body / _canonicalize_ap_description below). Pure, total, + no I/O. + + `base` is the full agreed canonical Markdown as of the last successful sync (None on a + never-synced issue). `ap_written_base` is the canonical of what was actually WRITTEN to the + card last time -- the two differ only when a prior run truncated an oversized description, so + the shorter, truncated text left on the card is compared against its own prior truncated form, + never against the full untruncated base. Without that split, a card still carrying last run's + truncated text would look AP-side-edited on every subsequent run, forever. + + Change is detected independently per side against its own reference point; `None` normalizes + to "" on both sides. Four outcomes: + - neither side changed -> no write; `merged` is the unchanged base (steady state, including + the truncated-steady-state where the card still carries last run's truncated text). + - exactly one side changed -> that side's value propagates to the other; `merged` is the + changed side's canonical text. + - both changed but landed on the SAME value -> no conflict, no write (independently + converged); `merged` becomes that shared value so the base can advance to it. + - both changed to DIFFERENT values -> conflict: neither side is written, `merged` stays the + old base, and `warning` names the conflict for a human to reconcile by hand. + """ + base_norm = base or "" + ap_written_norm = ap_written_base or "" + gh_changed = gh_canonical != base_norm + ap_changed = ap_canonical != ap_written_norm + + if not gh_changed and not ap_changed: + return DescriptionResolution(base_norm, False, False, False, None) + if gh_changed and not ap_changed: + return DescriptionResolution(gh_canonical, False, True, False, None) + if ap_changed and not gh_changed: + return DescriptionResolution(ap_canonical, True, False, False, None) + if gh_canonical == ap_canonical: + return DescriptionResolution(gh_canonical, False, False, False, None) + warning = ("description conflict: both the GitHub issue body and the AgilePlace card " + "description changed since the last sync and now disagree -- leaving both sides " + "untouched until a human reconciles them by hand") + return DescriptionResolution(base_norm, False, False, True, warning) + + +def _canonicalize_gh_body(body: str | None) -> str: + """Canonical Markdown for a GitHub issue body: a genuine md->html->md round trip absorbs + cosmetic Markdown variance (e.g. equivalent emphasis-marker choices) that would otherwise + register as a spurious edit every run. None/missing normalizes to "". + + Self-composition-idempotent: canonical(canonical(x)) == canonical(x). A second round trip + starts from output that is already a fixed point of richtext's md->html->md translation, so + re-canonicalizing it reproduces the same text.""" + return richtext.leankit_html_to_markdown(richtext.markdown_to_leankit_html(body or "")) + + +def _canonicalize_ap_description(description: str | None) -> str: + """Canonical Markdown for an AgilePlace card description, which AgilePlace stores as HTML: one + html->md translation. None/missing normalizes to "". + + NOT self-composition-idempotent: calling this twice in a row would feed its own Markdown + output back in as though it were HTML -- a type mismatch, not a no-op, since richtext's HTML-> + Markdown walker parses stray Markdown punctuation as literal text rather than recognizing it. + The real invariant is a round trip THROUGH HTML: + `_canonicalize_ap_description(html) == + _canonicalize_ap_description(markdown_to_leankit_html(_canonicalize_ap_description(html)))` + -- re-rendering the canonical Markdown back to HTML and canonicalizing THAT reproduces the + same canonical Markdown.""" + return richtext.leankit_html_to_markdown(description or "") diff --git a/tests/test_description_sync.py b/tests/test_description_sync.py new file mode 100644 index 0000000..18b5779 --- /dev/null +++ b/tests/test_description_sync.py @@ -0,0 +1,159 @@ +"""Unit tests for description_sync.py's pure merge (issue #65). No I/O -- pins the full 9-state +resolve_description matrix plus the two canonicalization helpers' (corrected) idempotence +invariants. Run: pytest -q +""" +from __future__ import annotations + +import sys +from pathlib import Path + +import pytest + +sys.path.insert(0, str(Path(__file__).resolve().parent.parent)) + +import richtext # noqa: E402 +from description_sync import ( # noqa: E402 + DescriptionResolution, + _canonicalize_ap_description, + _canonicalize_gh_body, + resolve_description, +) + + +def _assert_invariant(base: str | None, result: DescriptionResolution) -> None: + """The invariant description_sync.py's own docstring states: conflict==True iff write_gh and + write_ap are both False AND warning is not None AND merged == (base or "").""" + conflict_shape = ( + result.write_gh is False and result.write_ap is False + and result.warning is not None and result.merged == (base or "") + ) + assert result.conflict == conflict_shape + + +# ===================================================================================== +# resolve_description -- the 9 hand-traced states +# ===================================================================================== + +def test_seeding_card_empty_pushes_gh_body_to_agileplace(): + result = resolve_description(None, None, "Hello from GitHub", "") + assert result == DescriptionResolution("Hello from GitHub", False, True, False, None) + _assert_invariant(None, result) + + +def test_seeding_gh_body_empty_pulls_card_description_to_github(): + result = resolve_description(None, None, "", "Hello from AgilePlace") + assert result == DescriptionResolution("Hello from AgilePlace", True, False, False, None) + _assert_invariant(None, result) + + +def test_seeding_both_nonempty_and_different_is_a_conflict_that_writes_nothing(): + result = resolve_description(None, None, "GH text", "AP text") + assert result.write_gh is False + assert result.write_ap is False + assert result.conflict is True + assert result.merged == "" + assert result.warning is not None + _assert_invariant(None, result) + + +def test_steady_state_neither_side_changed_writes_nothing(): + result = resolve_description("Same text", "Same text", "Same text", "Same text") + assert result == DescriptionResolution("Same text", False, False, False, None) + _assert_invariant("Same text", result) + + +def test_truncated_steady_state_compares_ap_side_against_its_own_written_form(): + # desc_base is the FULL agreed canonical; desc_ap_written is the canonical of the truncated + # text a prior run actually wrote. Neither side changed since ITS OWN reference point, so this + # must be silent steady state -- comparing ap_canonical against the full base instead would + # wrongly look like an AP-side edit on every subsequent run. + full = "A very long description " * 50 + truncated = full[:100] + "...[truncated by sync]" + result = resolve_description(full, truncated, full, truncated) + assert result == DescriptionResolution(full, False, False, False, None) + _assert_invariant(full, result) + + +def test_gh_only_changed_propagates_to_agileplace(): + result = resolve_description("Old text", "Old text", "New GH text", "Old text") + assert result == DescriptionResolution("New GH text", False, True, False, None) + _assert_invariant("Old text", result) + + +def test_ap_only_changed_propagates_to_github(): + result = resolve_description("Old text", "Old text", "Old text", "New AP text") + assert result == DescriptionResolution("New AP text", True, False, False, None) + _assert_invariant("Old text", result) + + +def test_both_changed_to_different_values_is_a_conflict_that_writes_nothing(): + result = resolve_description("Old text", "Old text", "GH new", "AP new") + assert result.write_gh is False + assert result.write_ap is False + assert result.conflict is True + assert result.merged == "Old text" + assert result.warning is not None + _assert_invariant("Old text", result) + + +def test_both_changed_but_independently_converged_on_the_same_value_is_not_a_conflict(): + result = resolve_description("Old text", "Old text", "Same new text", "Same new text") + assert result == DescriptionResolution("Same new text", False, False, False, None) + _assert_invariant("Old text", result) + + +# ===================================================================================== +# None and "" are indistinguishable inputs on both reference points +# ===================================================================================== + +def test_none_and_empty_string_base_normalize_identically(): + assert resolve_description(None, None, "x", "") == resolve_description("", "", "x", "") + + +def test_none_and_empty_string_ap_written_base_normalize_identically(): + assert (resolve_description("b", None, "b", "y") + == resolve_description("b", "", "b", "y")) + + +# ===================================================================================== +# _canonicalize_gh_body -- genuine md->html->md round trip, self-composition-idempotent +# ===================================================================================== + +def test_canonicalize_gh_body_normalizes_none_and_missing_to_empty_string(): + assert _canonicalize_gh_body(None) == "" + assert _canonicalize_gh_body("") == "" + + +def test_canonicalize_gh_body_is_self_composition_idempotent(): + body = "Some **bold** and *italic* text\n\n- item one\n- item two\n" + once = _canonicalize_gh_body(body) + twice = _canonicalize_gh_body(once) + assert once == twice + + +# ===================================================================================== +# _canonicalize_ap_description -- one-directional html->md, NOT self-composition-idempotent; +# the real invariant is a round trip THROUGH HTML (spike finding #3 correction). +# ===================================================================================== + +def test_canonicalize_ap_description_normalizes_none_and_missing_to_empty_string(): + assert _canonicalize_ap_description(None) == "" + assert _canonicalize_ap_description("") == "" + + +def test_canonicalize_ap_description_round_trips_through_html(): + html = "

Intro bold and italic text with code().

" + once = _canonicalize_ap_description(html) + rerendered_html = richtext.markdown_to_leankit_html(once) + twice = _canonicalize_ap_description(rerendered_html) + assert once == twice + + +def test_canonicalize_ap_description_is_not_self_composition_idempotent(): + # Feeding the function's own Markdown output back in AS IF it were HTML is a type mismatch, + # not a no-op: the HTML->Markdown walker escapes the literal '*' characters it sees as plain + # text, so a naive f(f(x)) != f(x) here -- this is the CORRECTED invariant's whole point. + html = "

Intro bold text.

" + once = _canonicalize_ap_description(html) + naive_twice = _canonicalize_ap_description(once) + assert once != naive_twice From 44c99ad9ad98e9a1b8fa027d9adb2b3873e0ad3c Mon Sep 17 00:00:00 2001 From: thewrz Date: Wed, 22 Jul 2026 17:29:27 -0700 Subject: [PATCH 03/10] feat(description-sync): add binary-search truncation for oversized descriptions Replaces the spike's naive per-word shrink loop (O(n^2), 52+s on a 25k-char body) with a binary search over the markdown cut-length against rendered-HTML length: each candidate is snapped to its preceding whitespace boundary and re-rendered once, giving O(log n) renders regardless of body size (GH issue bodies run up to 65,536 chars). Never negative-length slices; a max_length too small even for TRUNCATION_MARKER degrades to a marker-only result rather than looping forever. Pins: under-limit passthrough, over-limit truncation fitting max_length with TRUNCATION_MARKER appended, graceful degeneration on a tiny max_length, and a wall-clock-bounded test on a >=20k-char body so the O(n^2) regression can never silently creep back in. Part of issue #65 (task 3/7). Co-Authored-By: Claude Sonnet 5 --- description_sync.py | 52 ++++++++++++++++++++++++ tests/test_description_sync.py | 74 ++++++++++++++++++++++++++++++++++ 2 files changed, 126 insertions(+) diff --git a/description_sync.py b/description_sync.py index f82f716..753186f 100644 --- a/description_sync.py +++ b/description_sync.py @@ -21,6 +21,11 @@ import richtext +# Appended to a truncated AgilePlace description so a reader knows the full text lives on GitHub. +# Exact text pinned by the issue #65 design doc -- config.py's DEFAULT_AP_DESCRIPTION_MAX_LENGTH +# docstring references this same constant by name. +TRUNCATION_MARKER = "…[truncated by sync — full text on GitHub]" + class DescriptionResolution(NamedTuple): """Result of one resolve_description() call. @@ -105,3 +110,50 @@ def _canonicalize_ap_description(description: str | None) -> str: -- re-rendering the canonical Markdown back to HTML and canonicalizing THAT reproduces the same canonical Markdown.""" return richtext.leankit_html_to_markdown(description or "") + + +def _snap_to_whitespace_boundary(markdown: str, index: int) -> int: + """Snap a candidate cut `index` down to the nearest whitespace boundary at or before it, so a + truncation cut never splits a word. Clamped to [0, len(markdown)] first, so callers can pass + any binary-search midpoint without pre-validating it. Returns 0 when no whitespace precedes + `index` (e.g. one giant unbroken token) -- a zero-length prefix is a valid degenerate result, + never a negative slice.""" + index = max(0, min(index, len(markdown))) + while index > 0 and not markdown[index - 1].isspace(): + index -= 1 + return index + + +def _truncate_for_agileplace(markdown: str, max_length: int) -> tuple[str, bool]: + """Render `markdown` to AgilePlace HTML, truncating at a clean word boundary with + TRUNCATION_MARKER appended if the render exceeds `max_length` characters. Returns + (html, was_truncated). + + Binary-searches the markdown cut-length against rendered-HTML length (each candidate cut + snapped to its preceding whitespace boundary, re-rendered once, with TRUNCATION_MARKER's + length folded into the same length check) instead of shrinking one word at a time -- O(log n) + renders instead of O(n). That distinction is the difference between milliseconds and 50+ + seconds on a realistically large GH issue body (up to 65,536 chars): a prior per-word shrink + loop re-rendered the whole prefix on every single word removed. + + Never negative-length slices (see _snap_to_whitespace_boundary). A max_length too small to fit + even TRUNCATION_MARKER degrades to a marker-only result rather than looping forever or raising + -- the search floor is always a zero-length markdown prefix, which still renders cleanly.""" + full_html = richtext.markdown_to_leankit_html(markdown) + if len(full_html) <= max_length: + return full_html, False + + lo, hi = 0, len(markdown) + best_cut = 0 + while lo <= hi: + mid = (lo + hi) // 2 + cut = _snap_to_whitespace_boundary(markdown, mid) + candidate_len = len(richtext.markdown_to_leankit_html(markdown[:cut])) + len(TRUNCATION_MARKER) + if candidate_len <= max_length: + best_cut = cut + lo = mid + 1 + else: + hi = mid - 1 + + final_html = richtext.markdown_to_leankit_html(markdown[:best_cut]) + TRUNCATION_MARKER + return final_html, True diff --git a/tests/test_description_sync.py b/tests/test_description_sync.py index 18b5779..b61c5e6 100644 --- a/tests/test_description_sync.py +++ b/tests/test_description_sync.py @@ -5,6 +5,7 @@ from __future__ import annotations import sys +import time from pathlib import Path import pytest @@ -13,9 +14,11 @@ import richtext # noqa: E402 from description_sync import ( # noqa: E402 + TRUNCATION_MARKER, DescriptionResolution, _canonicalize_ap_description, _canonicalize_gh_body, + _truncate_for_agileplace, resolve_description, ) @@ -157,3 +160,74 @@ def test_canonicalize_ap_description_is_not_self_composition_idempotent(): once = _canonicalize_ap_description(html) naive_twice = _canonicalize_ap_description(once) assert once != naive_twice + + +# ===================================================================================== +# _truncate_for_agileplace -- binary-search truncation (spike finding #1: replaces an O(n^2) +# per-word shrink loop that took 52+s on a 25k-char body). Pins: under-limit passthrough, +# over-limit truncation that fits max_length with TRUNCATION_MARKER appended, graceful +# degeneration on a tiny max_length, and an O(log n)-renders wall-clock bound on a >=20k body. +# ===================================================================================== + +def test_truncate_under_limit_returns_full_html_unchanged(): + markdown = "A short description that is well under any reasonable limit." + full_html = richtext.markdown_to_leankit_html(markdown) + html, was_truncated = _truncate_for_agileplace(markdown, max_length=len(full_html) + 100) + assert html == full_html + assert was_truncated is False + + +def test_truncate_exactly_at_limit_returns_full_html_unchanged(): + markdown = "Exactly at the boundary." + full_html = richtext.markdown_to_leankit_html(markdown) + html, was_truncated = _truncate_for_agileplace(markdown, max_length=len(full_html)) + assert html == full_html + assert was_truncated is False + + +def test_truncate_over_limit_fits_max_length_and_appends_marker(): + markdown = " ".join(f"word{i}" for i in range(2000)) + max_length = 500 + html, was_truncated = _truncate_for_agileplace(markdown, max_length=max_length) + assert was_truncated is True + assert len(html) <= max_length + assert html.endswith(TRUNCATION_MARKER) + + +def test_truncate_never_negative_length_slices_on_tiny_input(): + # A single "word" with no whitespace at all -- snapping to a whitespace boundary must degrade + # to an empty prefix rather than slicing with a negative index. + markdown = "x" * 50 + html, was_truncated = _truncate_for_agileplace(markdown, max_length=10) + assert was_truncated is True + assert html.endswith(TRUNCATION_MARKER) + + +def test_truncate_degenerate_tiny_max_length_degrades_to_marker_only(): + markdown = "Several words that would normally survive truncation easily here." + html, was_truncated = _truncate_for_agileplace(markdown, max_length=1) + assert was_truncated is True + # Even a budget smaller than the marker itself must terminate and produce the marker-only + # result, never raise and never loop forever. + assert html == TRUNCATION_MARKER + + +def test_truncate_result_never_exceeds_max_length_when_achievable(): + markdown = "Alpha beta gamma delta epsilon zeta eta theta iota kappa. " * 20 + max_length = 200 + html, was_truncated = _truncate_for_agileplace(markdown, max_length=max_length) + assert was_truncated is True + assert len(html) <= max_length + + +def test_truncate_large_body_completes_in_low_single_digit_seconds(): + # Pins the O(log n) fix: the spike's O(n^2) per-word shrink loop took 52+s on a 25k-char body. + # GH issue bodies run up to 65,536 chars. A handful of renders (~log2(n)) must stay fast. + markdown = ("The quick brown fox jumps over the lazy dog. " * 1500) + assert len(markdown) >= 20_000 + start = time.monotonic() + html, was_truncated = _truncate_for_agileplace(markdown, max_length=5000) + elapsed = time.monotonic() - start + assert was_truncated is True + assert len(html) <= 5000 + assert elapsed < 5.0, f"truncation took {elapsed:.2f}s -- the O(n^2) regression may have returned" From 61572138e2b696af7d12204d13f342faa5da7cc9 Mon Sep 17 00:00:00 2001 From: thewrz Date: Wed, 22 Jul 2026 17:35:21 -0700 Subject: [PATCH 04/10] feat(description-sync): add sync_description wiring + coupled base-advance gate Wires resolve_description's pure merge into GitHub/AgilePlace writes: sync_description(cfg, apply, issue, card, issues_state, queue). Mirrors sync_dates' (issue #6) merge-base contract exactly -- desc_base and desc_ap_written only ever advance together, gated on apply=True AND a confirmed (or unneeded) GitHub-side write, never on the independently-fired AgilePlace queue write. Verified the coupled-gating regression the design step corrected: a naive apply-only gate lets a failed GitHub write silently advance both merge-base fields, which the new tests catch. Co-Authored-By: Claude Sonnet 5 --- description_sync.py | 50 +++++++++++++ tests/test_description_sync.py | 126 +++++++++++++++++++++++++++++++++ 2 files changed, 176 insertions(+) diff --git a/description_sync.py b/description_sync.py index 753186f..19d5c51 100644 --- a/description_sync.py +++ b/description_sync.py @@ -17,9 +17,13 @@ """ from __future__ import annotations +from collections.abc import Callable from typing import NamedTuple +import agileplace +import ghkit import richtext +from stages import issue_custom_id # Appended to a truncated AgilePlace description so a reader knows the full text lives on GitHub. # Exact text pinned by the issue #65 design doc -- config.py's DEFAULT_AP_DESCRIPTION_MAX_LENGTH @@ -157,3 +161,49 @@ def _truncate_for_agileplace(markdown: str, max_length: int) -> tuple[str, bool] final_html = richtext.markdown_to_leankit_html(markdown[:best_cut]) + TRUNCATION_MARKER return final_html, True + + +def sync_description(cfg: dict, apply: bool, issue: dict, card: dict, issues_state: dict, + queue: Callable) -> None: + """Wire resolve_description's pure merge into GitHub/AgilePlace writes, with a coupled + base-advance gate mirroring sync_dates' (issue #6) merge-base contract exactly: `desc_base` and + `desc_ap_written` only ever advance TOGETHER, and only when `apply` is True AND the GitHub-side + write is confirmed (`gh_write_ok`). `gh_write_ok` defaults to True whenever no GitHub write was + needed at all (write_gh is False) -- there is nothing to have failed, same as sync_dates. + + The AgilePlace-side queue write is unconditional, also like sync_dates: it fires whenever + write_ap is True regardless of apply or gh_write_ok, and its eventual success/failure at PATCH- + flush time is not fed back into this gate -- a poisoned card's flush skip already holds the + whole run's state per sync.py's own contract (see sync.py's "poisoned card(s) this run" branch). + A write that is skipped, a dry run, or a failed GitHub write must never advance either field -- + and the two fields never advance independently of one another. + """ + url = issue["url"] + prev = issues_state[url] + key = issue_custom_id(issue) + + gh_canonical = _canonicalize_gh_body(issue.get("body")) + ap_canonical = _canonicalize_ap_description(agileplace.card_description(cfg, card)) + result = resolve_description(prev.get("desc_base"), prev.get("desc_ap_written"), + gh_canonical, ap_canonical) + + if result.conflict: + print(f"WARN [{key}] {result.warning}") + return + + gh_write_ok = True + if result.write_gh: + gh_write_ok = ghkit.edit_issue_body(cfg, apply, issue["number"], result.merged) + + new_ap_written = ap_canonical + if result.write_ap: + html, _ = _truncate_for_agileplace(result.merged, cfg["ap_description_max_length"]) + queue(card, [agileplace.op_description(html)], "description") + new_ap_written = _canonicalize_ap_description(html) + + if result.write_gh or result.write_ap: + print(f"desc [{key}] gh={result.write_gh} ap={result.write_ap}") + + if apply and gh_write_ok: + prev["desc_base"] = result.merged + prev["desc_ap_written"] = new_ap_written diff --git a/tests/test_description_sync.py b/tests/test_description_sync.py index b61c5e6..f431f49 100644 --- a/tests/test_description_sync.py +++ b/tests/test_description_sync.py @@ -7,6 +7,7 @@ import sys import time from pathlib import Path +from unittest.mock import patch import pytest @@ -20,8 +21,33 @@ _canonicalize_gh_body, _truncate_for_agileplace, resolve_description, + sync_description, ) +ISSUE_URL = "https://github.com/acme/repo/issues/1" + + +def _issue(body="", url=ISSUE_URL, number=1, title="[T1] widget"): + return {"number": number, "title": title, "url": url, "body": body} + + +def _card(card_id="C1"): + return {"id": card_id} + + +def _cfg(): + return {"ap_description_max_length": 20000} + + +class _Queue: + """Records every queue(card, ops, note) call for assertions -- same spy shape as + test_sync_dates.py's _Queue.""" + def __init__(self): + self.calls = [] + + def __call__(self, card, ops, note): + self.calls.append((card, ops, note)) + def _assert_invariant(base: str | None, result: DescriptionResolution) -> None: """The invariant description_sync.py's own docstring states: conflict==True iff write_gh and @@ -231,3 +257,103 @@ def test_truncate_large_body_completes_in_low_single_digit_seconds(): assert was_truncated is True assert len(html) <= 5000 assert elapsed < 5.0, f"truncation took {elapsed:.2f}s -- the O(n^2) regression may have returned" + + +# ===================================================================================== +# sync_description -- wiring entry point + coupled base-advance gate (issue #65 task 4). +# +# Mirrors sync_dates' merge-base contract (test_sync_dates.py) exactly: desc_base and +# desc_ap_written only ever advance TOGETHER, and only when apply is True AND the GitHub-side +# write is confirmed (gh_write_ok -- trivially True whenever no GitHub write was needed at all). +# The AgilePlace-side queue write is unconditional -- it fires whenever write_ap is True +# regardless of apply/gh_write_ok, and that firing must never leak into the base-advance gate. +# ===================================================================================== + +def test_sync_description_dry_run_does_not_advance_base_despite_queued_ap_write(): + # gh-changed-only (seeding): write_ap fires and queue is called unconditionally even though + # apply=False -- but the merge base must NOT advance on a dry run. + issue = _issue(body="Hello from GitHub") + card = _card() + state = {ISSUE_URL: {}} + queue = _Queue() + with patch("description_sync.agileplace.card_description", return_value=""), \ + patch("description_sync.agileplace.op_description", return_value="OP") as op_mock, \ + patch("description_sync.ghkit.edit_issue_body") as edit_mock: + sync_description(_cfg(), False, issue, card, state, queue) + op_mock.assert_called_once() + edit_mock.assert_not_called() + assert len(queue.calls) == 1 + assert "desc_base" not in state[ISSUE_URL] + assert "desc_ap_written" not in state[ISSUE_URL] + + +def test_sync_description_failed_gh_write_blocks_base_advance(): + # ap-changed-only (seeding): write_gh fires but the GitHub write is reported as failed/skipped + # -- neither field may advance, even though apply is True. + issue = _issue(body="") + card = _card() + state = {ISSUE_URL: {}} + queue = _Queue() + with patch("description_sync.agileplace.card_description", return_value="Hello from AgilePlace"), \ + patch("description_sync.agileplace.op_description") as op_mock, \ + patch("description_sync.ghkit.edit_issue_body", return_value=False) as edit_mock: + sync_description(_cfg(), True, issue, card, state, queue) + edit_mock.assert_called_once_with(_cfg(), True, issue["number"], "Hello from AgilePlace") + op_mock.assert_not_called() + assert queue.calls == [] + assert "desc_base" not in state[ISSUE_URL] + assert "desc_ap_written" not in state[ISSUE_URL] + + +def test_sync_description_conflict_makes_no_writes_and_no_advance(): + issue = _issue(body="GH text") + card = _card() + state = {ISSUE_URL: {}} + queue = _Queue() + with patch("description_sync.agileplace.card_description", return_value="AP text"), \ + patch("description_sync.agileplace.op_description") as op_mock, \ + patch("description_sync.ghkit.edit_issue_body") as edit_mock: + sync_description(_cfg(), True, issue, card, state, queue) + edit_mock.assert_not_called() + op_mock.assert_not_called() + assert queue.calls == [] + assert "desc_base" not in state[ISSUE_URL] + assert "desc_ap_written" not in state[ISSUE_URL] + + +def test_sync_description_confirmed_gh_write_advances_both_fields_together(): + # ap-changed-only, confirmed GitHub write -> both fields advance to the SAME merged value + # (write_ap never fired here, so desc_ap_written simply reflects the unchanged AP canonical). + issue = _issue(body="") + card = _card() + state = {ISSUE_URL: {}} + queue = _Queue() + with patch("description_sync.agileplace.card_description", return_value="Hello from AgilePlace"), \ + patch("description_sync.agileplace.op_description") as op_mock, \ + patch("description_sync.ghkit.edit_issue_body", return_value=True) as edit_mock: + sync_description(_cfg(), True, issue, card, state, queue) + edit_mock.assert_called_once_with(_cfg(), True, issue["number"], "Hello from AgilePlace") + op_mock.assert_not_called() + assert queue.calls == [] + assert state[ISSUE_URL]["desc_base"] == "Hello from AgilePlace" + assert state[ISSUE_URL]["desc_ap_written"] == "Hello from AgilePlace" + + +def test_sync_description_confirmed_ap_write_advances_both_fields_together(): + # gh-changed-only (seeding), no GitHub write needed at all -> gh_write_ok is trivially True, + # so both fields advance even though the only write that happened was the queued AP op. + issue = _issue(body="Hello from GitHub") + card = _card() + state = {ISSUE_URL: {}} + queue = _Queue() + written_html = richtext.markdown_to_leankit_html("Hello from GitHub") + expected_ap_written = _canonicalize_ap_description(written_html) + with patch("description_sync.agileplace.card_description", return_value=""), \ + patch("description_sync.agileplace.op_description", return_value="OP") as op_mock, \ + patch("description_sync.ghkit.edit_issue_body") as edit_mock: + sync_description(_cfg(), True, issue, card, state, queue) + edit_mock.assert_not_called() + op_mock.assert_called_once_with(written_html) + assert queue.calls == [(card, ["OP"], "description")] + assert state[ISSUE_URL]["desc_base"] == "Hello from GitHub" + assert state[ISSUE_URL]["desc_ap_written"] == expected_ap_written From 70b97fb80c0cc6ff8dddd63c39e2931e1163b7e0 Mon Sep 17 00:00:00 2001 From: thewrz Date: Wed, 22 Jul 2026 17:45:13 -0700 Subject: [PATCH 05/10] feat(sync): wire sync_description into the per-issue loop Adds the single sync_description(cfg, apply, issue, card, issues_state, queue) call right after sync_dates in sync.main()'s per-issue loop, so GitHub/AgilePlace descriptions actually reconcile on every run. Wiring this in surfaced two real regressions (per the design spike's flagged CRITICAL blast-radius, not deferred): - Every pre-existing card fixture across nine test files (test_sync_main, test_vetting_latch, test_run, test_hierarchy_ownership, test_retired_issues, test_sync_card_coherence, test_sync_contested_cards, test_sync_intake_call_site) that reaches the per-issue loop now carries a 'description' key, keeping agileplace.card_description() on its zero-I/O path instead of falling back to a real, unmocked agileplace.get_card() (confirmed live: a real host answers HTTP 401 -> SystemExit). - sync_description now short-circuits a dry-run-only "_planOnly" card to an empty AgilePlace-side description instead of calling card_description(), matching the same convention sync.py's dependency-sync loop already uses for plan-only cards -- a freshly planned card has no server-side description yet, so reading one would GET an id that was never created. tests/test_description_sync_wiring_fixtures.py pins the fixture-safety invariant directly: none of the three most complex wired test files may let a card fixture reach a real agileplace.api() call. Co-Authored-By: Claude Sonnet 5 --- description_sync.py | 11 +++- sync.py | 2 + tests/test_description_sync.py | 24 ++++++++ .../test_description_sync_wiring_fixtures.py | 59 +++++++++++++++++++ tests/test_hierarchy_ownership.py | 3 + tests/test_retired_issues.py | 3 + tests/test_run.py | 6 ++ tests/test_sync_card_coherence.py | 3 + tests/test_sync_contested_cards.py | 3 + tests/test_sync_intake_call_site.py | 8 ++- tests/test_sync_main.py | 11 +++- 11 files changed, 129 insertions(+), 4 deletions(-) create mode 100644 tests/test_description_sync_wiring_fixtures.py diff --git a/description_sync.py b/description_sync.py index 19d5c51..55ad94c 100644 --- a/description_sync.py +++ b/description_sync.py @@ -177,13 +177,22 @@ def sync_description(cfg: dict, apply: bool, issue: dict, card: dict, issues_sta whole run's state per sync.py's own contract (see sync.py's "poisoned card(s) this run" branch). A write that is skipped, a dry run, or a failed GitHub write must never advance either field -- and the two fields never advance independently of one another. + + A dry-run-only planned card (``card["_planOnly"]``) exists only as a synthetic snapshot for + continuing this one dry run -- it has no server-side identity yet, so it never carries a + 'description' key. agileplace.card_description()'s lazy get_card() fallback would otherwise + issue a live GET for an id that was never created on the server. Same convention sync.py's + dependency-sync loop already uses for plan-only cards ("a fresh card has no server-side + dependencies; never read a plan-only id") -- a fresh card likewise has no server-side + description yet, so the AgilePlace side is treated as "" without any network read. """ url = issue["url"] prev = issues_state[url] key = issue_custom_id(issue) gh_canonical = _canonicalize_gh_body(issue.get("body")) - ap_canonical = _canonicalize_ap_description(agileplace.card_description(cfg, card)) + ap_description = "" if card.get("_planOnly") else agileplace.card_description(cfg, card) + ap_canonical = _canonicalize_ap_description(ap_description) result = resolve_description(prev.get("desc_base"), prev.get("desc_ap_written"), gh_canonical, ap_canonical) diff --git a/sync.py b/sync.py index 77c6d85..691816d 100644 --- a/sync.py +++ b/sync.py @@ -28,6 +28,7 @@ from card_coherence import (contested_cards, fence_run_indices, filter_poisoned_edges, laneid_op_value, lane_conflict, poisoned_card_ids, same_card) from config import STATE_FILE, env_config +from description_sync import sync_description from metadata_sync import sync_dates, sync_metadata from stages import (epic_key_for_task, is_retired_issue, issue_custom_id, issue_stage, normalize_status, title_key) @@ -679,6 +680,7 @@ def queue(card, ops, note): sync_metadata(cfg, apply, issue, card, cfg["label_sync_ignore"], issues_state, queue) if field_meta: sync_dates(cfg, apply, issue, card, project_items.get(issue["url"]), field_meta, issues_state, queue) + sync_description(cfg, apply, issue, card, issues_state, queue) # 3) parent/child connections (see sync_child_connections for the full contract). poisoned = poisoned_card_ids(card_ops) diff --git a/tests/test_description_sync.py b/tests/test_description_sync.py index f431f49..fe2fd52 100644 --- a/tests/test_description_sync.py +++ b/tests/test_description_sync.py @@ -357,3 +357,27 @@ def test_sync_description_confirmed_ap_write_advances_both_fields_together(): assert queue.calls == [(card, ["OP"], "description")] assert state[ISSUE_URL]["desc_base"] == "Hello from GitHub" assert state[ISSUE_URL]["desc_ap_written"] == expected_ap_written + + +def test_sync_description_never_reads_a_plan_only_dry_run_card(): + # Discovered running the wiring end-to-end (tests/test_run.py's dry-run path, issue #65 task + # 5/7): a dry-run-only card carries agileplace.create_card's synthetic "_planOnly" marker and + # has no server-side identity yet, so it never has a 'description' key. Without a guard, + # agileplace.card_description()'s lazy get_card() fallback fires a live GET for an id that was + # never created on the server -- exactly the network read sync.py's own dependency-sync loop + # already refuses for plan-only cards ("a fresh card has no server-side dependencies; never + # read a plan-only id"). sync_description must apply that same convention: treat a plan-only + # card's AgilePlace-side description as "" without calling agileplace.card_description() at all. + issue = _issue(body="Hello from GitHub") + card = {**_card(), "_planOnly": True} + state = {ISSUE_URL: {}} + queue = _Queue() + written_html = richtext.markdown_to_leankit_html("Hello from GitHub") + with patch("description_sync.agileplace.card_description") as card_description_mock, \ + patch("description_sync.agileplace.op_description", return_value="OP") as op_mock, \ + patch("description_sync.ghkit.edit_issue_body") as edit_mock: + sync_description(_cfg(), False, issue, card, state, queue) + card_description_mock.assert_not_called() + edit_mock.assert_not_called() + op_mock.assert_called_once_with(written_html) + assert queue.calls == [(card, ["OP"], "description")] diff --git a/tests/test_description_sync_wiring_fixtures.py b/tests/test_description_sync_wiring_fixtures.py new file mode 100644 index 0000000..c8846dc --- /dev/null +++ b/tests/test_description_sync_wiring_fixtures.py @@ -0,0 +1,59 @@ +"""Guards issue #65 task 5's fixture-repair invariant: wiring sync_description into sync.py's +per-issue loop must never let a pre-existing card fixture in test_sync_main.py, +test_vetting_latch.py, or test_run.py fall through to a real agileplace.api() call. + +agileplace.card_description() takes a zero-I/O path whenever the card dict already carries a +'description' key (even ""); a fixture that omits it makes sync_description's +card_description(cfg, card) call fall back to agileplace.get_card(), which none of these three +files mock -- so it hits the real HTTP client. Confirmed live in the design spike: a real host +answers with HTTP 401 -> SystemExit inside test_sync_main.py/test_vetting_latch.py (which patch +agileplace at the function level, not the transport), and test_run.py's own request-dispatch stub +(FixtureWorld.open_url) raises its own AssertionError("unexpected AgilePlace request") for the +same reason -- neither is a legitimate assertion failure, both are this exact regression. + +Rather than re-deriving by static analysis which fixtures reach the per-issue loop (fragile across +three differently-shaped harnesses -- test_sync_main.py's shared _mock_io, test_vetting_latch.py's +reuse of it, test_run.py's own FixtureWorld HTTP stub), this runs each file as its own subprocess +and requires zero failures/errors: the same boundary the spike's regression was actually caught at. + +Run: pytest -q tests/test_description_sync_wiring_fixtures.py +""" +from __future__ import annotations + +import subprocess +import sys +from pathlib import Path + +REPO_ROOT = Path(__file__).resolve().parent.parent + +# The three files task 5/7 is scoped to repair -- every pre-existing card fixture reaching +# sync.main()'s per-issue loop lives in one of these. +WIRED_TEST_FILES = ( + "tests/test_sync_main.py", + "tests/test_vetting_latch.py", + "tests/test_run.py", +) + + +def test_no_wired_test_file_lets_a_card_fixture_reach_real_agileplace_api(): + failures = [] + for relative_path in WIRED_TEST_FILES: + result = subprocess.run( + [sys.executable, "-m", "pytest", "-q", relative_path], + cwd=REPO_ROOT, + capture_output=True, + text=True, + timeout=120, + ) + if result.returncode != 0: + summary = result.stdout.strip().splitlines()[-1] if result.stdout.strip() else "" + failures.append( + f"{relative_path}: exit={result.returncode} summary={summary!r}\n" + f"{result.stdout[-3000:]}" + ) + + assert not failures, ( + "one or more wired test files let a card fixture reach sync_description's " + "agileplace.card_description() fallback (real agileplace.api() call) -- give the fixture " + "a 'description' key or explicitly mock agileplace.get_card:\n\n" + "\n\n".join(failures) + ) diff --git a/tests/test_hierarchy_ownership.py b/tests/test_hierarchy_ownership.py index 035c749..f8e1a49 100644 --- a/tests/test_hierarchy_ownership.py +++ b/tests/test_hierarchy_ownership.py @@ -39,6 +39,8 @@ def _card(number: int, custom_id: str, *, url: str | None = None, "laneId": None, "blockedStatus": {"isBlocked": False, "reason": ""}, "childCards": [{"id": child_id} for child_id in children], + # issue #65: keeps agileplace.card_description() on its zero-I/O path. + "description": "", } return {**card, **({"externalLink": {"url": url}} if url else {})} @@ -52,6 +54,7 @@ def _config(tmp_path: Path) -> dict: "label_sync_ignore": frozenset(), "stage_lane_map": {}, "gh_project": {}, + "ap_description_max_length": 20000, # issue #65: sync_description reads this unconditionally } diff --git a/tests/test_retired_issues.py b/tests/test_retired_issues.py index c1b49fc..6baa506 100644 --- a/tests/test_retired_issues.py +++ b/tests/test_retired_issues.py @@ -39,6 +39,7 @@ def _config(tmp_path) -> dict: "label_sync_ignore": frozenset(), "stage_lane_map": {}, "gh_project": {}, + "ap_description_max_length": 20000, # issue #65: sync_description reads this unconditionally } @@ -86,6 +87,8 @@ def _card(number: int, lane_id: str, *, blocked: bool) -> dict: "blockedStatus": {"isBlocked": blocked, "reason": "Blocked by #10" if blocked else ""}, "plannedStart": None, "plannedFinish": None, + # issue #65: keeps agileplace.card_description() on its zero-I/O path. + "description": "", } diff --git a/tests/test_run.py b/tests/test_run.py index 98dd50d..906c220 100644 --- a/tests/test_run.py +++ b/tests/test_run.py @@ -119,6 +119,10 @@ def __init__(self): "plannedFinish": None, "blockedStatus": {"isBlocked": False, "reason": ""}, "childCards": [], + # issue #65: keeps agileplace.card_description() on its zero-I/O path -- without this + # key it falls back to the real (unmocked) GET card/{id}, which this fixture's own + # open_url dispatch doesn't expect and raises "unexpected AgilePlace request". + "description": "", } def run_process(self, argv, **_kwargs): @@ -200,6 +204,7 @@ def open_url(self, request, **_kwargs): "plannedStart": None, "plannedFinish": None, "blockedStatus": {"isBlocked": False, "reason": ""}, + "description": "", # issue #65: same zero-I/O contract as epic_card above }, }) if method in {"POST", "DELETE"} and path == "card/connections": @@ -411,6 +416,7 @@ def __init__(self): "plannedFinish": None, "blockedStatus": {"isBlocked": False, "reason": ""}, "childCards": [], + "description": "", # issue #65: same zero-I/O contract as FixtureWorld.epic_card } def run_process(self, argv, **kwargs): diff --git a/tests/test_sync_card_coherence.py b/tests/test_sync_card_coherence.py index 0cacb26..c0c5938 100644 --- a/tests/test_sync_card_coherence.py +++ b/tests/test_sync_card_coherence.py @@ -84,6 +84,8 @@ def _card(card_id: str, custom_id: str, urls: list[str], *, lane_id: str = "L-EL "tags": [], "plannedStart": None, "plannedFinish": None, + # issue #65: keeps agileplace.card_description() on its zero-I/O path. + "description": "", } @@ -96,6 +98,7 @@ def _cfg(tmp_path) -> dict: "label_sync_ignore": frozenset(), "stage_lane_map": {}, "gh_project": {}, + "ap_description_max_length": 20000, # issue #65: sync_description reads this unconditionally } diff --git a/tests/test_sync_contested_cards.py b/tests/test_sync_contested_cards.py index 46e15e9..01cd0ae 100644 --- a/tests/test_sync_contested_cards.py +++ b/tests/test_sync_contested_cards.py @@ -45,6 +45,7 @@ def _config(tmp_path) -> dict: "label_sync_ignore": frozenset(), "stage_lane_map": {}, "gh_project": {}, + "ap_description_max_length": 20000, # issue #65: sync_description reads this unconditionally } @@ -60,6 +61,8 @@ def _card_with_urls(card_id: str, custom_id: str, urls: list[str], *, lane_id: s "tags": [], "plannedStart": None, "plannedFinish": None, + # issue #65: keeps agileplace.card_description() on its zero-I/O path. + "description": "", } diff --git a/tests/test_sync_intake_call_site.py b/tests/test_sync_intake_call_site.py index 2eb9065..82a150b 100644 --- a/tests/test_sync_intake_call_site.py +++ b/tests/test_sync_intake_call_site.py @@ -39,9 +39,11 @@ def _issue(): def _card(): + # "description": "" (issue #65) keeps agileplace.card_description() on its zero-I/O path. return {"id": "C1", "version": 1, "customId": "1", "externalLink": {"url": ISSUE_URL}, "tags": [], - "plannedStart": None, "plannedFinish": None, "laneId": "LANE1"} + "plannedStart": None, "plannedFinish": None, "laneId": "LANE1", + "description": ""} def _cfg(tmp_path): @@ -52,6 +54,7 @@ def _cfg(tmp_path): "stage_lane_map": {"Intake": ["New Requests"]}, "gh_project": {"owner": "acme", "number": "7", "status_field": "Status", "start_field": "Start", "target_field": "Target"}, + "ap_description_max_length": 20000, # issue #65: sync_description reads this unconditionally } @@ -110,7 +113,8 @@ def test_main_does_not_create_a_duplicate_card_for_a_resumed_active_issue(tmp_pa active_url = "https://github.com/acme/repo/issues/7" active_issue = {"number": 7, "title": "Raw idea", "state": "OPEN", "labels": [], "milestone": None, "assignees": [], "url": active_url} - intake_card = {"id": "C-intake", "version": 1, "laneId": "lane-intake", "title": "Raw idea"} + intake_card = {"id": "C-intake", "version": 1, "laneId": "lane-intake", "title": "Raw idea", + "description": ""} # issue #65: this card reaches the per-issue loop below intake_lane = {"id": "lane-intake", "title": "New Requests"} cfg = {**_sync_main_cfg(tmp_path), "stage_lane_map": {"Intake": ["New Requests"]}} state_file = tmp_path / ".sync-state.json" diff --git a/tests/test_sync_main.py b/tests/test_sync_main.py index db4e70a..025de73 100644 --- a/tests/test_sync_main.py +++ b/tests/test_sync_main.py @@ -37,9 +37,13 @@ def _issue(): def _card(): + # "description": "" (issue #65) keeps agileplace.card_description() on its zero-I/O path -- + # without the key it falls back to the real (unmocked) agileplace.get_card(), which hits the + # live HTTP client and SystemExits (confirmed live in the design spike). return {"id": "C1", "version": 1, "customId": "1", "externalLink": {"url": ISSUE_URL}, "tags": [], - "plannedStart": "2026-02-01", "plannedFinish": None, "laneId": None} + "plannedStart": "2026-02-01", "plannedFinish": None, "laneId": None, + "description": ""} def _field_meta(): @@ -55,6 +59,7 @@ def _cfg(tmp_path): "stage_lane_map": {}, "gh_project": {"owner": "acme", "number": "7", "status_field": "Status", "start_field": "Start", "target_field": "Target"}, + "ap_description_max_length": 20000, # issue #65: sync_description reads this unconditionally } @@ -210,6 +215,10 @@ def test_legacy_state_resets_merge_base_before_relearning_live_metadata(tmp_path assert state["issues"][ISSUE_URL] == { "card_id": "C1", "labels": ["bug"], "milestone": "1.0", "start": "2026-02-01", "target": None, + # issue #65: sync_description's own additive merge-base keys, advanced together since + # both the issue body and the card description are "" here (no GitHub write needed, so + # gh_write_ok is trivially True) -- unrelated to this test's own labels/dates focus. + "desc_base": "", "desc_ap_written": "", } run_mock.assert_called_once() assert run_mock.call_args.args[1][:2] == ["project", "item-edit"] From 47cad3d21008796a356118e2be5e1c4edebec9da Mon Sep 17 00:00:00 2001 From: thewrz Date: Wed, 22 Jul 2026 17:48:19 -0700 Subject: [PATCH 06/10] test(sync): pin sync.main()'s per-issue sync_description call site Task 6/7 of issue #65: a dedicated, thin wiring test (mirroring test_sync_intake_call_site.py's convention of monkeypatching the collaborator rather than duplicating its own behavior tests) asserting main() calls description_sync.sync_description() exactly once per syncable issue with the expected positional args (cfg, apply, issue, card, issues_state, queue), and that the unresolved-card `continue` guard skips it entirely when no card matches this run. Re-verified test_regression_budget.py: sync.py stays at 728 lines (well within the 726 + 40-line wiring budget and the 800-line hard cap), so no re-anchor of PRE_CHANGE_SYNC_LINES was needed. Co-Authored-By: Claude Sonnet 5 --- tests/test_sync_description_call_site.py | 85 ++++++++++++++++++++++++ 1 file changed, 85 insertions(+) create mode 100644 tests/test_sync_description_call_site.py diff --git a/tests/test_sync_description_call_site.py b/tests/test_sync_description_call_site.py new file mode 100644 index 0000000..ed14649 --- /dev/null +++ b/tests/test_sync_description_call_site.py @@ -0,0 +1,85 @@ +"""sync.main()'s description-sync call site (Task 6/7, issue #65). + +Thin, isolated from the large test_sync_main.py suite: this file's only concern is that main()'s +per-issue loop actually calls description_sync.sync_description() with the right positional +arguments (cfg, apply, issue, card, issues_state, queue) -- once per syncable issue, after the +lane/metadata/dates steps for that issue. It does not re-verify sync_description()'s own merge/ +truncation/conflict behavior -- that is tests/test_description_sync.py's job -- so +sync_description itself is monkeypatched here, matching the low-level-transport-boundary +convention this repo's other main()-level call-site tests use (test_sync_intake_call_site.py +patches intake.promote() the same way; patching the collaborator, not its internals). + +sync.py imports the name directly (`from description_sync import sync_description`), so the call +site lives in sync's own namespace -- the patch target is "sync.sync_description", not +"description_sync.sync_description". + +Run: pytest -q tests/test_sync_description_call_site.py +""" +from __future__ import annotations + +import sys +from pathlib import Path +from unittest.mock import patch + +sys.path.insert(0, str(Path(__file__).resolve().parent.parent)) + +import sync # noqa: E402 + +# _mock_io/_card/_cfg are test_sync_main.py's richer I/O-boundary-mocking helpers -- reused here +# rather than duplicated, matching test_sync_intake_call_site.py's own precedent. +from test_sync_main import _card, _cfg, _issue, _mock_io # noqa: E402 + +ISSUE_URL = "https://github.com/acme/repo/issues/1" + + +def test_main_calls_sync_description_once_per_issue_with_expected_args(tmp_path): + state_file = tmp_path / ".sync-state.json" + cfg = _cfg(tmp_path) + issue = _issue() + card = _card() + stack, _run_mock, _patch_card_mock, _create_card_mock = _mock_io( + card, ({}, []), field_meta_return=None) + sync_description_mock = stack.enter_context(patch("sync.sync_description")) + + with stack, patch("sync.env_config", return_value=cfg), patch("sync.STATE_FILE", state_file), \ + patch("sys.argv", ["sync.py", "--apply"]): + sync.main() + + sync_description_mock.assert_called_once() + call_cfg, call_apply, call_issue, call_card, call_issues_state, call_queue = ( + sync_description_mock.call_args.args) + assert call_cfg is cfg + assert call_apply is True + # main() annotates has_open_pr onto each issue (issue #14, pre-#65) before the per-issue loop, + # so the issue sync_description receives carries that field too -- same precedent + # test_sync_intake_call_site.py's own comment documents for intake.promote()'s issues arg. + assert call_issue == {**issue, "has_open_pr": False} + assert call_card["id"] == card["id"] + # sync_metadata (labels/milestone) runs on this same issue immediately before sync_description + # in the per-issue loop, so by the time sync_description is called issues_state already carries + # the keys sync_metadata itself learned this run -- this call site doesn't own those keys, it + # only asserts sync_description sees the same live per-issue state dict main() threads through. + assert call_issues_state == { + ISSUE_URL: {"card_id": "C1", "labels": [], "milestone": None}, + } + assert callable(call_queue) + + +def test_main_never_reaches_real_sync_description_for_an_unresolved_card(tmp_path): + """A syncable issue with no matching/created card this run (card_for(issue) is falsy) must + `continue` before reaching sync_description -- mirrors the existing lane/metadata/dates guard + a few lines above the call site.""" + state_file = tmp_path / ".sync-state.json" + cfg = _cfg(tmp_path) + stack, _run_mock, _patch_card_mock, _create_card_mock = _mock_io( + _card(), ({}, []), field_meta_return=None, existing_cards=[]) + sync_description_mock = stack.enter_context(patch("sync.sync_description")) + # No card matches this issue and creation is suppressed by patching create_card to return {} + # (no "id"), so card_for(issue) stays falsy and the per-issue loop's `continue` guard fires + # before ever reaching sync_description. + + with stack, patch("sync.env_config", return_value=cfg), patch("sync.STATE_FILE", state_file), \ + patch("sys.argv", ["sync.py", "--apply"]): + sync.main() + + sync_description_mock.assert_not_called() From c7c23ce2ab1010315e2cb9be81897d5a746cc664 Mon Sep 17 00:00:00 2001 From: thewrz Date: Wed, 22 Jul 2026 17:54:02 -0700 Subject: [PATCH 07/10] feat(smoke): add description write + length probe steps (issue #65) Exercise agileplace.op_description's exact write shape through the same versioned-PATCH path (patch_card) the sync uses, plus a fact-finding probe of the configured ap_description_max_length against the live server, per the issue #65 design doc's smoke-script validation plan. Flags the untested replace-vs-remove uncertainty for clearing a description (design decision #7) in a code comment rather than silently assuming "replace" covers that case too. Updates test_smoke.py's fake AgilePlace tenant to model the /description PATCH path and the exact confirmed-run write sequence, keeping the existing offline smoke suite green. Co-Authored-By: Claude Sonnet 5 --- smoke.py | 49 ++++++++++++++++++++++++++++++++++++++++++--- tests/test_smoke.py | 10 +++++++-- 2 files changed, 54 insertions(+), 5 deletions(-) diff --git a/smoke.py b/smoke.py index 20ed1e8..057ede9 100644 --- a/smoke.py +++ b/smoke.py @@ -3,9 +3,9 @@ Reads .env exactly like sync.py, previews the target board (title, lanes, existing cards), and only after an explicit confirmation creates two clearly-marked throwaway cards, exercises every write shape the sync uses -- card create, versioned PATCH tag add/remove, externalLink add on a bare card, -connect/disconnect children, and a deliberately stale-version PATCH that MUST be rejected -- then -deletes both cards and confirms they are gone. The steps mirror the ``[live-check]`` items in -API-VALIDATION.md so one confirmed run retires them. +connect/disconnect children, description write + length probe (issue #65), and a deliberately +stale-version PATCH that MUST be rejected -- then deletes both cards and confirms they are gone. The +steps mirror the ``[live-check]`` items in API-VALIDATION.md so one confirmed run retires them. GitHub is never touched (dry run already exercises every gh read live), and the sync state file is never read or written. Every server rejection is printed with its full, untruncated response body so @@ -280,6 +280,48 @@ def _check_dependencies(cfg: dict, parent_id: str, child_id: str, results: list) f"incoming read back: {sorted(incoming) if incoming is not None else 'unavailable'}")) +def _check_description(cfg: dict, parent_id: str, results: list) -> None: + """Steps 14-15: agileplace.op_description's exact write shape through the same versioned-PATCH + path (patch_card) the sync itself uses (description_sync.sync_description), then a + fact-finding probe of the configured ap_description_max_length against the live server (the + issue #65 design doc's "max-length probe": cross-check config.DEFAULT_AP_DESCRIPTION_MAX_LENGTH + -- or its .env override -- against what the real API actually accepts). + + NOTE -- untested replace-vs-remove uncertainty (issue #65 design decision #7): op_description + always issues an RFC-6902 "replace", including when description_sync writes "" to clear a + card's description. Other fields that need clearing (planned dates, blocked-state) use a + dedicated "remove" op instead of a replace-to-null/empty -- see steps 5-6's live-validated + contract above -- but whether AgilePlace's PATCH endpoint honors a replace-to-"" the same way + for /description has never been checked against the real API. This step only validates a + non-empty replace round-trips correctly; it deliberately does NOT exercise clearing to "", so + that uncertainty stays open (tracked in the design doc) until a live run confirms one way or + the other.""" + _step(14, "description write via op_description through patch_card") + fresh = agileplace.get_card(cfg, parent_id) + html = "

SMOKE description write

" + agileplace.patch_card(cfg, True, fresh, [agileplace.op_description(html)]) + readback = agileplace.card_description(cfg, agileplace.get_card(cfg, parent_id)) + results.append(("description write round-trip (op_description via patch_card)", + readback == html, f"description now {readback!r}")) + + _step(15, "description length probe (fact-finding, not pass/fail)") + limit = cfg["ap_description_max_length"] + probe_html = f"

{'x' * limit}

" + fresh = agileplace.get_card(cfg, parent_id) + try: + agileplace.patch_card(cfg, True, fresh, [agileplace.op_description(probe_html)]) + except SystemExit as exc: + _print_http_failure(exc) + results.append(("description length probe", None, + f"server REJECTED a {len(probe_html)}-char description " + f"(configured max_length={limit}): {exc}")) + return + readback = agileplace.card_description(cfg, agileplace.get_card(cfg, parent_id)) + results.append(("description length probe", None, + f"sent {len(probe_html)} chars, server stored {len(readback)} chars back " + f"(configured max_length={limit})")) + + def _check_stale_patch(cfg: dict, parent_id: str, stale_version: str, results: list) -> None: """Step 13: a stale x-lk-resource-version PATCH must be rejected, not silently applied.""" _step(13, "deliberately stale-version PATCH (the server MUST reject this)") @@ -338,6 +380,7 @@ def _run_checks(cfg: dict, lane_id: str | None, run_id: str, created: list[str], child_id = _check_child_and_link(cfg, lane_id, CHILD_CUSTOM_ID_PREFIX + run_id, created, results) _check_connections(cfg, parent_id, child_id, results) _check_dependencies(cfg, parent_id, child_id, results) + _check_description(cfg, parent_id, results) _check_stale_patch(cfg, parent_id, baseline_version, results) diff --git a/tests/test_smoke.py b/tests/test_smoke.py index 2b9d856..70f9890 100644 --- a/tests/test_smoke.py +++ b/tests/test_smoke.py @@ -54,7 +54,7 @@ def __init__(self, *, accept_stale: bool = False, fail_child_create_body: str | self.writes: list[tuple[str, str]] = [] self.cards: dict[str, dict] = { "P1": {"id": "P1", "title": "Existing card", "customId": "EX-1", - "laneId": "L1", "tags": [], "version": 3}, + "laneId": "L1", "tags": [], "version": 3, "description": ""}, } self.children: dict[str, list[str]] = {} self.dependencies: dict[str, list[dict]] = {} @@ -114,7 +114,7 @@ def _create(self, url: str, body: dict): self.created_custom_ids.append(body["customId"]) card = {"id": card_id, "title": body["title"], "customId": body["customId"], "laneId": body.get("laneId"), "tags": [], "version": 1, - "plannedStart": None, "plannedFinish": None, + "plannedStart": None, "plannedFinish": None, "description": "", "blockedStatus": {"isBlocked": False, "reason": ""}} if "externalLink" in body: card["externalLink"] = body["externalLink"] @@ -151,6 +151,8 @@ def _patch(self, req, card_id: str, ops: list): elif op["path"] == "/externalLink": if not self.ignore_external_link: card["externalLink"] = op["value"] + elif op["path"] == "/description": + card["description"] = op["value"] elif op["path"] == "/isBlocked": card["blockedStatus"]["isBlocked"] = op["value"] elif op["path"] == "/blockReason": @@ -249,6 +251,8 @@ def test_confirmed_run_executes_whole_sequence_and_cleans_up(tenant_env, capsys) ("POST", "card/dependency"), # dependency create (child dependsOn parent) ("POST", "card/dependency"), # duplicate-create fact-finding probe ("DELETE", "card/dependency"), # dependency delete + ("PATCH", "card/S1"), # description write round-trip + ("PATCH", "card/S1"), # description length probe ("PATCH", "card/S1"), # deliberate stale-version probe ("DELETE", "card/S2"), # cleanup child ("DELETE", "card/S1"), # cleanup parent @@ -263,6 +267,8 @@ def test_confirmed_run_executes_whole_sequence_and_cleans_up(tenant_env, capsys) assert "planned" in out # planned-date round-trip reported assert "duplicate create rejected" in out # the live 409 contract, mirrored by the double assert "Dependency already exists" in out + assert "description write round-trip" in out + assert "description length probe" in out def test_unexpected_duplicate_create_failure_fails_the_run(tenant_env, capsys): From ae6d37a208427811364da14d716e58e9cad12fb4 Mon Sep 17 00:00:00 2001 From: thewrz Date: Wed, 22 Jul 2026 18:11:36 -0700 Subject: [PATCH 08/10] fix(description-sync): stop truncated-card edits from destroying the full GH body Critical: resolve_description's "AP changed, GH unchanged" branch always promoted ap_canonical straight to `merged` and pushed it to GitHub. When a prior run had truncated an oversized description for the AgilePlace card (desc_ap_written diverging from the full desc_base), any further edit on top of that truncated stub was blindly treated as the whole authoritative body -- silently overwriting the GitHub issue with the short truncated+marker stub and permanently losing the untruncated tail, with conflict=False so no human ever got a chance to intervene. Detect the truncated-stand-in case (ap_written diverges from base) and degrade it to the same warn-and-skip conflict outcome as a genuine both-sides conflict instead of a blind overwrite. Medium: agileplace.py was already over the 800-line file cap on this branch's base; this PR's new card_description()/op_description() helpers pushed it further over. Extracted both into a new agileplace_description.py module (and their tests into tests/test_agileplace_description.py), returning agileplace.py to its pre-PR line count instead of growing an already-over- budget file. Low: the O(log n)-renders truncation test asserted a 5s wall-clock ceiling, which is only a proxy for the render-count invariant the code actually claims -- a coarser-but-still-polynomial regression could still pass under a generous time budget, and a correct implementation could flake on slow CI. Replaced it with a test that counts real calls to richtext.markdown_to_leankit_html and bounds them logarithmically. Co-Authored-By: Claude Sonnet 5 --- agileplace.py | 21 ----- agileplace_description.py | 32 +++++++ description_sync.py | 38 ++++++-- smoke.py | 11 +-- tests/test_agileplace.py | 63 +------------ tests/test_agileplace_description.py | 72 +++++++++++++++ tests/test_description_sync.py | 90 ++++++++++++++----- .../test_description_sync_wiring_fixtures.py | 4 +- tests/test_hierarchy_ownership.py | 2 +- tests/test_retired_issues.py | 2 +- tests/test_run.py | 2 +- tests/test_sync_card_coherence.py | 2 +- tests/test_sync_contested_cards.py | 2 +- tests/test_sync_intake_call_site.py | 2 +- tests/test_sync_main.py | 2 +- 15 files changed, 218 insertions(+), 127 deletions(-) create mode 100644 agileplace_description.py create mode 100644 tests/test_agileplace_description.py diff --git a/agileplace.py b/agileplace.py index d219294..eb1eab5 100644 --- a/agileplace.py +++ b/agileplace.py @@ -332,19 +332,6 @@ def get_card(cfg: dict, card_id: str) -> dict: return card -def card_description(cfg: dict, card: dict) -> str: - """The card's description text, normalized to "" for None/absent. list_cards() never returns - `description` (no field-selection params sent, confirmed against the live board), so a card - reaching this function via the board snapshot is almost always missing the key entirely -- that - (and ONLY that) triggers one lazy get_card refetch. A card that DOES carry the key -- including - description="" -- takes the zero-I/O path: an explicit empty string is a real, current - description, not "unknown", and must never be treated as a reason to hit the network.""" - if "description" in card: - return card["description"] or "" - fresh = get_card(cfg, card["id"]) - return fresh.get("description") or "" - - def _warn_card_field(card: dict, detail: str) -> None: print(f"WARN card {card.get('id', '')} {detail} -- skipping malformed value") @@ -493,14 +480,6 @@ def op_custom_id(custom_id: str) -> dict: return {"op": "replace", "path": "/customId", "value": custom_id} -def op_description(html: str) -> dict: - """A single unconditional replace of the card's description with `html` (already rendered by - richtext.markdown_to_leankit_html and, when oversized, truncated by - description_sync._truncate_for_agileplace). No validation here -- description_sync owns the - decision of *whether* to write; this is purely the op-builder, matching op_custom_id's shape.""" - return {"op": "replace", "path": "/description", "value": html} - - def op_lane(lane_id: str) -> dict: return {"op": "replace", "path": "/laneId", "value": lane_id} diff --git a/agileplace_description.py b/agileplace_description.py new file mode 100644 index 0000000..17902eb --- /dev/null +++ b/agileplace_description.py @@ -0,0 +1,32 @@ +"""AgilePlace card-description read/write helpers (issue #65 Task 1/7). + +Split out of agileplace.py -- rather than growing that module further -- because it was already at +the project's 800-line file cap on this branch's base (code.md: "never add to a file already over +budget -- extract first"). Depends on agileplace.get_card for the lazy-refetch fallback in +card_description(); agileplace.py has no dependency back on this module, so there is no cycle. +""" +from __future__ import annotations + +import agileplace + + +def card_description(cfg: dict, card: dict) -> str: + """The card's description text, normalized to "" for None/absent. list_cards() never returns + `description` (no field-selection params sent, confirmed against the live board), so a card + reaching this function via the board snapshot is almost always missing the key entirely -- that + (and ONLY that) triggers one lazy get_card refetch. A card that DOES carry the key -- including + description="" -- takes the zero-I/O path: an explicit empty string is a real, current + description, not "unknown", and must never be treated as a reason to hit the network.""" + if "description" in card: + return card["description"] or "" + fresh = agileplace.get_card(cfg, card["id"]) + return fresh.get("description") or "" + + +def op_description(html: str) -> dict: + """A single unconditional replace of the card's description with `html` (already rendered by + richtext.markdown_to_leankit_html and, when oversized, truncated by + description_sync._truncate_for_agileplace). No validation here -- description_sync owns the + decision of *whether* to write; this is purely the op-builder, matching agileplace.op_custom_id's + shape.""" + return {"op": "replace", "path": "/description", "value": html} diff --git a/description_sync.py b/description_sync.py index 55ad94c..fac0538 100644 --- a/description_sync.py +++ b/description_sync.py @@ -20,7 +20,7 @@ from collections.abc import Callable from typing import NamedTuple -import agileplace +import agileplace_description import ghkit import richtext from stages import issue_custom_id @@ -61,11 +61,19 @@ def resolve_description(base: str | None, ap_written_base: str | None, gh_canoni truncated text would look AP-side-edited on every subsequent run, forever. Change is detected independently per side against its own reference point; `None` normalizes - to "" on both sides. Four outcomes: + to "" on both sides. Five outcomes: - neither side changed -> no write; `merged` is the unchanged base (steady state, including the truncated-steady-state where the card still carries last run's truncated text). - - exactly one side changed -> that side's value propagates to the other; `merged` is the - changed side's canonical text. + - only the GH side changed -> propagates to AgilePlace; `merged` is the GH canonical text. + - only the AP side changed, AND `ap_written_base` already equalled `base` (the card carried + the FULL text, never a truncated stand-in) -> propagates to GitHub; `merged` is the AP + canonical text. + - only the AP side changed, but `ap_written_base` diverges from `base` (the card has only + ever carried a truncated stand-in for the full body) -> treated as unresolvable: the edit + touches only the visible truncated portion, never the untruncated tail that lives solely + in `base`, so promoting it to GitHub would silently destroy that tail. Degrades to the same + conflict outcome as both-sides-changed: neither side is written, `merged` stays the old + base, and `warning` names the issue for a human to reconcile by hand. - both changed but landed on the SAME value -> no conflict, no write (independently converged); `merged` becomes that shared value so the base can advance to it. - both changed to DIFFERENT values -> conflict: neither side is written, `merged` stays the @@ -81,6 +89,19 @@ def resolve_description(base: str | None, ap_written_base: str | None, gh_canoni if gh_changed and not ap_changed: return DescriptionResolution(gh_canonical, False, True, False, None) if ap_changed and not gh_changed: + if ap_written_norm != base_norm: + # ap_written_base diverges from base ONLY when a prior run truncated the full body for + # the card (see _truncate_for_agileplace) -- the card has never carried anything but a + # truncated stand-in. An edit on top of that stub only ever touches the visible + # truncated portion, never the untruncated tail that lives solely in `base`. Blindly + # promoting ap_canonical to `merged` here would push that short stub to GitHub and + # permanently destroy the lost tail with no warning. Degrade to the same warn-and-skip + # conflict policy as a genuine both-sides conflict instead. + warning = ("description conflict: the AgilePlace card description changed, but the " + "card holds only a truncated stand-in for the full GitHub issue body -- the " + "edit cannot be safely merged back without risking loss of the untruncated " + "tail -- leaving both sides untouched until a human reconciles them by hand") + return DescriptionResolution(base_norm, False, False, True, warning) return DescriptionResolution(ap_canonical, True, False, False, None) if gh_canonical == ap_canonical: return DescriptionResolution(gh_canonical, False, False, False, None) @@ -180,8 +201,8 @@ def sync_description(cfg: dict, apply: bool, issue: dict, card: dict, issues_sta A dry-run-only planned card (``card["_planOnly"]``) exists only as a synthetic snapshot for continuing this one dry run -- it has no server-side identity yet, so it never carries a - 'description' key. agileplace.card_description()'s lazy get_card() fallback would otherwise - issue a live GET for an id that was never created on the server. Same convention sync.py's + 'description' key. agileplace_description.card_description()'s lazy get_card() fallback would + otherwise issue a live GET for an id that was never created on the server. Same convention sync.py's dependency-sync loop already uses for plan-only cards ("a fresh card has no server-side dependencies; never read a plan-only id") -- a fresh card likewise has no server-side description yet, so the AgilePlace side is treated as "" without any network read. @@ -191,7 +212,8 @@ def sync_description(cfg: dict, apply: bool, issue: dict, card: dict, issues_sta key = issue_custom_id(issue) gh_canonical = _canonicalize_gh_body(issue.get("body")) - ap_description = "" if card.get("_planOnly") else agileplace.card_description(cfg, card) + ap_description = ("" if card.get("_planOnly") + else agileplace_description.card_description(cfg, card)) ap_canonical = _canonicalize_ap_description(ap_description) result = resolve_description(prev.get("desc_base"), prev.get("desc_ap_written"), gh_canonical, ap_canonical) @@ -207,7 +229,7 @@ def sync_description(cfg: dict, apply: bool, issue: dict, card: dict, issues_sta new_ap_written = ap_canonical if result.write_ap: html, _ = _truncate_for_agileplace(result.merged, cfg["ap_description_max_length"]) - queue(card, [agileplace.op_description(html)], "description") + queue(card, [agileplace_description.op_description(html)], "description") new_ap_written = _canonicalize_ap_description(html) if result.write_gh or result.write_ap: diff --git a/smoke.py b/smoke.py index 057ede9..9e9f5a8 100644 --- a/smoke.py +++ b/smoke.py @@ -19,6 +19,7 @@ import sys import agileplace +import agileplace_description from config import env_config PARENT_TITLE = "SMOKE parent (safe to delete)" @@ -281,7 +282,7 @@ def _check_dependencies(cfg: dict, parent_id: str, child_id: str, results: list) def _check_description(cfg: dict, parent_id: str, results: list) -> None: - """Steps 14-15: agileplace.op_description's exact write shape through the same versioned-PATCH + """Steps 14-15: agileplace_description.op_description's exact write shape through the same versioned-PATCH path (patch_card) the sync itself uses (description_sync.sync_description), then a fact-finding probe of the configured ap_description_max_length against the live server (the issue #65 design doc's "max-length probe": cross-check config.DEFAULT_AP_DESCRIPTION_MAX_LENGTH @@ -299,8 +300,8 @@ def _check_description(cfg: dict, parent_id: str, results: list) -> None: _step(14, "description write via op_description through patch_card") fresh = agileplace.get_card(cfg, parent_id) html = "

SMOKE description write

" - agileplace.patch_card(cfg, True, fresh, [agileplace.op_description(html)]) - readback = agileplace.card_description(cfg, agileplace.get_card(cfg, parent_id)) + agileplace.patch_card(cfg, True, fresh, [agileplace_description.op_description(html)]) + readback = agileplace_description.card_description(cfg, agileplace.get_card(cfg, parent_id)) results.append(("description write round-trip (op_description via patch_card)", readback == html, f"description now {readback!r}")) @@ -309,14 +310,14 @@ def _check_description(cfg: dict, parent_id: str, results: list) -> None: probe_html = f"

{'x' * limit}

" fresh = agileplace.get_card(cfg, parent_id) try: - agileplace.patch_card(cfg, True, fresh, [agileplace.op_description(probe_html)]) + agileplace.patch_card(cfg, True, fresh, [agileplace_description.op_description(probe_html)]) except SystemExit as exc: _print_http_failure(exc) results.append(("description length probe", None, f"server REJECTED a {len(probe_html)}-char description " f"(configured max_length={limit}): {exc}")) return - readback = agileplace.card_description(cfg, agileplace.get_card(cfg, parent_id)) + readback = agileplace_description.card_description(cfg, agileplace.get_card(cfg, parent_id)) results.append(("description length probe", None, f"sent {len(probe_html)} chars, server stored {len(readback)} chars back " f"(configured max_length={limit})")) diff --git a/tests/test_agileplace.py b/tests/test_agileplace.py index f7efba8..074f2ea 100644 --- a/tests/test_agileplace.py +++ b/tests/test_agileplace.py @@ -12,7 +12,6 @@ from agileplace import ( # noqa: E402 _card_with_version, api, - card_description, card_external_urls, card_block_reason, card_is_blocked, @@ -23,7 +22,6 @@ get_card, list_cards, op_custom_id, - op_description, op_planned_date, op_tag, ops_blocked, @@ -791,64 +789,9 @@ def fake_api_double_miss(cfg, method, path, body=None, params=None, headers=None patch_card(CFG, True, card, ops) -# --- op_description / card_description (issue #65 Task 1/7) -------------- - -def test_op_description_returns_single_replace_op(): - """No validation here (description_sync owns write-vs-not) -- matches op_custom_id's shape.""" - op = op_description("

hello

") - assert op == {"op": "replace", "path": "/description", "value": "

hello

"} - - -def test_card_description_returns_present_description_without_network_call(): - card = {"id": "1", "description": "

hi

"} - with patch("agileplace.get_card") as get_card_mock: - result = card_description(CFG, card) - get_card_mock.assert_not_called() - assert result == "

hi

" - - -def test_card_description_present_but_empty_string_takes_zero_io_path(): - """A card literally carrying description="" is a real, current empty description -- NOT - 'unknown' -- and must not trigger the lazy get_card fallback (see struct #7 in the design: - every fixture reaching this must either carry the key or explicitly mock get_card).""" - card = {"id": "1", "description": ""} - with patch("agileplace.get_card") as get_card_mock: - result = card_description(CFG, card) - get_card_mock.assert_not_called() - assert result == "" - - -def test_card_description_present_but_none_normalizes_to_empty_string(): - card = {"id": "1", "description": None} - with patch("agileplace.get_card") as get_card_mock: - result = card_description(CFG, card) - get_card_mock.assert_not_called() - assert result == "" - - -def test_card_description_missing_key_falls_back_to_lazy_get_card(): - """list_cards() never returns description (no field-selection params sent) -- a card summary - missing the key entirely must trigger exactly one get_card refetch.""" - card = {"id": "77"} - with patch("agileplace.get_card", - return_value={"id": "77", "description": "

fresh

"}) as get_card_mock: - result = card_description(CFG, card) - get_card_mock.assert_called_once_with(CFG, "77") - assert result == "

fresh

" - - -def test_card_description_lazy_fallback_normalizes_missing_description_to_empty_string(): - card = {"id": "78"} - with patch("agileplace.get_card", return_value={"id": "78"}): - result = card_description(CFG, card) - assert result == "" - - -def test_card_description_never_mutates_input_card(): - card = {"id": "1", "description": "

hi

"} - before = dict(card) - card_description(CFG, card) - assert card == before +# op_description / card_description (issue #65 Task 1/7) moved to agileplace_description.py and +# tests/test_agileplace_description.py -- see review finding: keep this file from growing past the +# project's file-size cap by extracting the description read/write surface into its own module. def test_connect_children_disconnect_children_create_card_have_no_version_header_logic(): diff --git a/tests/test_agileplace_description.py b/tests/test_agileplace_description.py new file mode 100644 index 0000000..18cf47c --- /dev/null +++ b/tests/test_agileplace_description.py @@ -0,0 +1,72 @@ +"""Unit tests for agileplace_description.py's card-description read/write helpers (issue #65 Task +1/7). No network -- pins the JSON Patch shape and the lazy-refetch fallback contract. Split out of +test_agileplace.py alongside the card_description/op_description extraction (review finding: keep +agileplace.py, and its test file, from growing past the project's file-size cap). Run: pytest -q +""" +import sys +from pathlib import Path +from unittest.mock import patch + +sys.path.insert(0, str(Path(__file__).resolve().parent.parent)) + +from agileplace_description import card_description, op_description # noqa: E402 + +CFG = {"token": "t", "host": "h", "board_id": "b1"} + + +def test_op_description_returns_single_replace_op(): + """No validation here (description_sync owns write-vs-not) -- matches op_custom_id's shape.""" + op = op_description("

hello

") + assert op == {"op": "replace", "path": "/description", "value": "

hello

"} + + +def test_card_description_returns_present_description_without_network_call(): + card = {"id": "1", "description": "

hi

"} + with patch("agileplace_description.agileplace.get_card") as get_card_mock: + result = card_description(CFG, card) + get_card_mock.assert_not_called() + assert result == "

hi

" + + +def test_card_description_present_but_empty_string_takes_zero_io_path(): + """A card literally carrying description="" is a real, current empty description -- NOT + 'unknown' -- and must not trigger the lazy get_card fallback (see struct #7 in the design: + every fixture reaching this must either carry the key or explicitly mock get_card).""" + card = {"id": "1", "description": ""} + with patch("agileplace_description.agileplace.get_card") as get_card_mock: + result = card_description(CFG, card) + get_card_mock.assert_not_called() + assert result == "" + + +def test_card_description_present_but_none_normalizes_to_empty_string(): + card = {"id": "1", "description": None} + with patch("agileplace_description.agileplace.get_card") as get_card_mock: + result = card_description(CFG, card) + get_card_mock.assert_not_called() + assert result == "" + + +def test_card_description_missing_key_falls_back_to_lazy_get_card(): + """list_cards() never returns description (no field-selection params sent) -- a card summary + missing the key entirely must trigger exactly one get_card refetch.""" + card = {"id": "77"} + with patch("agileplace_description.agileplace.get_card", + return_value={"id": "77", "description": "

fresh

"}) as get_card_mock: + result = card_description(CFG, card) + get_card_mock.assert_called_once_with(CFG, "77") + assert result == "

fresh

" + + +def test_card_description_lazy_fallback_normalizes_missing_description_to_empty_string(): + card = {"id": "78"} + with patch("agileplace_description.agileplace.get_card", return_value={"id": "78"}): + result = card_description(CFG, card) + assert result == "" + + +def test_card_description_never_mutates_input_card(): + card = {"id": "1", "description": "

hi

"} + before = dict(card) + card_description(CFG, card) + assert card == before diff --git a/tests/test_description_sync.py b/tests/test_description_sync.py index fe2fd52..6983690 100644 --- a/tests/test_description_sync.py +++ b/tests/test_description_sync.py @@ -1,11 +1,11 @@ -"""Unit tests for description_sync.py's pure merge (issue #65). No I/O -- pins the full 9-state +"""Unit tests for description_sync.py's pure merge (issue #65). No I/O -- pins the full 10-state resolve_description matrix plus the two canonicalization helpers' (corrected) idempotence invariants. Run: pytest -q """ from __future__ import annotations +import math import sys -import time from pathlib import Path from unittest.mock import patch @@ -60,7 +60,7 @@ def _assert_invariant(base: str | None, result: DescriptionResolution) -> None: # ===================================================================================== -# resolve_description -- the 9 hand-traced states +# resolve_description -- the 10 hand-traced states # ===================================================================================== def test_seeding_card_empty_pushes_gh_body_to_agileplace(): @@ -103,6 +103,28 @@ def test_truncated_steady_state_compares_ap_side_against_its_own_written_form(): _assert_invariant(full, result) +def test_ap_edit_on_top_of_a_truncated_stub_is_not_blindly_pushed_to_github(): + # desc_base is the FULL agreed canonical; desc_ap_written is only the canonical of the + # TRUNCATED stand-in a prior run actually wrote (see the truncated-steady-state test above). + # If the card's truncated stub is edited further (a comment appended, a typo fixed on the + # visible portion, ...), ap_canonical now differs from desc_ap_written -- but ap_canonical is + # still only that truncated stub, never the untruncated tail that lives solely in `base`. + # Blindly promoting it to `merged` and pushing it to GitHub would permanently destroy the lost + # tail with no warning (critical review finding for issue #65). This must degrade to the same + # warn-and-skip conflict policy as a genuine both-sides conflict: nothing is written, the full + # base stays put, and a human is told to reconcile by hand. + full = "A very long description " * 50 + truncated = full[:100] + "...[truncated by sync]" + edited_truncated = truncated + " one more sentence appended on the card" + result = resolve_description(full, truncated, full, edited_truncated) + assert result.write_gh is False + assert result.write_ap is False + assert result.conflict is True + assert result.merged == full + assert result.warning is not None + _assert_invariant(full, result) + + def test_gh_only_changed_propagates_to_agileplace(): result = resolve_description("Old text", "Old text", "New GH text", "Old text") assert result == DescriptionResolution("New GH text", False, True, False, None) @@ -246,17 +268,37 @@ def test_truncate_result_never_exceeds_max_length_when_achievable(): assert len(html) <= max_length -def test_truncate_large_body_completes_in_low_single_digit_seconds(): - # Pins the O(log n) fix: the spike's O(n^2) per-word shrink loop took 52+s on a 25k-char body. - # GH issue bodies run up to 65,536 chars. A handful of renders (~log2(n)) must stay fast. +def test_truncate_large_body_uses_a_logarithmic_number_of_renders_not_linear(): + # Pins the O(log n)-RENDERS invariant _truncate_for_agileplace's own docstring claims, by + # counting actual calls to richtext.markdown_to_leankit_html instead of inferring it from + # wall-clock time. A wall-clock ceiling is only a proxy: a regression to a coarser-but-still- + # polynomial scheme (fixed-percentage chunks, an accidental O(sqrt(n)) loop, ...) could still + # finish under a generous time budget and pass, and a genuinely correct O(log n) run could + # exceed a tight one on slow/loaded CI hardware. Counting calls pins the claim directly. + # Wraps (never replaces) the real renderer so truncation behavior itself is unaffected. markdown = ("The quick brown fox jumps over the lazy dog. " * 1500) assert len(markdown) >= 20_000 - start = time.monotonic() - html, was_truncated = _truncate_for_agileplace(markdown, max_length=5000) - elapsed = time.monotonic() - start + real_render = richtext.markdown_to_leankit_html + calls = [] + + def _counting_render(md): + calls.append(md) + return real_render(md) + + with patch("description_sync.richtext.markdown_to_leankit_html", side_effect=_counting_render): + html, was_truncated = _truncate_for_agileplace(markdown, max_length=5000) + assert was_truncated is True assert len(html) <= 5000 - assert elapsed < 5.0, f"truncation took {elapsed:.2f}s -- the O(n^2) regression may have returned" + # A binary search over len(markdown) candidate cuts takes ~log2(n) iterations plus a couple of + # fixed renders (the initial full-length probe, the final confirmed cut) -- nowhere near + # proportional to len(markdown). A regression back to a per-word/per-chunk shrink loop would + # blow this bound by orders of magnitude on a ~70,000-char input. + max_expected_renders = math.ceil(math.log2(len(markdown))) + 4 + assert len(calls) <= max_expected_renders, ( + f"_truncate_for_agileplace made {len(calls)} renders (expected <= {max_expected_renders}) " + "-- the O(log n) fix may have regressed to a linear/near-linear scheme" + ) # ===================================================================================== @@ -276,8 +318,8 @@ def test_sync_description_dry_run_does_not_advance_base_despite_queued_ap_write( card = _card() state = {ISSUE_URL: {}} queue = _Queue() - with patch("description_sync.agileplace.card_description", return_value=""), \ - patch("description_sync.agileplace.op_description", return_value="OP") as op_mock, \ + with patch("description_sync.agileplace_description.card_description", return_value=""), \ + patch("description_sync.agileplace_description.op_description", return_value="OP") as op_mock, \ patch("description_sync.ghkit.edit_issue_body") as edit_mock: sync_description(_cfg(), False, issue, card, state, queue) op_mock.assert_called_once() @@ -294,8 +336,8 @@ def test_sync_description_failed_gh_write_blocks_base_advance(): card = _card() state = {ISSUE_URL: {}} queue = _Queue() - with patch("description_sync.agileplace.card_description", return_value="Hello from AgilePlace"), \ - patch("description_sync.agileplace.op_description") as op_mock, \ + with patch("description_sync.agileplace_description.card_description", return_value="Hello from AgilePlace"), \ + patch("description_sync.agileplace_description.op_description") as op_mock, \ patch("description_sync.ghkit.edit_issue_body", return_value=False) as edit_mock: sync_description(_cfg(), True, issue, card, state, queue) edit_mock.assert_called_once_with(_cfg(), True, issue["number"], "Hello from AgilePlace") @@ -310,8 +352,8 @@ def test_sync_description_conflict_makes_no_writes_and_no_advance(): card = _card() state = {ISSUE_URL: {}} queue = _Queue() - with patch("description_sync.agileplace.card_description", return_value="AP text"), \ - patch("description_sync.agileplace.op_description") as op_mock, \ + with patch("description_sync.agileplace_description.card_description", return_value="AP text"), \ + patch("description_sync.agileplace_description.op_description") as op_mock, \ patch("description_sync.ghkit.edit_issue_body") as edit_mock: sync_description(_cfg(), True, issue, card, state, queue) edit_mock.assert_not_called() @@ -328,8 +370,8 @@ def test_sync_description_confirmed_gh_write_advances_both_fields_together(): card = _card() state = {ISSUE_URL: {}} queue = _Queue() - with patch("description_sync.agileplace.card_description", return_value="Hello from AgilePlace"), \ - patch("description_sync.agileplace.op_description") as op_mock, \ + with patch("description_sync.agileplace_description.card_description", return_value="Hello from AgilePlace"), \ + patch("description_sync.agileplace_description.op_description") as op_mock, \ patch("description_sync.ghkit.edit_issue_body", return_value=True) as edit_mock: sync_description(_cfg(), True, issue, card, state, queue) edit_mock.assert_called_once_with(_cfg(), True, issue["number"], "Hello from AgilePlace") @@ -348,8 +390,8 @@ def test_sync_description_confirmed_ap_write_advances_both_fields_together(): queue = _Queue() written_html = richtext.markdown_to_leankit_html("Hello from GitHub") expected_ap_written = _canonicalize_ap_description(written_html) - with patch("description_sync.agileplace.card_description", return_value=""), \ - patch("description_sync.agileplace.op_description", return_value="OP") as op_mock, \ + with patch("description_sync.agileplace_description.card_description", return_value=""), \ + patch("description_sync.agileplace_description.op_description", return_value="OP") as op_mock, \ patch("description_sync.ghkit.edit_issue_body") as edit_mock: sync_description(_cfg(), True, issue, card, state, queue) edit_mock.assert_not_called() @@ -363,18 +405,18 @@ def test_sync_description_never_reads_a_plan_only_dry_run_card(): # Discovered running the wiring end-to-end (tests/test_run.py's dry-run path, issue #65 task # 5/7): a dry-run-only card carries agileplace.create_card's synthetic "_planOnly" marker and # has no server-side identity yet, so it never has a 'description' key. Without a guard, - # agileplace.card_description()'s lazy get_card() fallback fires a live GET for an id that was + # agileplace_description.card_description()'s lazy get_card() fallback fires a live GET for an id that was # never created on the server -- exactly the network read sync.py's own dependency-sync loop # already refuses for plan-only cards ("a fresh card has no server-side dependencies; never # read a plan-only id"). sync_description must apply that same convention: treat a plan-only - # card's AgilePlace-side description as "" without calling agileplace.card_description() at all. + # card's AgilePlace-side description as "" without calling agileplace_description.card_description() at all. issue = _issue(body="Hello from GitHub") card = {**_card(), "_planOnly": True} state = {ISSUE_URL: {}} queue = _Queue() written_html = richtext.markdown_to_leankit_html("Hello from GitHub") - with patch("description_sync.agileplace.card_description") as card_description_mock, \ - patch("description_sync.agileplace.op_description", return_value="OP") as op_mock, \ + with patch("description_sync.agileplace_description.card_description") as card_description_mock, \ + patch("description_sync.agileplace_description.op_description", return_value="OP") as op_mock, \ patch("description_sync.ghkit.edit_issue_body") as edit_mock: sync_description(_cfg(), False, issue, card, state, queue) card_description_mock.assert_not_called() diff --git a/tests/test_description_sync_wiring_fixtures.py b/tests/test_description_sync_wiring_fixtures.py index c8846dc..fdf7fbe 100644 --- a/tests/test_description_sync_wiring_fixtures.py +++ b/tests/test_description_sync_wiring_fixtures.py @@ -2,7 +2,7 @@ per-issue loop must never let a pre-existing card fixture in test_sync_main.py, test_vetting_latch.py, or test_run.py fall through to a real agileplace.api() call. -agileplace.card_description() takes a zero-I/O path whenever the card dict already carries a +agileplace_description.card_description() takes a zero-I/O path whenever the card dict already carries a 'description' key (even ""); a fixture that omits it makes sync_description's card_description(cfg, card) call fall back to agileplace.get_card(), which none of these three files mock -- so it hits the real HTTP client. Confirmed live in the design spike: a real host @@ -54,6 +54,6 @@ def test_no_wired_test_file_lets_a_card_fixture_reach_real_agileplace_api(): assert not failures, ( "one or more wired test files let a card fixture reach sync_description's " - "agileplace.card_description() fallback (real agileplace.api() call) -- give the fixture " + "agileplace_description.card_description() fallback (real agileplace.api() call) -- give the fixture " "a 'description' key or explicitly mock agileplace.get_card:\n\n" + "\n\n".join(failures) ) diff --git a/tests/test_hierarchy_ownership.py b/tests/test_hierarchy_ownership.py index f8e1a49..5656f89 100644 --- a/tests/test_hierarchy_ownership.py +++ b/tests/test_hierarchy_ownership.py @@ -39,7 +39,7 @@ def _card(number: int, custom_id: str, *, url: str | None = None, "laneId": None, "blockedStatus": {"isBlocked": False, "reason": ""}, "childCards": [{"id": child_id} for child_id in children], - # issue #65: keeps agileplace.card_description() on its zero-I/O path. + # issue #65: keeps agileplace_description.card_description() on its zero-I/O path. "description": "", } return {**card, **({"externalLink": {"url": url}} if url else {})} diff --git a/tests/test_retired_issues.py b/tests/test_retired_issues.py index 6baa506..4fae184 100644 --- a/tests/test_retired_issues.py +++ b/tests/test_retired_issues.py @@ -87,7 +87,7 @@ def _card(number: int, lane_id: str, *, blocked: bool) -> dict: "blockedStatus": {"isBlocked": blocked, "reason": "Blocked by #10" if blocked else ""}, "plannedStart": None, "plannedFinish": None, - # issue #65: keeps agileplace.card_description() on its zero-I/O path. + # issue #65: keeps agileplace_description.card_description() on its zero-I/O path. "description": "", } diff --git a/tests/test_run.py b/tests/test_run.py index 906c220..f2af37b 100644 --- a/tests/test_run.py +++ b/tests/test_run.py @@ -119,7 +119,7 @@ def __init__(self): "plannedFinish": None, "blockedStatus": {"isBlocked": False, "reason": ""}, "childCards": [], - # issue #65: keeps agileplace.card_description() on its zero-I/O path -- without this + # issue #65: keeps agileplace_description.card_description() on its zero-I/O path -- without this # key it falls back to the real (unmocked) GET card/{id}, which this fixture's own # open_url dispatch doesn't expect and raises "unexpected AgilePlace request". "description": "", diff --git a/tests/test_sync_card_coherence.py b/tests/test_sync_card_coherence.py index c0c5938..22b7e10 100644 --- a/tests/test_sync_card_coherence.py +++ b/tests/test_sync_card_coherence.py @@ -84,7 +84,7 @@ def _card(card_id: str, custom_id: str, urls: list[str], *, lane_id: str = "L-EL "tags": [], "plannedStart": None, "plannedFinish": None, - # issue #65: keeps agileplace.card_description() on its zero-I/O path. + # issue #65: keeps agileplace_description.card_description() on its zero-I/O path. "description": "", } diff --git a/tests/test_sync_contested_cards.py b/tests/test_sync_contested_cards.py index 01cd0ae..bf60e9a 100644 --- a/tests/test_sync_contested_cards.py +++ b/tests/test_sync_contested_cards.py @@ -61,7 +61,7 @@ def _card_with_urls(card_id: str, custom_id: str, urls: list[str], *, lane_id: s "tags": [], "plannedStart": None, "plannedFinish": None, - # issue #65: keeps agileplace.card_description() on its zero-I/O path. + # issue #65: keeps agileplace_description.card_description() on its zero-I/O path. "description": "", } diff --git a/tests/test_sync_intake_call_site.py b/tests/test_sync_intake_call_site.py index 82a150b..2db8b48 100644 --- a/tests/test_sync_intake_call_site.py +++ b/tests/test_sync_intake_call_site.py @@ -39,7 +39,7 @@ def _issue(): def _card(): - # "description": "" (issue #65) keeps agileplace.card_description() on its zero-I/O path. + # "description": "" (issue #65) keeps agileplace_description.card_description() on its zero-I/O path. return {"id": "C1", "version": 1, "customId": "1", "externalLink": {"url": ISSUE_URL}, "tags": [], "plannedStart": None, "plannedFinish": None, "laneId": "LANE1", diff --git a/tests/test_sync_main.py b/tests/test_sync_main.py index 025de73..bf33682 100644 --- a/tests/test_sync_main.py +++ b/tests/test_sync_main.py @@ -37,7 +37,7 @@ def _issue(): def _card(): - # "description": "" (issue #65) keeps agileplace.card_description() on its zero-I/O path -- + # "description": "" (issue #65) keeps agileplace_description.card_description() on its zero-I/O path -- # without the key it falls back to the real (unmocked) agileplace.get_card(), which hits the # live HTTP client and SystemExits (confirmed live in the design spike). return {"id": "C1", "version": 1, "customId": "1", From 704b9e0e17d159bc867ec9bf4900645111e862e6 Mon Sep 17 00:00:00 2001 From: thewrz Date: Wed, 22 Jul 2026 18:23:08 -0700 Subject: [PATCH 09/10] test(description-sync): widen fixture-regression canary to all four repaired suites The subprocess canary for issue #65's card-fixture regression (a missing 'description' key falling through to a real agileplace.api() call) only covered test_sync_main.py, test_vetting_latch.py, and test_run.py. This same PR also had to add explicit 'description': '' fixtures to test_hierarchy_ownership.py, test_retired_issues.py, test_sync_card_coherence.py, and test_sync_contested_cards.py to dodge the identical regression -- none of which the canary was watching. Verified red: reverting the fixture fix in test_hierarchy_ownership.py reproduces the real HTTP 401 fallthrough and the old canary (3 files) missed it entirely; adding the file to WIRED_TEST_FILES makes the canary fail as expected, and restoring the fixture makes it pass again. Co-Authored-By: Claude Sonnet 5 --- .../test_description_sync_wiring_fixtures.py | 34 ++++++++++++------- 1 file changed, 21 insertions(+), 13 deletions(-) diff --git a/tests/test_description_sync_wiring_fixtures.py b/tests/test_description_sync_wiring_fixtures.py index fdf7fbe..e5de125 100644 --- a/tests/test_description_sync_wiring_fixtures.py +++ b/tests/test_description_sync_wiring_fixtures.py @@ -1,20 +1,22 @@ """Guards issue #65 task 5's fixture-repair invariant: wiring sync_description into sync.py's -per-issue loop must never let a pre-existing card fixture in test_sync_main.py, -test_vetting_latch.py, or test_run.py fall through to a real agileplace.api() call. +per-issue loop must never let a pre-existing card fixture in any file exercising sync.main() +fall through to a real agileplace.api() call. agileplace_description.card_description() takes a zero-I/O path whenever the card dict already carries a 'description' key (even ""); a fixture that omits it makes sync_description's -card_description(cfg, card) call fall back to agileplace.get_card(), which none of these three +card_description(cfg, card) call fall back to agileplace.get_card(), which none of these files mock -- so it hits the real HTTP client. Confirmed live in the design spike: a real host -answers with HTTP 401 -> SystemExit inside test_sync_main.py/test_vetting_latch.py (which patch -agileplace at the function level, not the transport), and test_run.py's own request-dispatch stub -(FixtureWorld.open_url) raises its own AssertionError("unexpected AgilePlace request") for the -same reason -- neither is a legitimate assertion failure, both are this exact regression. +answers with HTTP 401 -> SystemExit inside files that patch agileplace at the function level +(not the transport), and test_run.py's own request-dispatch stub (FixtureWorld.open_url) raises +its own AssertionError("unexpected AgilePlace request") for the same reason -- neither is a +legitimate assertion failure, both are this exact regression. -Rather than re-deriving by static analysis which fixtures reach the per-issue loop (fragile across -three differently-shaped harnesses -- test_sync_main.py's shared _mock_io, test_vetting_latch.py's -reuse of it, test_run.py's own FixtureWorld HTTP stub), this runs each file as its own subprocess -and requires zero failures/errors: the same boundary the spike's regression was actually caught at. +Rather than re-deriving by static analysis which fixtures reach the per-issue loop (fragile +across the differently-shaped harnesses in play -- test_sync_main.py's shared _mock_io, +test_vetting_latch.py's reuse of it, test_run.py's own FixtureWorld HTTP stub, and the +per-file card builders in the hierarchy/retired/coherence/contested-card suites), this runs +each file as its own subprocess and requires zero failures/errors: the same boundary the +spike's regression was actually caught at. Run: pytest -q tests/test_description_sync_wiring_fixtures.py """ @@ -26,12 +28,18 @@ REPO_ROOT = Path(__file__).resolve().parent.parent -# The three files task 5/7 is scoped to repair -- every pre-existing card fixture reaching -# sync.main()'s per-issue loop lives in one of these. +# Every file whose card fixtures reach sync.main()'s per-issue loop -- and therefore +# sync_description's agileplace_description.card_description() call -- belongs here. This +# includes task 5/7's original three plus the hierarchy/retired/coherence/contested-card +# suites, all of which needed the same 'description' key fixture repair. WIRED_TEST_FILES = ( "tests/test_sync_main.py", "tests/test_vetting_latch.py", "tests/test_run.py", + "tests/test_hierarchy_ownership.py", + "tests/test_retired_issues.py", + "tests/test_sync_card_coherence.py", + "tests/test_sync_contested_cards.py", ) From df3ad47410ccd9c57ceb51f3d3ef2a12c274d550 Mon Sep 17 00:00:00 2001 From: thewrz Date: Wed, 22 Jul 2026 23:10:33 -0700 Subject: [PATCH 10/10] test(description-sync): add intake call-site suite to wiring fixture guard tests/test_sync_intake_call_site.py runs sync.main() through the same mocked I/O harness as the other wired suites, so its card fixtures reach sync_description's card_description() call and belong under the same no-real-API regression guard. Co-Authored-By: Claude Fable 5 --- tests/test_description_sync_wiring_fixtures.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/test_description_sync_wiring_fixtures.py b/tests/test_description_sync_wiring_fixtures.py index e5de125..388db19 100644 --- a/tests/test_description_sync_wiring_fixtures.py +++ b/tests/test_description_sync_wiring_fixtures.py @@ -34,6 +34,7 @@ # suites, all of which needed the same 'description' key fixture repair. WIRED_TEST_FILES = ( "tests/test_sync_main.py", + "tests/test_sync_intake_call_site.py", "tests/test_vetting_latch.py", "tests/test_run.py", "tests/test_hierarchy_ownership.py",