diff --git a/.gitignore b/.gitignore index 2c8bee5..2f5efbf 100644 --- a/.gitignore +++ b/.gitignore @@ -221,3 +221,6 @@ __marimo__/ # Streamlit .streamlit/secrets.toml + +# Claude Code workflow worktrees (session-local, never repo content) +.claude/worktrees/ diff --git a/API-VALIDATION.md b/API-VALIDATION.md index 910e0e1..1155d58 100644 --- a/API-VALIDATION.md +++ b/API-VALIDATION.md @@ -4,8 +4,8 @@ This file records how the API calls this tool makes were checked before running account. On 2026-07-17 each call was compared with the public Planview LeanKit io v2 docs and, where it documents the same operation, the official LeanKit Node client. Most calls are confirmed. A few, marked `[live-check]`, are not fully pinned down by public docs and should be smoke-tested once -against a disposable card. The last two sections record what was verified live: GitHub shapes on -2026-07-18, and AgilePlace write paths on 2026-07-20 (a `smoke.py` run). +against a disposable card. The last sections record what was verified live: GitHub shapes on +2026-07-18, and AgilePlace write paths on 2026-07-20 and 2026-07-21 (`smoke.py` runs). ## Confirmed against the cited sources @@ -103,10 +103,11 @@ the code still fails closed (refetch, validate, or abort) rather than assuming e Target values read from paginated GraphQL `ProjectV2Item.fieldValues` by field id; writes via `item-edit --date`. A successful GraphQL snapshot with no matching values means the field is cleared project-wide; a failed, malformed, or incomplete snapshot skips every date write that run. - The GraphQL date-read shape was exercised live on 2026-07-20 (see below). The - `plannedStart`/`plannedFinish` card PATCH is covered by `smoke.py` steps 5-6 but those steps were - added after the 2026-07-20 run -- it is the one AgilePlace `[live-check]` still awaiting a live - pass. + The GraphQL date-read shape was exercised live on 2026-07-20 (see below). The card PATCH was + live-run on 2026-07-21: setting both dates as strings is confirmed, but clearing them with a null + `replace` was rejected (HTTP 422 "Invalid value: must be string"), so clears now use an RFC-6902 + `remove` op (issue #52). A second run the same day, after the fix merged, confirmed the + remove-based clear and the blocked-state clear live -- see "Validated live on 2026-07-21" below. ## GitHub side (standard and stable, noted for completeness) @@ -156,8 +157,8 @@ Two gh behaviors learned the same day, recorded so this repo never has to redisc A confirmed `python smoke.py` run (create -> mutate -> connect -> stale-probe -> delete on two throwaway cards) retired every AgilePlace `[live-check]` item above except the planned-date card -PATCH -- smoke steps 5-6 (blocked state + planned dates) were added after this run and await the -next live pass: +PATCH -- smoke steps 5-6 (blocked state + planned dates) were added after this run; their first +live outcomes are recorded in the 2026-07-21 section below: 1. **Tag removal (item 1): confirmed.** Index-based `{op:"remove", path:"/tags/{i}"}` cleared the tag in an add-then-remove round-trip (`tags` ended empty). @@ -180,3 +181,108 @@ Model 2 additions confirmed by the same run: card create with `customId`+`extern an external link both accepted; `card/connections` connect/disconnect round-trip works and the children read reflects it immediately (including the authoritative empty read after disconnect). `DELETE /io/card/{id}` (smoke cleanup only) deletes for real -- a follow-up GET returns 404. + +## Validated live on 2026-07-21 (smoke.py runs, production tenant board) + +Two runs: the first exercised steps 5-6 (blocked state + planned dates, added after the 2026-07-20 +run) and surfaced the date-clear rejection; the second, after the issue #52 fix merged, passed the +full sequence: + +1. **Blocked-state set: confirmed.** `/isBlocked` true + `/blockReason` string round-trips + (`isBlocked=True`, reason read back). +2. **Planned-date set: confirmed.** String `replace` on `/plannedStart` and `/plannedFinish` + round-trips (`2026-01-01`/`2026-01-02` read back exactly). +3. **Planned-date clear via null replace: REJECTED -- and the rejection taught two facts.** + `{op:"replace", path:"/plannedStart", value:null}` fails HTTP 422 "Invalid value: must be + string": the server type-validates `replace` values on these paths. And PATCH validation is + atomic: the 422 listed only the two date ops as invalid, yet no op in the batch (including the + valid blocked-clear ops) was applied. `op_planned_date` now emits `{op:"remove", path:"/{field}"}` + for a clear, and the offline smoke double mirrors the 422 (issue #52). +4. **Remove-based date clear + blocked-state clear: confirmed.** The second (post-fix) run's step 6 + read back `isBlocked=False`, `plannedStart=None`, `plannedFinish=None`: the RFC-6902 `remove` + clears both dates, and the blocked-clear ops apply once no invalid op poisons the batch. + +Both runs also re-confirmed create (`customId`+`externalLink`), the version-less create response, +flat single-card GET, both tag round-trips, the connections round-trip, the stale-version HTTP 428 +rejection, and `DELETE` + 404 cleanup. + +The same day's first live migration (53 cards, issue #55) established one more create-response +fact: it echoes neither `customId` nor `laneId` -- for sync purposes the response is the new card +id and nothing else. `sync.py` therefore refetches each just-created card once and indexes the +fresh card as its snapshot, and the offline sync-run double mirrors the id-only response. + +With that, every AgilePlace `[live-check]` item in this file is retired -- each one now has a +recorded live outcome. + +## Dependencies API discovery (2026-07-21, issue #57) + +The io v2 public docs do not document the Dependencies feature's endpoints, so +`probe_dependencies.py` (strictly read-only; the GET-only invariant is test-pinned) probed the +production tenant: + +- **Read endpoint confirmed: `GET /io/card/{cardId}/dependency` -> HTTP 200 `{"dependencies":[]}`.** + Singular path, matching the documented `connection/children` singular pattern. +- All other candidates returned a clean 404: `card/{cardId}/dependencies`, + `dependencies?cardId=`, `dependency?cardId=`, `board/{boardId}/dependencies`, + `card/{cardId}/connection/dependencies`. The documented `connection/children` control endpoint + answered 200 in the same run, so those 404s are real misses, not plumbing failures. +- The single-card GET embeds no dependency data: none of its 49 top-level keys is dependency-ish + (it does embed `parentCards` and `connectedCardStats` for the connections feature). + +**Populated entry shape: confirmed live 2026-07-21.** After a UI-created dependency +(EP-3A -> JPOWER1, mirroring the real GitHub blocked-by edge #31 -> #68), the blocked card's read +returned: + +```json +{"dependencies": [{"direction": "incoming", "cardId": "2490185684", + "timing": "finishToStart", "createdOn": "2026-07-21T16:19:24.147Z"}]} +``` + +- `direction` is relative to the card being read (`incoming` = the other card must progress first); + `cardId` is the OTHER end; `timing` uses camelCase enum values (`finishToStart` observed). +- **Entries carry no dependency id.** A dependency is evidently identified by its + (card, direction, other-card, timing) tuple, which implies deletion is addressed by card pair, + not by id -- the delete capture below must confirm how. + +**Delete shape: confirmed live 2026-07-21** (devtools capture of the UI removing the EP-3A -> +JPOWER1 dependency): + +```http +DELETE /io/card/dependency +{"cardIds": ["2490186236"], "dependsOnCardIds": ["2490185684"]} +``` + +Pair-addressed, as the id-less read entries implied: `cardIds` is the dependent (blocked) side, +`dependsOnCardIds` the blocker side, both plural batch arrays in the same style as +`card/connections`. No card id in the path; the UI's own session auth rode a cookie, but the +endpoint lives under the same `/io/` surface as everything the token-authenticated client uses. + +**Create shape: confirmed live 2026-07-21** (devtools capture of the UI recreating the same +dependency): + +```http +POST /io/card/dependency +{"cardIds": ["2490186236"], "dependsOnCardIds": ["2490185684"], "timing": "finishToStart"} +``` + +Same pair body as delete plus an explicit `timing` member (camelCase enum, `finishToStart` +observed). The POST *response* returns the card's updated `dependencies` list where each entry +additionally carries a `face` projection of the other card -- including a `dependencyStats` object +(`incoming/outgoing/total` x `Count/ResolvedCount/ExceptionCount/UnresolvedBlockedCount`), which is +the native health tracking. Plain reads (`GET card/{cardId}/dependency`) omit `face`; the sync needs +only `direction`/`cardId`/`timing`. + +**Timing types.** The UI offers four: FS (Finish->Start), SS (Start->Start), FF (Finish->Finish), +SF (Start->Finish). Only `finishToStart`'s wire value has been observed; the other three +presumably follow the same camelCase pattern but are unconfirmed and unused -- GitHub blocked-by +has exactly one semantic ("the blocker must finish first"), which is FS. The sync reconciles +dependencies by card PAIR and never inspects or rewrites `timing` after creation: a human refining +a synced dependency's timing in the UI keeps their refinement. + +**Round-trip and duplicate-create: confirmed live 2026-07-21** (`smoke.py` steps 11-12, run +suffix 10a01e, production tenant): create -> incoming read (right card, `finishToStart`) -> +delete -> empty read all behaved as coded. Duplicate create is REJECTED -- **HTTP 409 Conflict +`{"message": "Dependency already exists", "data": {"dependsOnCardId", "cardId"}}`** -- not +idempotent. The sync's diff-before-write never re-creates an existing pair, and its fail-closed +skip on unknown reads is what keeps a blind re-create (and its 409 SystemExit) impossible. The +offline doubles mirror the 409. No dependency `[live-check]` remains open. diff --git a/README.md b/README.md index 6a4cacf..d09c3f2 100644 --- a/README.md +++ b/README.md @@ -13,7 +13,7 @@ With `--apply`, each run: 1. Creates a card for every active issue, epics and tasks alike. Cards are matched to issues by the card's external-link URL, with the card's customId as a fallback. Issues closed as `NOT_PLANNED`/`DUPLICATE` never get new cards; if one already has a URL-matched card, the run - retires it to Done and clears its stale blocked state without syncing its metadata back to GitHub. + retires it to Done without syncing its metadata back to GitHub. 2. Moves each card to the lane for its stage. For open issues, the stage is the issue's Status on the GitHub Projects v2 board (Backlog / Ready / In progress / In review / Done), which is the source of truth. A closed issue always resolves to Done, even when its Project item retains a @@ -30,21 +30,23 @@ With `--apply`, each run: the paginated AgilePlace connections endpoint. Links are removed only when both the native GitHub sub-issue snapshot and the AgilePlace child snapshot are authoritative; either read failing makes that epic add-only for the run. LeanKit then rolls child progress up to the parent on its own. -4. Marks a card blocked while any issue blocking it is not Done, and clears the blocked state - otherwise. A `NOT_PLANNED`/`DUPLICATE` blocker is known Done, so it cannot block dependents - forever; a blocker missing from the complete issue snapshot remains incomplete (fail-closed). +4. Mirrors GitHub blocked-by relationships as native AgilePlace card dependencies (blocker -> + blocked, Finish-to-Start), every edge included -- a Done or retired blocker's edge is structural, + and AgilePlace's own dependency health display shows satisfaction. Reconciliation is authoritative + only between two sync-managed cards, and a failed dependency read skips that card for the run. + The card's Blocked flag belongs to humans: the sync never writes `/isBlocked` or `/blockReason`. 5. Reconciles metadata and dates in both directions using a three-way merge against `.sync-state.json`. GitHub labels and milestone sync with card tags, and removals carry over in both directions. The Project's Start and Target date fields sync with the card's `plannedStart` and `plannedFinish`; if both sides changed the same date, the AgilePlace value wins. -Field updates queued for an existing card -- lane, tags, dates, and blocked state -- are combined +Field updates queued for an existing card -- lane, tags, and dates -- are combined into at most one versioned PATCH per run, so a stale write fails instead of silently overwriting someone else's edit. A version-less card is refetched once; the PATCH proceeds only if every queued field still matches that fresh snapshot, and otherwise the run aborts before saving merge state. -Card creation and hierarchy connections use separate POST/DELETE requests, and GitHub-side writes -are issued separately. Dry run is the default: `python sync.py` prints every planned action and -writes nothing. +Card creation, hierarchy connections, and dependencies use separate POST/DELETE requests, and +GitHub-side writes are issued separately. Dry run is the default: `python sync.py` prints every +planned action and writes nothing. > Before the first live run: `API-VALIDATION.md` records which API shapes are confirmed and which > still need a one-time check against the live APIs (the AgilePlace connection, blocked-state, tag, @@ -59,9 +61,11 @@ AgilePlace *write* shapes. It previews the configured board first -- title, lane currently on it -- and asks for typed confirmation before writing anything. It then creates two clearly-marked throwaway cards (custom ids carry a fresh per-run suffix so they can never collide with, or be adopted by, a real sync run), round-trips a tag add/remove, sets and clears blocked -state and planned dates, adds an external link to a bare card and reads it back, connects and -disconnects them as parent/child, sends a deliberately stale-version PATCH (which the server must -reject), and deletes both cards again, confirming each is gone. Every step is printed, +state and planned dates (the blocked round-trip validates API surface only -- the sync itself never +writes the flag), adds an external link to a bare card and reads it back, connects and disconnects +them as parent/child, round-trips a dependency create/read/delete, sends a deliberately +stale-version PATCH (which the server must reject), and deletes both cards again, confirming each +is gone. Every step is printed, and a rejected write shows the server's full, untruncated response body so a wrong shape is diagnosable from the console. The final summary maps one-to-one onto the `[live-check]` items in `API-VALIDATION.md`. `--yes` skips the prompt. GitHub is never touched and `.sync-state.json` is @@ -115,7 +119,7 @@ The account running the schedule needs `python` and `gh` on PATH, `gh auth login - `sync.py`: orchestration. Creates, moves, connects, and blocks cards; reconciles tags and dates; sends at most one field-update PATCH per existing card. -- `stages.py`: pure stage derivation, blocked-reason text, and lane/title matching (unit-tested). +- `stages.py`: pure stage derivation and lane/title matching (unit-tested). - `reconcile.py`: pure three-way set and single-value merges (unit-tested). - `ghkit.py`: GitHub via the `gh` CLI (issues, sub-issues, open-PR and blocked-by reads, label/milestone writes). diff --git a/agileplace.py b/agileplace.py index 5e305ed..6387b05 100644 --- a/agileplace.py +++ b/agileplace.py @@ -496,7 +496,12 @@ def ops_tag_remove(current_tags: list[str], tags_to_remove: set[str]) -> list[di def op_planned_date(field: str, date: str | None) -> dict: - return {"op": "replace", "path": f"/{field}", "value": date} # field = plannedStart|plannedFinish + # field = plannedStart|plannedFinish. The server type-validates replace values on these paths as + # strings and 422s on null (observed live 2026-07-21, issue #52), so clearing must be a remove. + # Callers only queue a clear when the card currently has the date, so remove-on-absent can't occur. + if date is None: + return {"op": "remove", "path": f"/{field}"} + return {"op": "replace", "path": f"/{field}", "value": date} def ops_blocked(blocked: bool, reason: str | None) -> list[dict]: @@ -677,3 +682,65 @@ def disconnect_children(cfg: dict, apply: bool, parent_card_id: str, child_card_ mutate(cfg, apply, "DELETE", "card/connections", body={"cardIds": [parent_card_id], "connections": {"children": ids}}, note=f"disconnect {parent_card_id} -> {len(ids)} child card(s)") + + +# --- dependencies (sequencing) -- shapes confirmed live 2026-07-21, issue #57 --- +# The endpoints are undocumented in the io v2 public docs; every shape below was captured from the +# UI and validated against the production tenant (API-VALIDATION.md "Dependencies API discovery"). + +DEPENDENCY_TIMING = "finishToStart" + + +def card_dependencies(cfg: dict, card_id: str) -> list[dict] | None: + """The card's dependency entries ({direction, cardId, timing, ...}), or None when the read + fails or is unrecognized. None means UNKNOWN -- callers must skip reconciliation for the card + (fail closed), never treat it as an empty set. The confirmed response carries the whole list + with no pageMeta; if the server ever paginates this, the shape check below fails closed.""" + try: + data = api(cfg, "GET", f"{_card_path(card_id)}/dependency") + except SystemExit as err: + print(f"WARN card {card_id} dependency read FAILED: {err} -- skipping dependency reconciliation") + return None + entries = data.get("dependencies") if isinstance(data, dict) else None + # Reject anything short of a complete, well-formed snapshot: a pageMeta member would mean + # the server started paginating (entries could be missing -> re-create -> 409 abort), and an + # entry without a usable direction/cardId hides state the diff would then act against. + if (not isinstance(data, dict) or "pageMeta" in data or not isinstance(entries, list) + or any(not _valid_dependency_entry(e) for e in entries)): + print(f"WARN card {card_id} dependency read returned an unrecognized shape -- " + "skipping dependency reconciliation") + return None + return entries + + +def _valid_dependency_entry(entry) -> bool: + return (isinstance(entry, dict) + and entry.get("direction") in {"incoming", "outgoing"} + and bool(entry.get("cardId"))) + + +def incoming_dependency_ids(entries: list[dict]) -> set[str]: + """Ids of the cards this card depends on (its blockers), from a card_dependencies read.""" + return {str(entry["cardId"]) for entry in entries + if entry.get("direction") == "incoming" and entry.get("cardId")} + + +def create_dependencies(cfg: dict, apply: bool, card_id: str, depends_on_ids) -> None: + """Make card_id depend on each of depends_on_ids (finish-to-start; they block it).""" + ids = sorted(str(i) for i in depends_on_ids if i) + if not ids: + return + mutate(cfg, apply, "POST", "card/dependency", + body={"cardIds": [str(card_id)], "dependsOnCardIds": ids, "timing": DEPENDENCY_TIMING}, + note=f"depend {card_id} on {len(ids)} card(s)") + + +def delete_dependencies(cfg: dict, apply: bool, card_id: str, depends_on_ids) -> None: + """Remove card_id's dependency on each of depends_on_ids (pair-addressed; no dependency ids + exist -- see API-VALIDATION.md).""" + ids = sorted(str(i) for i in depends_on_ids if i) + if not ids: + return + mutate(cfg, apply, "DELETE", "card/dependency", + body={"cardIds": [str(card_id)], "dependsOnCardIds": ids}, + note=f"undepend {card_id} from {len(ids)} card(s)") diff --git a/docs/superpowers/specs/2026-07-21-blocked-by-dependencies-design.md b/docs/superpowers/specs/2026-07-21-blocked-by-dependencies-design.md new file mode 100644 index 0000000..24601d8 --- /dev/null +++ b/docs/superpowers/specs/2026-07-21-blocked-by-dependencies-design.md @@ -0,0 +1,116 @@ +# Sync GitHub blocked-by edges as native AgilePlace Dependencies + +Design agreed 2026-07-21 (issue #57). Decisions made with the maintainer: +discovery via read-only probe script; the Blocked flag was retained through +Phase 1 and, after the native dependency visuals were judged on the live board, +retired in Phase 2 -- the flag is human-owned now, never written by the sync; +GitHub is authoritative for dependencies between sync-managed cards. + +## Problem + +GitHub blocked-by edges currently surface in AgilePlace only as the card-level +Blocked flag (`isBlocked` + "Blocked by #N" free text). That is loud but +structureless: no link to the blocking card, an empty Dependencies tab +(INCOMING/OUTGOING both 0), no health tracking, and the "#N" reference is a +GitHub issue number a board user cannot click. AgilePlace's native lexicon: +parent/child connections express hierarchy (already synced correctly); +Dependencies express sequencing -- the correct home for blocked-by. + +## Gating constraint + +The io v2 public docs do not document a dependencies endpoint (checked +2026-07-21: documented write surface is cards, connections (parent/child), +comments, attachments, custom fields, lanes; the "dependencies" doc hits are the +read-only Advanced Reporting export and UI guides). The UI's Dependencies tab +calls an undocumented endpoint. Nothing may be built against guessed shapes -- +the planned-date HTTP 422 (issue #52) is this repo's standing proof of why. + +## Phase 0 -- read-only discovery probe (this phase) + +`probe_dependencies.py`, stdlib-only, reads `.env` exactly like `smoke.py`. +Strictly read-only: every request it issues is a GET; a test pins this +invariant. No confirmation prompt is needed because nothing mutates. + +Behavior: + +1. Env check: requires `AGILEPLACE_TOKEN`, `AGILEPLACE_HOST`, + `AGILEPLACE_BOARD_ID` (same message pattern as smoke). +2. Pick a card: first card returned by `list_cards`, or `--card-id` override. +3. Baseline dump: GET the card and print its sorted top-level keys, flagging any + key containing "depend" -- dependencies may already be embedded in card reads. +4. Positive control: GET `card/{id}/connection/children` (documented, known + good). If the control fails, the probe's plumbing/credentials are broken and + candidate results are meaningless; say so and stop. +5. Probe candidates, printing one line each (`FOUND HTTP 200` / `MISS HTTP 404` + / `HTTP `) plus a truncated body for anything that is not a 404: + - `card/{id}/dependencies` + - `card/{id}/dependency` + - `dependencies` with `?cardId={id}` + - `dependency` with `?cardId={id}` + - `board/{boardId}/dependencies` + - `card/{id}/connection/dependencies` +6. Summary: list FOUND endpoints; if none, print the fallback instruction + (create one dependency in the UI with browser devtools open, capture the + request as cURL, record it in API-VALIDATION.md). + +The probe uses its own GET helper that returns `(status, body)` for every HTTP +outcome -- `agileplace.api` deliberately converts HTTP errors to `SystemExit`, +which is correct for the sync and wrong for a probe, where 404 is data. + +Probe findings land in API-VALIDATION.md (a "dependencies discovery" entry) in +the same evidence-recording style as every other live check. + +## Phase 1 -- sync integration (contingent on confirmed shapes) + +Not built until Phase 0 (plus, if needed, a smoke-style write probe on throwaway +cards) confirms read and write shapes. + +- `agileplace.py`: `card_dependencies(cfg, card_id)` (read), + `create_dependencies(cfg, apply, card_id, depends_on_ids)`, + `delete_dependencies(cfg, apply, card_id, depends_on_ids)` -- batch pair + bodies, as confirmed live. Direction blocker -> blocked, timing + `finishToStart` (GitHub blocked-by's one semantic is FS; the UI's other + timings -- SS, FF, SF -- are never written by the sync). +- Timing ownership: reconciliation matches dependencies by card PAIR only. A + human who refines a synced dependency's timing in the UI keeps that + refinement; the sync neither inspects nor rewrites `timing` after creation. +- `sync.py`, after the connections step: build the desired edge set from GitHub + blocked-by (already fetched for `blocked_reason`), read current dependencies + for managed cards, diff, create missing, delete unmatched. +- Authority rule: reconciliation touches a dependency ONLY when both endpoint + cards are sync-managed. Any dependency involving a non-managed card is + invisible to the sync, in both directions. +- Edge semantics: dependencies mirror ALL GitHub blocked-by edges, including + edges whose blocker is Done -- the edge is structural and AgilePlace's health + display shows satisfaction. This deliberately differs from the Blocked flag, + which continues to reflect incomplete blockers only. +- Failure semantics: a failed or malformed dependency read fails closed -- warn + and skip dependency reconciliation for that run (mirrors children reads). +- Dry run: planned dependency ops print with the same planned-card-id boundary + rules as connections; planned ids never escape the run. +- Tests: the desired-vs-current diff is a pure function, unit-tested directly; + the offline fake tenant grows dependency endpoints mirroring the CONFIRMED + shapes only. + +## Phase 2 -- Blocked-flag retirement (decided 2026-07-21) + +The maintainer judged the native depends-on card icon sufficient. Mechanism +(deliberately cruft-free -- no transitional clearing logic ships in sync.py): + +- sync.py loses ALL Blocked-flag code: the flag-text mirror in step 4, the + flag-clear during card retirement, and `stages.blocked_reason` (now dead). + The sync never writes `/isBlocked` or `/blockReason`; the flag belongs to + humans. `ops_blocked`/`card_is_blocked`/`card_block_reason` stay in + agileplace.py as validated API surface (smoke still exercises the shape). +- A one-shot, throwaway `clear_legacy_blocked_flags.py` batch-clears the flags + the old behavior left behind -- ONLY flags whose reason matches the old + sync's exact signature ("Blocked by #N[, #M...]"); human-authored flags are + never touched. Dry-run by default; idempotent; the script (and its + signature-match tests) are DELETED from the repo once the board is clean. + +## Process + +Issue #57 tracks the feature. Phase 0 ships on branch `feat/dependency-probe` +(no PR: Codex-reviewed locally, pushed for the maintainer to run against the +live tenant from another machine). Phase 1 branches after shapes are confirmed +and goes through the normal draft-PR review flow. diff --git a/probe_dependencies.py b/probe_dependencies.py new file mode 100644 index 0000000..8a11c80 --- /dev/null +++ b/probe_dependencies.py @@ -0,0 +1,154 @@ +"""Read-only discovery probe for the AgilePlace dependencies API (issue #57, Phase 0). + +The io v2 public docs do not document the Dependencies feature's endpoints, so this +probe answers one question against a real tenant: is there a readable dependencies +resource, and under which path? It issues GET requests only -- the read-only +invariant is pinned by tests/test_probe_dependencies.py -- so it needs no +confirmation prompt and is safe to run against a production board. + +Reads .env exactly like sync.py/smoke.py. Findings belong in API-VALIDATION.md. +Design: docs/superpowers/specs/2026-07-21-blocked-by-dependencies-design.md. + +Run: python probe_dependencies.py [--card-id ID] +""" +from __future__ import annotations + +import argparse +import urllib.error +import urllib.parse +import urllib.request + +import agileplace +from config import env_config + +BODY_EXCERPT = 500 + + +def probe_get(cfg: dict, path: str, params: dict | None = None) -> tuple[int, str]: + """(status, body_text) for one GET. Unlike agileplace.api, every HTTP status is + data here, never an error -- a probe's 404 is its answer, not a failure. + Transport failures (DNS, TLS, connection, timeout) return status 0 so the + caller can report them as inconclusive instead of dying in a traceback.""" + url = f"https://{cfg['host']}/io/{path.lstrip('/')}" + if params: + url += "?" + urllib.parse.urlencode(params) + req = urllib.request.Request(url, method="GET", headers={ + "Authorization": f"Bearer {cfg['token']}", "Accept": "application/json"}) + try: + with urllib.request.urlopen(req, timeout=agileplace.REQUEST_TIMEOUT) as resp: + return resp.status, resp.read().decode(errors="replace") + except urllib.error.HTTPError as err: + return err.code, err.read().decode(errors="replace") + except (urllib.error.URLError, TimeoutError) as err: + return 0, f"transport failure: {getattr(err, 'reason', None) or err}" + + +def candidate_probes(card_id: str, board_id: str) -> list[tuple[str, dict | None]]: + """Candidate (path, params) pairs for a readable dependencies resource.""" + return [ + (f"card/{card_id}/dependencies", None), + (f"card/{card_id}/dependency", None), + ("dependencies", {"cardId": card_id}), + ("dependency", {"cardId": card_id}), + (f"board/{board_id}/dependencies", None), + (f"card/{card_id}/connection/dependencies", None), + ] + + +def _require_env() -> dict: + cfg = env_config() + missing = [env for env, key in (("AGILEPLACE_TOKEN", "token"), ("AGILEPLACE_HOST", "host"), + ("AGILEPLACE_BOARD_ID", "board_id")) if not cfg.get(key)] + if missing: + raise SystemExit(f"dependency probe needs {', '.join(missing)} set (.env) -- refusing to run") + return cfg + + +def _dump_card_keys(cfg: dict, card_id: str) -> None: + card = agileplace.get_card(cfg, card_id) + keys = sorted(card) + dependish = [k for k in keys if "depend" in k.lower()] + print(f"card GET top-level keys: {', '.join(keys)}") + print("dependency-ish keys: " + + (", ".join(dependish) if dependish else "none -- not embedded in card reads")) + + +def _check_control(cfg: dict, card_id: str) -> None: + """A documented, known-good GET. If this fails, credentials/plumbing are broken + and every candidate MISS below would be meaningless -- stop instead.""" + path = f"card/{card_id}/connection/children" + status, body = probe_get(cfg, path, {"limit": 1, "offset": 0}) + if status != 200: + raise SystemExit(f"control endpoint {path} returned HTTP {status} -- probe plumbing or " + f"credentials are broken; candidate results would be meaningless\n" + f"{body[:BODY_EXCERPT]}") + print(f"control {path} -> HTTP 200 (plumbing OK)") + + +def _probe_candidates(cfg: dict, card_id: str) -> tuple[list[str], list[str]]: + """(found, inconclusive). Only 2xx counts as found (204 = route exists, empty + collection) and only 404 counts as a miss; anything else -- 429, 5xx, auth + errors, transport failures -- is inconclusive and must block the all-missing + conclusion.""" + found, inconclusive = [], [] + for path, params in candidate_probes(card_id, str(cfg["board_id"])): + shown = path + ("?" + urllib.parse.urlencode(params) if params else "") + status, body = probe_get(cfg, path, params) + if 200 <= status < 300: + found.append(shown) + print(f"FOUND {shown} -> HTTP {status}") + print(f" {body[:BODY_EXCERPT] if body else '(empty body)'}") + elif status == 404: + print(f"MISS {shown} -> HTTP 404") + else: + inconclusive.append(shown) + print(f"? {shown} -> {f'HTTP {status}' if status else 'no HTTP response'}") + print(f" {body[:BODY_EXCERPT]}") + return found, inconclusive + + +def _summarize(found: list[str], inconclusive: list[str]) -> None: + print("\n--- probe summary ---") + if found: + print("readable dependency endpoint(s) found:") + for endpoint in found: + print(f" {endpoint}") + print("record the response shapes in API-VALIDATION.md; next step is the write probe " + "(issue #57, Phase 0b)") + return + if inconclusive: + print("no candidate answered 200, but these were INCONCLUSIVE (not a clean 404):") + for endpoint in inconclusive: + print(f" {endpoint}") + print("re-run the probe before drawing conclusions -- rate limits and transport hiccups") + print("look like this; only a clean 404 on every candidate rules the read surface out.") + return + print("no readable dependency endpoint found among the candidates (every one returned 404).") + print("fallback: open browser devtools (Network tab), create ONE dependency between two") + print("cards in the AgilePlace UI, and capture the request(s) -- method, URL, JSON body") + print("('Copy as cURL' is ideal). Record them in API-VALIDATION.md (issue #57).") + + +def _first_card_id(cfg: dict) -> str: + cards = agileplace.list_cards(cfg) + if not cards: + raise SystemExit("board has no cards to probe against -- pass --card-id") + return str(cards[0]["id"]) + + +def main() -> None: + parser = argparse.ArgumentParser( + description="Read-only probe for the undocumented AgilePlace dependencies API (issue #57)") + parser.add_argument("--card-id", help="probe against this card instead of the board's first card") + args = parser.parse_args() + cfg = _require_env() + + card_id = args.card_id or _first_card_id(cfg) + print(f"Probing dependencies read surface with card {card_id} on board {cfg['board_id']}") + _dump_card_keys(cfg, card_id) + _check_control(cfg, card_id) + _summarize(*_probe_candidates(cfg, card_id)) + + +if __name__ == "__main__": + main() diff --git a/smoke.py b/smoke.py index b4eb68e..20ed1e8 100644 --- a/smoke.py +++ b/smoke.py @@ -160,7 +160,9 @@ def _date_matches(value, expected: str | None) -> bool: def _check_blocked_and_dates(cfg: dict, parent_id: str, results: list) -> None: - """Steps 5-6: blocked-state and planned-date writes -- the sync's remaining PATCH shapes.""" + """Steps 5-6: blocked-state and planned-date write round-trips. Planned dates are a sync + write shape; the blocked-state ops validate API surface only -- the sync never writes the + flag (issue #57 Phase 2).""" _step(5, "set blocked state + planned dates in one versioned PATCH") fresh = agileplace.get_card(cfg, parent_id) ops = [*agileplace.ops_blocked(True, "smoke block"), @@ -237,9 +239,50 @@ def _check_connections(cfg: dict, parent_id: str, child_id: str, results: list) f"children read back: {sorted(children) if children is not None else 'unavailable'}")) +def _check_dependencies(cfg: dict, parent_id: str, child_id: str, results: list) -> None: + """Steps 11-12: dependency create/read/delete round-trip through the shapes captured live + 2026-07-21 (undocumented API -- see API-VALIDATION.md "Dependencies API discovery"), plus a + pin on the confirmed duplicate-create contract (HTTP 409 Conflict, nothing else passes).""" + _step(11, "create dependency child dependsOn parent, then read it back") + agileplace.create_dependencies(cfg, True, child_id, [parent_id]) + entries = agileplace.card_dependencies(cfg, child_id) + incoming = agileplace.incoming_dependency_ids(entries) if entries is not None else None + timing_ok = entries is not None and any( + e.get("direction") == "incoming" and str(e.get("cardId")) == parent_id + and e.get("timing") == agileplace.DEPENDENCY_TIMING for e in entries) + results.append(("dependency create + incoming read round-trip", + incoming == {parent_id} and timing_ok, + f"incoming read back: {sorted(incoming) if incoming is not None else 'unavailable'}")) + + # The duplicate-create contract was confirmed live 2026-07-21: HTTP 409 Conflict. Only that + # exact rejection passes -- acceptance contradicts the recorded contract, and a 401/5xx/ + # transport failure must fail the run, never hide behind an informational line. + try: + agileplace.create_dependencies(cfg, True, child_id, [parent_id]) + dup_ok = False + dup_detail = "duplicate create ACCEPTED -- contradicts the confirmed HTTP 409 contract" + except SystemExit as exc: + dup_ok = getattr(exc, "http_status", None) == 409 + dup_detail = (f"duplicate create rejected: {exc}" if dup_ok + else f"unexpected failure (not the confirmed 409): {exc}") + entries = agileplace.card_dependencies(cfg, child_id) + incoming = agileplace.incoming_dependency_ids(entries) if entries is not None else None + dup_count = (sum(1 for e in entries if e.get("direction") == "incoming" + and str(e.get("cardId")) == parent_id) if entries is not None else None) + results.append(("duplicate dependency create rejected (HTTP 409)", dup_ok, + f"{dup_detail}; incoming entries for parent now: {dup_count}")) + + _step(12, "delete the dependency, then confirm the empty read") + agileplace.delete_dependencies(cfg, True, child_id, [parent_id]) + entries = agileplace.card_dependencies(cfg, child_id) + incoming = agileplace.incoming_dependency_ids(entries) if entries is not None else None + results.append(("dependency delete + empty read round-trip", incoming == set(), + f"incoming read back: {sorted(incoming) if incoming is not None else 'unavailable'}")) + + def _check_stale_patch(cfg: dict, parent_id: str, stale_version: str, results: list) -> None: - """Step 9: a stale x-lk-resource-version PATCH must be rejected, not silently applied.""" - _step(11, "deliberately stale-version PATCH (the server MUST reject this)") + """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)") body = [agileplace.op_tag("smoke-stale")] print(f" PATCH /io/card/{parent_id} with stale x-lk-resource-version={stale_version} " f"body={json.dumps(body)}") @@ -294,6 +337,7 @@ def _run_checks(cfg: dict, lane_id: str | None, run_id: str, created: list[str], _check_blocked_and_dates(cfg, parent_id, results) 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_stale_patch(cfg, parent_id, baseline_version, results) diff --git a/stages.py b/stages.py index 6788c0e..10aa6db 100644 --- a/stages.py +++ b/stages.py @@ -85,15 +85,6 @@ def title_key(title: str) -> str | None: return None -def blocked_reason(blockers: list[int], stage_by_number: dict) -> str | None: - """A card is Blocked while any of its GitHub blocked-by issues isn't Done. Returns the reason string - (naming the incomplete blockers) or None when nothing incomplete blocks it. Pure -- unit-tested.""" - incomplete = sorted(b for b in blockers if stage_by_number.get(b) != "Done") - if not incomplete: - return None - return "Blocked by " + ", ".join(f"#{b}" for b in incomplete) - - def epic_key_for_task(task_key: str) -> str | None: """Convention fallback used only when native sub-issues are unavailable: task key '0C2' -> epic key 'EP-0C' (strip the trailing task number).""" diff --git a/sync.py b/sync.py index c3c6fe2..e51bb7e 100644 --- a/sync.py +++ b/sync.py @@ -5,8 +5,8 @@ Per run: ensure a card per active issue (matched by URL, then customId); retire existing cards for NOT_PLANNED/DUPLICATE issues; move each active card to the lane for its stage (Projects v2 Status = source of truth, label/PR fallback); mirror sub-issues as parent/child connections; mirror blocked-by -as the card Blocked state; bidirectionally reconcile labels/milestone <-> tags and planned dates <-> -Project date fields. Every mutation to one card is batched into a single versioned PATCH (optimistic +as native card dependencies (the Blocked flag is human-owned -- never written by the sync); +bidirectionally reconcile labels/milestone <-> tags and planned dates <-> Project date fields. Every mutation to one card is batched into a single versioned PATCH (optimistic concurrency). DRY RUN by default. State is target-scoped, issue-URL-keyed, records each issue's card id (so a @@ -18,13 +18,14 @@ import json import os import tempfile +from collections.abc import Mapping import agileplace import ghkit import ghproject from config import STATE_FILE, env_config from reconcile import reconcile, reconcile_value -from stages import (blocked_reason, epic_key_for_task, is_retired_issue, issue_stage, +from stages import (epic_key_for_task, is_retired_issue, issue_stage, normalize_status, title_key) MS_PREFIX = "milestone:" @@ -107,6 +108,65 @@ def _child_connection_changes(desired: set[str], existing: set[str], managed: se return adds, removes +def _dependency_changes(desired: set[str], current: set[str], + managed: set[str]) -> tuple[list[str], list[str]]: + """Dependency additions and safe removals for one card. Removals stay limited to + dependencies on cards managed by this sync -- a human-made dependency involving any + other card is invisible to reconciliation, in both directions.""" + return sorted(desired - current), sorted((current & managed) - desired) + + +def _blocker_cards(by_number: dict, card_for, retired_issues: list, + retired_card_by_url: dict) -> dict: + """Issue number -> card for every issue that can act as a blocker -- retired + (NOT_PLANNED/DUPLICATE) issues included, via their URL-owned cards. A retired Done + blocker's edge is structural: resolving blockers through active issues only would drop + it from the desired set while its card stayed in the managed set, deleting the valid + native dependency as stale on every run.""" + cards = {number: card for number, issue in by_number.items() + if (card := card_for(issue)) and card.get("id")} + for issue in retired_issues: + card = retired_card_by_url.get(issue["url"]) + if card and card.get("id"): + cards.setdefault(issue["number"], card) + return cards + + +def sync_dependencies(cfg: dict, apply: bool, syncable_issues: list, blocked_by: dict, + blocker_card_by_number: dict, card_for, managed_card_ids: set[str]) -> None: + """Mirror GitHub blocked-by edges as native AgilePlace dependencies (issue #57). + + EVERY edge is mirrored, including edges whose blocker is Done -- the edge is structural, and + AgilePlace's own dependencyStats display satisfaction. (The Blocked flag deliberately differs: + it reflects incomplete blockers only.) GitHub is authoritative only between two sync-managed + cards. A failed or unrecognized dependency read skips the card entirely: duplicate-create + behavior is unconfirmed live, so nothing is ever re-created against unknown state.""" + for issue in syncable_issues: + card = card_for(issue) + if not card or not card.get("id"): + continue + desired = {str(blocker_card_by_number[number]["id"]) + for number in blocked_by.get(issue["number"], []) + if number in blocker_card_by_number} + cid = str(card["id"]) + key = issue_custom_id(issue) + if card.get("_planOnly"): + current = set() # a fresh card has no server-side dependencies; never read a plan-only id + else: + entries = agileplace.card_dependencies(cfg, cid) + if entries is None: + print(f"WARN [{key}] dependency state unknown -- leaving this card's dependencies untouched") + continue + current = agileplace.incoming_dependency_ids(entries) + adds, removes = _dependency_changes(desired, current, managed_card_ids) + if adds: + agileplace.create_dependencies(cfg, apply, cid, adds) + print(f"{'dep ' if apply else 'DRY '} [{key}] +{len(adds)} dependency(ies)") + if removes: + agileplace.delete_dependencies(cfg, apply, cid, removes) + print(f"{'undep ' if apply else 'DRY '} [{key}] -{len(removes)} dependency(ies)") + + def issue_card_title(issue: dict) -> str: t = issue["title"] k = title_key(t) @@ -212,7 +272,7 @@ def _protect_open_pr_stage(stage: str, current_lane_id: str, lanes: list, milest def _retire_card(issue: dict, card: dict, lanes: list, stage_map: dict | None, apply: bool, queue) -> None: - """Move one URL-matched retired issue card to Done and clear its stale blocked state.""" + """Move one URL-matched retired issue card to Done (lane only -- flags are human-owned).""" key = issue_custom_id(issue) reason = issue["state_reason"] if not card.get("id"): @@ -230,9 +290,6 @@ def _retire_card(issue: dict, card: dict, lanes: list, stage_map: dict | None, actions.append(f"already '{agileplace.lane_title(target)}'") else: print(f"WARN [{key}] cannot retire to Done: no unambiguous Done lane ({reason})") - if agileplace.card_is_blocked(card) or agileplace.card_block_reason(card): - ops.extend(agileplace.ops_blocked(False, None)) - actions.append("clear blocked") if ops: queue(card, ops, f"retire:{reason}") action = "; ".join(actions) or "no card changes available" @@ -393,6 +450,29 @@ def sync_dates(cfg, apply, issue, card, pitem, field_meta, issues_state, queue) prev[kind] = new +def _created_card_snapshot(cfg: dict, created: Mapping) -> Mapping: + """Refetch a just-created card once so its authoritative server state becomes the snapshot. + + The live create response is sparse -- the new id, but no version and no customId/laneId echo + (validated live 2026-07-21, issue #55). Indexed as-is it queues redundant /customId and /laneId + ops that the issue-#8 stale-ops guard then reads as a concurrent edit, aborting every fresh + create+sync apply. The refetched card carries a usable version, so the later PATCH skips its own + refetch: net API calls are unchanged. A failed or mismatched refetch falls back to the sparse + response (the pre-fix behavior) rather than failing a run whose create already succeeded. + """ + try: + fresh = agileplace.get_card(cfg, created["id"]) + except SystemExit as err: + print(f"WARN refetch of created card {created['id']} failed ({err}) -- " + "keeping the sparse create response as its snapshot") + return created + if str(fresh.get("id") or "") != str(created["id"]): + print(f"WARN refetch of created card {created['id']} returned card {fresh.get('id')!r} -- " + "keeping the sparse create response as its snapshot") + return created + return fresh + + def main() -> None: parser = argparse.ArgumentParser(description="Sync GitHub -> AgilePlace (per-issue cards, lanes, connections, metadata)") parser.add_argument("--apply", action="store_true", help="actually write (default: verbose dry run)") @@ -563,6 +643,8 @@ def queue(card, ops, note): lane, _ = agileplace.resolve_lane_for_stage(lanes, stage, issue.get("milestone") or "", smap) created = agileplace.create_card(cfg, apply, issue_card_title(issue), key, issue["url"], lane["id"] if lane else None) + if apply and created.get("id"): + created = _created_card_snapshot(cfg, created) # Real creates carry a server id; dry creates carry an obvious plan-only id. Index either # snapshot so metadata, hierarchy, dependency, and batched patch planning stay in parity. # Dry-run state is never saved, so the plan-only identity cannot escape this run. @@ -642,23 +724,19 @@ def queue(card, ops, note): agileplace.disconnect_children(cfg, apply, str(parent["id"]), removes) print(f"{'unlink' if apply else 'DRY '} [{key}] -{len(removes)} child card(s)") - # 4) dependencies -> card Blocked state (skip entirely unless the whole blocked-by snapshot is complete) + # 4) GitHub blocked-by edges -> native card dependencies (issue #57) -- all edges, managed + # pairs only, retired Done blockers resolving through their URL-owned cards. Skip entirely + # unless the whole blocked-by snapshot is complete. The card Blocked flag belongs to humans: + # the sync never writes /isBlocked or /blockReason (the old flag-text mirror was retired in + # issue #57 Phase 2; clear_legacy_blocked_flags.py cleaned up what it left behind). blocked_by = (ghkit.blocked_by_map(cfg, [i["number"] for i in syncable_issues]) if online and syncable_issues else {} if online else None) if online and blocked_by is None: - print("WARN blocked-by snapshot incomplete -- leaving ALL card Blocked states untouched this run") + print("WARN blocked-by snapshot incomplete -- leaving ALL card dependencies untouched this run") if blocked_by is not None: - stage_by_number = {i["number"]: resolve_issue_stage(i, project_status) for i in issues} - for issue in syncable_issues: - card = card_for(issue) - if not card or not card.get("id"): - continue - reason = blocked_reason(blocked_by.get(issue["number"], []), stage_by_number) - want = reason is not None - if want != agileplace.card_is_blocked(card) or (want and reason != agileplace.card_block_reason(card)): - queue(card, agileplace.ops_blocked(want, reason), f"{'block' if want else 'unblock'}") - key = issue_custom_id(issue) - print(f"{'block ' if want else 'unblock'} [{key}]{': ' + reason if reason else ''}") + sync_dependencies(cfg, apply, syncable_issues, blocked_by, + _blocker_cards(by_number, card_for, retired_issues, retired_card_by_url), + card_for, managed_card_ids) # 5) flush: ONE versioned PATCH per card (optimistic concurrency) for entry in card_ops.values(): diff --git a/tests/test_agileplace.py b/tests/test_agileplace.py index 7d72646..cd8f011 100644 --- a/tests/test_agileplace.py +++ b/tests/test_agileplace.py @@ -22,6 +22,7 @@ get_card, list_cards, op_custom_id, + op_planned_date, op_tag, ops_blocked, ops_tag_remove, @@ -261,6 +262,22 @@ def test_ops_blocked_unblock_forces_empty_reason(): assert ops[1] == {"op": "add", "path": "/blockReason", "value": ""} +# --- op_planned_date ------------------------------------------------------ + +def test_op_planned_date_set_is_a_string_replace(): + op = op_planned_date("plannedStart", "2026-01-01") + assert op == {"op": "replace", "path": "/plannedStart", "value": "2026-01-01"} + + +def test_op_planned_date_clear_is_a_remove_never_a_null_replace(): + """The live server type-validates replace values on /plannedStart and /plannedFinish as strings + and 422s on null ("Invalid value: must be string", observed live 2026-07-21). Clearing a date + must be an RFC-6902 remove, which carries no value member at all (issue #52).""" + op = op_planned_date("plannedFinish", None) + assert op == {"op": "remove", "path": "/plannedFinish"} + assert "value" not in op + + # --- op_custom_id --------------------------------------------------------- def test_op_custom_id_replaces_the_card_custom_id(): diff --git a/tests/test_agileplace_dependencies.py b/tests/test_agileplace_dependencies.py new file mode 100644 index 0000000..832ab5e --- /dev/null +++ b/tests/test_agileplace_dependencies.py @@ -0,0 +1,146 @@ +"""Unit tests for agileplace's dependency transport (issue #57, Phase 1). + +Shapes live-confirmed 2026-07-21 (see API-VALIDATION.md "Dependencies API discovery"): + read GET /io/card/{cardId}/dependency -> {"dependencies": [{direction, cardId, timing, ...}]} + create POST /io/card/dependency {"cardIds": [dep], "dependsOnCardIds": [...], "timing": "finishToStart"} + delete DELETE /io/card/dependency {"cardIds": [dep], "dependsOnCardIds": [...]} + +The invariants: reads fail CLOSED (None = unknown, never "no dependencies"); writes send +exactly the confirmed bodies; empty id lists are no-ops that never touch the network. + +Run: pytest -q +""" +import email.message +import io +import json +import sys +import urllib.error +import urllib.request +from pathlib import Path + +import pytest + +sys.path.insert(0, str(Path(__file__).resolve().parent.parent)) + +from agileplace import ( # noqa: E402 + card_dependencies, + create_dependencies, + delete_dependencies, + incoming_dependency_ids, +) + +CFG = {"token": "t", "host": "h", "board_id": "b1"} + + +class _Response: + def __init__(self, payload: object): + self._payload = json.dumps(payload).encode() + + def __enter__(self): + return self + + def __exit__(self, *_args): + return False + + def read(self) -> bytes: + return self._payload + + +def _serve(monkeypatch, payload=None, *, error_code=None): + calls = [] + + def fake_urlopen(req, timeout=None): + calls.append((req.get_method(), req.full_url, json.loads(req.data) if req.data else None)) + if error_code is not None: + raise urllib.error.HTTPError(req.full_url, error_code, "err", + email.message.Message(), io.BytesIO(b'{"m":"x"}')) + return _Response(payload) + + monkeypatch.setattr(urllib.request, "urlopen", fake_urlopen) + return calls + + +# --- card_dependencies (read, fail-closed) -------------------------------- + +def test_card_dependencies_returns_entries(monkeypatch): + entries = [{"direction": "incoming", "cardId": "B1", "timing": "finishToStart"}] + calls = _serve(monkeypatch, {"dependencies": entries}) + assert card_dependencies(CFG, "C1") == entries + method, url, body = calls[0] + assert (method, body) == ("GET", None) + assert url.endswith("/io/card/C1/dependency") + + +def test_card_dependencies_http_failure_returns_none_with_warn(monkeypatch, capsys): + _serve(monkeypatch, error_code=500) + assert card_dependencies(CFG, "C1") is None + assert "WARN" in capsys.readouterr().out + + +@pytest.mark.parametrize("payload", [ + {"dependencies": "nope"}, # not a list + {"dependencies": [{"ok": 1}, 7]}, # non-dict entry + {"unexpected": []}, # missing key + [], # not even an object + # gpt-5.6-sol review P2: an incomplete snapshot acted on authoritatively re-creates an + # existing pair -> the live 409 aborts the run. Entries must carry a usable + # direction/cardId, and a paginating response can no longer be assumed complete. + {"dependencies": [{"cardId": "B1"}]}, # no direction + {"dependencies": [{"direction": "incoming"}]}, # no cardId + {"dependencies": [{"direction": "sideways", "cardId": "B1"}]}, # unknown direction + {"dependencies": [], "pageMeta": {"totalRecords": 7}}, # server began paginating +]) +def test_card_dependencies_malformed_shapes_return_none(monkeypatch, capsys, payload): + _serve(monkeypatch, payload) + assert card_dependencies(CFG, "C1") is None + assert "WARN" in capsys.readouterr().out + + +def test_incoming_dependency_ids_filters_direction_and_missing_ids(): + entries = [ + {"direction": "incoming", "cardId": "B1", "timing": "finishToStart"}, + {"direction": "incoming", "cardId": 42}, # non-str id normalizes + {"direction": "outgoing", "cardId": "X9"}, # not a blocker of this card + {"direction": "incoming"}, # no cardId -> ignored + {"cardId": "Y2"}, # no direction -> ignored + ] + assert incoming_dependency_ids(entries) == {"B1", "42"} + + +# --- create/delete (writes, confirmed bodies) ----------------------------- + +def test_create_dependencies_sends_confirmed_body(monkeypatch): + calls = _serve(monkeypatch, {}) + create_dependencies(CFG, True, "C1", {"B2", "B1"}) + method, url, body = calls[0] + assert method == "POST" + assert url.endswith("/io/card/dependency") + assert body == {"cardIds": ["C1"], "dependsOnCardIds": ["B1", "B2"], + "timing": "finishToStart"} + + +def test_delete_dependencies_sends_confirmed_body_without_timing(monkeypatch): + calls = _serve(monkeypatch, {}) + delete_dependencies(CFG, True, "C1", {"B1"}) + method, url, body = calls[0] + assert method == "DELETE" + assert url.endswith("/io/card/dependency") + assert body == {"cardIds": ["C1"], "dependsOnCardIds": ["B1"]} + + +def test_empty_or_falsy_ids_are_network_free_no_ops(monkeypatch): + calls = _serve(monkeypatch, {}) + create_dependencies(CFG, True, "C1", set()) + create_dependencies(CFG, True, "C1", {"", None}) + delete_dependencies(CFG, True, "C1", []) + assert calls == [] + + +def test_dry_run_prints_instead_of_sending(monkeypatch, capsys): + calls = _serve(monkeypatch, {}) + create_dependencies(CFG, False, "C1", {"B1"}) + delete_dependencies(CFG, False, "C1", {"B1"}) + out = capsys.readouterr().out + assert calls == [] + assert "DRY POST /io/card/dependency" in out + assert "DRY DELETE /io/card/dependency" in out diff --git a/tests/test_hierarchy_ownership.py b/tests/test_hierarchy_ownership.py index 3818284..035c749 100644 --- a/tests/test_hierarchy_ownership.py +++ b/tests/test_hierarchy_ownership.py @@ -78,6 +78,7 @@ def _run_hierarchy(tmp_path: Path, monkeypatch, issues: list[dict], cards: list[ monkeypatch.setattr(agileplace, "board_layout", lambda _cfg: []) monkeypatch.setattr(agileplace, "list_cards", lambda _cfg: cards) monkeypatch.setattr(agileplace, "card_child_ids", child_reads) + monkeypatch.setattr(agileplace, "card_dependencies", lambda *_args: []) monkeypatch.setattr( agileplace, "create_card", diff --git a/tests/test_probe_dependencies.py b/tests/test_probe_dependencies.py new file mode 100644 index 0000000..1c58cda --- /dev/null +++ b/tests/test_probe_dependencies.py @@ -0,0 +1,201 @@ +"""Offline tests for probe_dependencies.py (issue #57 Phase 0). + +The invariant that matters most: the probe is READ-ONLY -- every request it ever +issues is a GET. A discovery tool that writes to a production tenant is the bug +class this file exists to make impossible. The rest pins the reporting contract: +one line per candidate, body excerpts for non-404s, and the devtools fallback +instruction when nothing is found. + +Run: pytest -q +""" +import email.message +import io +import json +import sys +import urllib.error +import urllib.parse +import urllib.request +from pathlib import Path + +import pytest + +sys.path.insert(0, str(Path(__file__).resolve().parent.parent)) + +import config # noqa: E402 +import probe_dependencies # noqa: E402 + + +class _Response: + def __init__(self, payload: object, status: int = 200): + self.status = status + self._payload = json.dumps(payload).encode() if payload is not None else b"" + + def __enter__(self): + return self + + def __exit__(self, *_args): + return False + + def read(self) -> bytes: + return self._payload + + +def _http_error(url: str, code: int, body: str) -> urllib.error.HTTPError: + return urllib.error.HTTPError(url, code, "error", email.message.Message(), + io.BytesIO(body.encode())) + + +class FakeTenant: + """Read-side io v2 double: serves the card list, the card, and the control + endpoint; candidate endpoints 404 unless listed in ``found``.""" + + def __init__(self, *, control_status: int = 200, found: dict | None = None, + card: dict | None = None, troubles: dict | None = None): + self.methods: list[str] = [] + self.paths: list[str] = [] + self.card = card or {"id": "C1", "title": "Card", "customId": "X1", "version": 2} + self.found = found or {} + self.troubles = troubles or {} # path -> HTTP status int, or an exception to raise + self.control_status = control_status + + def urlopen(self, req, timeout=None): + self.methods.append(req.get_method()) + path = urllib.parse.urlparse(req.full_url).path.removeprefix("/io/") + self.paths.append(path) + cid = self.card["id"] + if path == "card": + return _Response({"pageMeta": {"totalRecords": 1, "offset": 0, "limit": 200}, + "cards": [self.card]}) + if path == f"card/{cid}": + return _Response(self.card) + if path == f"card/{cid}/connection/children": + if self.control_status != 200: + raise _http_error(req.full_url, self.control_status, '{"message": "boom"}') + return _Response({"pageMeta": {"totalRecords": 0, "offset": 0, "limit": 1}, + "cards": []}) + if path in self.troubles: + trouble = self.troubles[path] + if isinstance(trouble, Exception): + raise trouble + if trouble == 204: + return _Response(None, status=204) + raise _http_error(req.full_url, trouble, '{"message": "trouble"}') + if path in self.found: + return _Response(self.found[path]) + raise _http_error(req.full_url, 404, '{"message": "not found"}') + + +@pytest.fixture +def tenant_env(monkeypatch): + def _install(tenant: FakeTenant): + monkeypatch.setenv("AGILEPLACE_TOKEN", "t") + monkeypatch.setenv("AGILEPLACE_HOST", "tenant.test") + monkeypatch.setenv("AGILEPLACE_BOARD_ID", "42") + monkeypatch.setattr(urllib.request, "urlopen", tenant.urlopen) + return _install + + +def _run(monkeypatch, capsys, argv=("--card-id", "C1")) -> str: + monkeypatch.setattr(sys, "argv", ["probe_dependencies.py", *argv]) + probe_dependencies.main() + return capsys.readouterr().out + + +def test_probe_never_issues_non_get_requests(tenant_env, monkeypatch, capsys): + tenant = FakeTenant(found={"card/C1/dependencies": {"dependencies": []}}) + tenant_env(tenant) + _run(monkeypatch, capsys) + assert tenant.methods and set(tenant.methods) == {"GET"} + + +def test_all_candidates_missing_prints_miss_lines_and_devtools_fallback( + tenant_env, monkeypatch, capsys): + tenant = FakeTenant() + tenant_env(tenant) + out = _run(monkeypatch, capsys) + for path, params in probe_dependencies.candidate_probes("C1", "42"): + shown = path + ("?" + urllib.parse.urlencode(params) if params else "") + assert f"MISS {shown} -> HTTP 404" in out + assert "no readable dependency endpoint" in out + assert "devtools" in out + + +def test_found_candidate_is_reported_with_body_excerpt(tenant_env, monkeypatch, capsys): + tenant = FakeTenant(found={"card/C1/dependencies": {"dependencies": [{"id": "D1"}]}}) + tenant_env(tenant) + out = _run(monkeypatch, capsys) + assert "FOUND card/C1/dependencies -> HTTP 200" in out + assert '"D1"' in out + assert "readable dependency endpoint(s) found" in out + + +def test_card_key_dump_flags_dependency_ish_keys(tenant_env, monkeypatch, capsys): + card = {"id": "C1", "title": "Card", "version": 2, "dependencyCounts": {"incoming": 0}} + tenant = FakeTenant(card=card) + tenant_env(tenant) + out = _run(monkeypatch, capsys) + assert "dependencyCounts" in out + assert "dependency-ish keys: dependencyCounts" in out + + +def test_no_dependency_ish_keys_is_stated_explicitly(tenant_env, monkeypatch, capsys): + tenant = FakeTenant() + tenant_env(tenant) + out = _run(monkeypatch, capsys) + assert "dependency-ish keys: none" in out + + +def test_control_failure_aborts_before_probing_candidates(tenant_env, monkeypatch, capsys): + tenant = FakeTenant(control_status=500) + tenant_env(tenant) + monkeypatch.setattr(sys, "argv", ["probe_dependencies.py", "--card-id", "C1"]) + with pytest.raises(SystemExit, match="control"): + probe_dependencies.main() + candidate_paths = {p for p, _ in probe_dependencies.candidate_probes("C1", "42")} + assert not candidate_paths & set(tenant.paths) + + +def test_auto_picks_first_board_card_when_no_card_id(tenant_env, monkeypatch, capsys): + tenant = FakeTenant() + tenant_env(tenant) + out = _run(monkeypatch, capsys, argv=()) + assert "card C1" in out + assert "card" in tenant.paths # the list read happened + + +def test_missing_env_refuses_to_run(monkeypatch, tmp_path): + # env_config() re-reads .env on every call ("real environment variables win over + # it"), so a configured checkout would repopulate the deleted vars and pass + # _require_env(). Point ENV_FILE somewhere empty so this test is deterministic + # everywhere -- including the machine the probe is actually run from. + monkeypatch.setattr(config, "ENV_FILE", tmp_path / "no-such.env") + for var in ("AGILEPLACE_TOKEN", "AGILEPLACE_HOST", "AGILEPLACE_BOARD_ID"): + monkeypatch.delenv(var, raising=False) + monkeypatch.setattr(sys, "argv", ["probe_dependencies.py"]) + with pytest.raises(SystemExit, match="AGILEPLACE_TOKEN"): + probe_dependencies.main() + + +def test_204_counts_as_found_route_not_a_miss(tenant_env, monkeypatch, capsys): + tenant = FakeTenant(troubles={"card/C1/dependencies": 204}) + tenant_env(tenant) + out = _run(monkeypatch, capsys) + assert "FOUND card/C1/dependencies -> HTTP 204" in out + assert "(empty body)" in out + assert "readable dependency endpoint(s) found" in out + + +def test_transport_failure_is_inconclusive_not_a_miss_and_no_traceback( + tenant_env, monkeypatch, capsys): + tenant = FakeTenant(troubles={ + "card/C1/dependencies": urllib.error.URLError("dns exploded"), + "card/C1/dependency": 503, + }) + tenant_env(tenant) + out = _run(monkeypatch, capsys) # completing at all proves no traceback escaped + assert "? card/C1/dependencies -> no HTTP response" in out + assert "transport failure" in out + assert "? card/C1/dependency -> HTTP 503" in out + assert "INCONCLUSIVE" in out + assert "re-run the probe" in out + assert "devtools" not in out # the definitive all-404 fallback must NOT fire diff --git a/tests/test_retired_issues.py b/tests/test_retired_issues.py index 1780ae5..9b8230e 100644 --- a/tests/test_retired_issues.py +++ b/tests/test_retired_issues.py @@ -12,7 +12,6 @@ import ghkit # noqa: E402 import sync # noqa: E402 -from stages import blocked_reason # noqa: E402 _PROJECT_DISABLED = object() @@ -66,6 +65,7 @@ def _run_main(tmp_path, monkeypatch, raw_issues, cards, blocked_by=None, lanes=( stack.enter_context(patch("ghproject.hydrate_item_dates", return_value=project_snapshot)) stack.enter_context(patch("agileplace.board_layout", return_value=list(lanes))) stack.enter_context(patch("agileplace.list_cards", return_value=cards)) + stack.enter_context(patch("agileplace.card_dependencies", return_value=[])) create_card = stack.enter_context(patch("agileplace.create_card", return_value={})) patch_card = stack.enter_context(patch("agileplace.patch_card")) with stack, patch("sync.env_config", return_value=_config(tmp_path)), \ @@ -89,7 +89,9 @@ def _card(number: int, lane_id: str, *, blocked: bool) -> dict: } -def test_retired_issues_remain_known_done_blockers(monkeypatch): +def test_retired_issues_resolve_to_done_stage(monkeypatch): + """Retired (NOT_PLANNED/DUPLICATE) issues normalize to stage Done -- this drives lane + retirement, and (via _blocker_cards) keeps their dependency edges resolvable.""" raw_issues = [ _github_issue(10, "not_planned"), _github_issue(11, "DUPLICATE"), @@ -109,7 +111,6 @@ def test_retired_issues_remain_known_done_blockers(monkeypatch): assert [issue["state_reason"] for issue in issues] == ["NOT_PLANNED", "DUPLICATE"] assert stage_by_number == {10: "Done", 11: "Done"} - assert blocked_reason([10, 11], stage_by_number) is None def test_retired_issue_without_card_is_not_created(tmp_path, monkeypatch): @@ -125,8 +126,11 @@ def test_retired_issue_without_card_is_not_created(tmp_path, monkeypatch): blocked_by_read.assert_not_called() -def test_existing_retired_card_is_retired_and_unblocks_dependent( +def test_existing_retired_card_is_retired_and_flags_stay_human_owned( tmp_path, monkeypatch, capsys): + """Since issue #57 Phase 2 the sync never writes /isBlocked or /blockReason: retirement + moves the lane and nothing else, and a dependent card's flag is not 'unblocked' by the + sync -- the native dependency (and its health display) carries that signal now.""" dependent = { **_github_issue(20, ""), "state": "OPEN", @@ -149,7 +153,7 @@ def test_existing_retired_card_is_retired_and_unblocks_dependent( ) out = capsys.readouterr().out - assert "DRY retire [10] -> 'Done'; clear blocked (NOT_PLANNED)" in out + assert "DRY retire [10] -> 'Done' (NOT_PLANNED)" in out create_card.assert_not_called() edit_label.assert_not_called() blocked_by_read.assert_called_once_with(_config(tmp_path), [20]) @@ -157,14 +161,10 @@ def test_existing_retired_card_is_retired_and_unblocks_dependent( assert ops_by_card == { "C10": [ {"op": "replace", "path": "/laneId", "value": "L5"}, - {"op": "replace", "path": "/isBlocked", "value": False}, - {"op": "add", "path": "/blockReason", "value": ""}, - ], - "C20": [ - {"op": "replace", "path": "/isBlocked", "value": False}, - {"op": "add", "path": "/blockReason", "value": ""}, ], } + assert not any("/isBlocked" in json.dumps(ops) or "/blockReason" in json.dumps(ops) + for ops in ops_by_card.values()) def test_retirement_uses_authoritative_closure_when_other_reads_fail( @@ -239,7 +239,9 @@ def test_active_issue_cannot_claim_url_owned_retired_card_by_custom_id( ] -def test_retirement_clears_reason_from_already_unblocked_card(tmp_path, monkeypatch): +def test_retirement_leaves_blocked_flag_and_reason_to_humans(tmp_path, monkeypatch): + """Pre-Phase-2 the sync scrubbed stale reason text during retirement; now the flag and + its reason belong to humans, so a card already in its Done lane gets NO patch at all.""" card = { **_card(10, "L5", blocked=False), "blockedStatus": {"isBlocked": False, "reason": "stale reason"}, @@ -253,10 +255,7 @@ def test_retirement_clears_reason_from_already_unblocked_card(tmp_path, monkeypa lanes=[{"id": "L5", "title": "Done", "cardStatus": "finished"}], ) - assert patch_card.call_args.args[3] == [ - {"op": "replace", "path": "/isBlocked", "value": False}, - {"op": "add", "path": "/blockReason", "value": ""}, - ] + patch_card.assert_not_called() def test_active_card_is_not_renamed_to_retired_card_custom_id( diff --git a/tests/test_run.py b/tests/test_run.py index 61b4801..257b0bb 100644 --- a/tests/test_run.py +++ b/tests/test_run.py @@ -17,6 +17,7 @@ sys.path.insert(0, str(Path(__file__).resolve().parent.parent)) +import config # noqa: E402 import sync # noqa: E402 @@ -96,7 +97,7 @@ class FixtureWorld: def __init__(self): self.http_writes: list[HttpWrite] = [] self.process_writes: list[tuple[str, ...]] = [] - self.created_card: dict | None = None + self.created_card_body: dict | None = None self.epic_card = { "id": "C1", "version": "v1", @@ -165,13 +166,20 @@ def open_url(self, request, **_kwargs): "pageMeta": {"offset": 0, "limit": 200, "totalRecords": 0}, }) if method == "POST" and path == "card": - self.created_card = {"id": "C2", **body} - return _Response(self.created_card) + # Live create responses are SPARSE: the new card id only -- no version and no + # customId/laneId echo (validated live 2026-07-21, issue #55). The server still + # persists the full body, which the single-card GET below serves back. + self.created_card_body = body + return _Response({"id": "C2"}) if method == "GET" and path == "card/C2": - assert self.created_card is not None + assert self.created_card_body is not None return _Response({ "card": { - **self.created_card, + "id": "C2", + "title": self.created_card_body["title"], + "customId": self.created_card_body["customId"], + "laneId": self.created_card_body.get("laneId"), + "externalLink": self.created_card_body.get("externalLink"), "version": "v2", "tags": [], "plannedStart": None, @@ -181,6 +189,10 @@ def open_url(self, request, **_kwargs): }) if method in {"POST", "DELETE"} and path == "card/connections": return _Response({}) + if method == "GET" and path.startswith("card/") and path.endswith("/dependency"): + return _Response({"dependencies": []}) + if method in {"POST", "DELETE"} and path == "card/dependency": + return _Response({}) if method == "PATCH" and path == "card/C2": return _Response({"id": "C2", "version": "v3"}) raise AssertionError(f"unexpected AgilePlace request: {method} {path}") @@ -192,6 +204,10 @@ def _completed(argv, payload): def _configure(monkeypatch, tmp_path): + # env_config() re-reads .env on every call and repopulates deleted variables, so a configured + # checkout would leak its real GH_PROJECT_* into this offline world (seen live 2026-07-21 as + # 'unexpected gh command: project item-list '). Point the loader at nothing. + monkeypatch.setattr(config, "ENV_FILE", tmp_path / "no-such.env") values = { "TARGET_REPO_PATH": str(tmp_path), "AGILEPLACE_TOKEN": "test-token", @@ -248,6 +264,12 @@ def _normalize_http(method: str, path: str, body: object): ) if method == "PATCH" and path.startswith("card/"): return "patch", _card_role(path.removeprefix("card/")), json.dumps(body, sort_keys=True) + if method in {"POST", "DELETE"} and path == "card/dependency": + return ( + "depend" if method == "POST" else "undepend", + tuple(_card_role(card_id) for card_id in body["cardIds"]), + tuple(_card_role(card_id) for card_id in body["dependsOnCardIds"]), + ) raise AssertionError(f"unexpected mutation in action set: {method} {path}") @@ -285,7 +307,7 @@ def test_new_card_dry_run_plans_every_action_apply_executes(paired_runs): assert not dry.state_file.exists() assert Counter(_planned_actions(dry.output)) == Counter(_executed_actions(apply)) assert [action[0] for action in _planned_actions(dry.output)] == [ - "create", "gh", "connect", "patch", + "create", "gh", "connect", "depend", "patch", ] assert PLAN_ID_PREFIX not in json.dumps([ {"path": write.path, "body": write.body} for write in apply.http_writes @@ -293,6 +315,22 @@ def test_new_card_dry_run_plans_every_action_apply_executes(paired_runs): assert PLAN_ID_PREFIX not in apply.state_file.read_text(encoding="utf-8") +def test_created_card_snapshot_is_refetched_not_the_sparse_create_echo(paired_runs): + """Issue #55: the sparse create response indexed as the snapshot queued redundant /customId + and /laneId ops, which then tripped the issue-#8 stale-ops guard and aborted every fresh + create+sync apply before metadata landed. The apply must refetch the created card, queue no + redundant identity ops, and drive the batched PATCH to completion.""" + _dry, apply = paired_runs + task_patch_ops = [op + for write in apply.http_writes + if write.method == "PATCH" and write.path == "card/C2" + for op in write.body] + + assert task_patch_ops # metadata landed -- the run did not abort + assert {op["path"] for op in task_patch_ops}.isdisjoint({"/customId", "/laneId"}) + assert apply.state_file.exists() + + def test_whole_run_batches_one_versioned_patch_per_card(paired_runs): _dry, apply = paired_runs patches = [write for write in apply.http_writes if write.method == "PATCH"] diff --git a/tests/test_smoke.py b/tests/test_smoke.py index 646a221..2b9d856 100644 --- a/tests/test_smoke.py +++ b/tests/test_smoke.py @@ -43,13 +43,13 @@ class FakeTenant: """Minimal stateful AgilePlace io v2 double for the whole smoke sequence.""" def __init__(self, *, accept_stale: bool = False, fail_child_create_body: str | None = None, - create_returns_version: bool = True, ignore_external_link: bool = False, - ignore_tag_add: bool = False): + ignore_external_link: bool = False, ignore_tag_add: bool = False, + duplicate_status: int = 409): self.accept_stale = accept_stale self.fail_child_create_body = fail_child_create_body - self.create_returns_version = create_returns_version self.ignore_external_link = ignore_external_link self.ignore_tag_add = ignore_tag_add + self.duplicate_status = duplicate_status # live contract is 409; override to model outages self.created_custom_ids: list[str] = [] self.writes: list[tuple[str, str]] = [] self.cards: dict[str, dict] = { @@ -57,6 +57,7 @@ def __init__(self, *, accept_stale: bool = False, fail_child_create_body: str | "laneId": "L1", "tags": [], "version": 3}, } self.children: dict[str, list[str]] = {} + self.dependencies: dict[str, list[dict]] = {} self._next_id = 0 def urlopen(self, req, timeout=None): @@ -74,6 +75,8 @@ def urlopen(self, req, timeout=None): return self._patch(req, path.removeprefix("card/"), body) if path == "card/connections": return self._connections(method, body) + if path == "card/dependency": + return self._dependency(req, method, body) if method == "DELETE" and path.startswith("card/"): del self.cards[path.removeprefix("card/")] return _Response(None) @@ -95,6 +98,8 @@ def _get(self, url: str, path: str): return _Response({"cards": [{"id": cid} for cid in child_ids], "pageMeta": {"offset": 0, "limit": 200, "totalRecords": len(child_ids)}}) + if path.endswith("/dependency"): + return _Response({"dependencies": self.dependencies.get(path.split("/")[1], [])}) card_id = path.removeprefix("card/") if card_id not in self.cards: raise _http_error(url, 404, json.dumps({"message": "card not found"})) @@ -114,10 +119,10 @@ def _create(self, url: str, body: dict): if "externalLink" in body: card["externalLink"] = body["externalLink"] self.cards[card_id] = card - response = {"id": card_id, **body} - if self.create_returns_version: - response["version"] = "1" - return _Response(response) + # Live create responses are SPARSE: the new card id only -- no version and no + # customId/laneId echo (validated live 2026-07-21, issue #55). The tenant still + # persists the full card, which the single-card GET serves back. + return _Response({"id": card_id}) def _patch(self, req, card_id: str, ops: list): card = self.cards[card_id] @@ -126,6 +131,17 @@ def _patch(self, req, card_id: str, ops: list): if not self.accept_stale and sent != str(card["version"]): raise _http_error(req.full_url, 409, json.dumps( {"message": f"version conflict: sent {sent}, current {card['version']}"})) + # Mirror the live server's atomic validation (observed 2026-07-21, issue #52): a replace on + # a planned-date path must carry a string value; the whole batch is rejected before any op + # is applied. + invalid = [{**op, "error": "Invalid value: must be string"} + for op in ops + if op["path"] in ("/plannedStart", "/plannedFinish") + and op["op"] == "replace" and not isinstance(op.get("value"), str)] + if invalid: + raise _http_error(req.full_url, 422, json.dumps( + {"statusCode": 422, "error": "Unprocessable Entity", + "message": "Invalid patch operations", "data": {"operations": invalid}})) for op in ops: if op["path"] == "/tags/-": if not self.ignore_tag_add: @@ -140,10 +156,39 @@ def _patch(self, req, card_id: str, ops: list): elif op["path"] == "/blockReason": card["blockedStatus"]["reason"] = op["value"] elif op["path"] in ("/plannedStart", "/plannedFinish"): - card[op["path"].removeprefix("/")] = op["value"] + field = op["path"].removeprefix("/") + card[field] = None if op["op"] == "remove" else op["value"] card["version"] += 1 return _Response({"id": card_id, "version": str(card["version"])}) + def _dependency(self, req, method: str, body: dict): + # Mirrors the live contract (confirmed 2026-07-21): duplicate create is HTTP 409 + # "Dependency already exists", deletion is pair-addressed and ignores timing. + for dependent in body["cardIds"]: + for blocker in body["dependsOnCardIds"]: + if method == "POST": + existing = self.dependencies.get(dependent, []) + if any(e["direction"] == "incoming" and e["cardId"] == blocker + for e in existing): + if self.duplicate_status != 409: + raise _http_error(req.full_url, self.duplicate_status, + json.dumps({"message": "boom"})) + raise _http_error(req.full_url, 409, json.dumps( + {"statusCode": 409, "error": "Conflict", + "message": "Dependency already exists", + "data": {"dependsOnCardId": blocker, "cardId": dependent}})) + pairs = (("incoming", dependent, blocker), ("outgoing", blocker, dependent)) + for direction, holder, other in pairs: + entries = self.dependencies.setdefault(holder, []) + if method == "POST": + entries.append({"direction": direction, "cardId": other, + "timing": body.get("timing")}) + else: + self.dependencies[holder] = [ + e for e in entries + if not (e["direction"] == direction and e["cardId"] == other)] + return _Response({}) + def _connections(self, method: str, body: dict): parent = body["cardIds"][0] kids = body["connections"]["children"] @@ -201,6 +246,9 @@ def test_confirmed_run_executes_whole_sequence_and_cleans_up(tenant_env, capsys) ("PATCH", "card/S2"), # externalLink add on bare card ("POST", "card/connections"), # connect child ("DELETE", "card/connections"), # disconnect child + ("POST", "card/dependency"), # dependency create (child dependsOn parent) + ("POST", "card/dependency"), # duplicate-create fact-finding probe + ("DELETE", "card/dependency"), # dependency delete ("PATCH", "card/S1"), # deliberate stale-version probe ("DELETE", "card/S2"), # cleanup child ("DELETE", "card/S1"), # cleanup parent @@ -213,6 +261,20 @@ def test_confirmed_run_executes_whole_sequence_and_cleans_up(tenant_env, capsys) assert "404" in out # post-delete GET confirms the cards are gone assert "blocked" in out # blocked-state round-trip reported 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 + + +def test_unexpected_duplicate_create_failure_fails_the_run(tenant_env, capsys): + """PR #61 review finding: only the confirmed HTTP 409 passes the duplicate probe. An auth/5xx/ + transport failure during the duplicate POST must FAIL the smoke run, never hide as an + informational line that leaves the summary green.""" + tenant = FakeTenant(duplicate_status=503) + tenant_env(tenant) + assert smoke.main([]) == 1 + out = capsys.readouterr().out + assert "FAIL duplicate dependency create rejected (HTTP 409)" in out + assert "unexpected failure (not the confirmed 409)" in out def test_custom_ids_are_unique_per_run(tenant_env, monkeypatch): @@ -260,8 +322,9 @@ def test_tag_add_never_visible_still_summarizes_and_cleans_up(tenant_env, capsys def test_versionless_create_response_is_informational_not_a_failure(tenant_env, capsys): """Live tenants return no version on create (confirmed 2026-07-20); the sync's - refetch-before-PATCH path handles that, so smoke must report the fact without failing.""" - tenant_env(FakeTenant(create_returns_version=False)) + refetch-before-PATCH path handles that, so smoke must report the fact without failing. + The default double is sparse for the same reason -- this pins what it reports.""" + tenant_env(FakeTenant()) assert smoke.main([]) == 0 diff --git a/tests/test_sync.py b/tests/test_sync.py index b88f433..920ffc6 100644 --- a/tests/test_sync.py +++ b/tests/test_sync.py @@ -14,7 +14,7 @@ from agileplace import resolve_lane_for_stage # noqa: E402 from ghproject import parse_items # noqa: E402 from reconcile import reconcile, reconcile_value # noqa: E402 -from stages import (blocked_reason, epic_key_for_task, issue_stage, # noqa: E402 +from stages import (epic_key_for_task, issue_stage, # noqa: E402 lane_matches_stage, normalize_status, title_key) from sync import (MS_PREFIX, _card_milestones, _child_connection_changes, # noqa: E402 _epic_task_resolution, _protect_open_pr_stage, _reconciled_custom_id_index, @@ -672,9 +672,3 @@ def test_protect_open_pr_stage_prints_no_warn_on_misconfigured_stage_lane_map(ca assert "WARN" not in out -def test_blocked_reason(): - stages = {10: "Done", 11: "In progress", 12: "Backlog"} - assert blocked_reason([], stages) is None - assert blocked_reason([10], stages) is None # blocker Done -> unblocked - assert blocked_reason([10, 11], stages) == "Blocked by #11" - assert blocked_reason([12, 11], stages) == "Blocked by #11, #12" # incomplete, sorted diff --git a/tests/test_sync_dependencies.py b/tests/test_sync_dependencies.py new file mode 100644 index 0000000..fe559ed --- /dev/null +++ b/tests/test_sync_dependencies.py @@ -0,0 +1,148 @@ +"""Unit tests for sync.sync_dependencies (issue #57, Phase 1). + +The contract: GitHub blocked-by edges are mirrored as native AgilePlace dependencies, +EVERY edge (a Done blocker's edge is structural -- unlike the Blocked flag, which only +reflects incomplete blockers). GitHub is authoritative ONLY between two sync-managed +cards; dependencies touching non-managed cards are invisible in both directions. A +failed/malformed read (None) skips that card entirely -- duplicate-create behavior is +unconfirmed live, so nothing may be blindly re-created against unknown state. + +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 sync import _dependency_changes, sync_dependencies # noqa: E402 + + +# --- _dependency_changes (pure) ------------------------------------------- + +def test_changes_adds_missing_and_removes_stale_managed(): + adds, removes = _dependency_changes({"B1", "B2"}, {"B2", "B3"}, {"B1", "B2", "B3"}) + assert adds == ["B1"] + assert removes == ["B3"] + + +def test_changes_never_removes_dependencies_on_non_managed_cards(): + adds, removes = _dependency_changes(set(), {"HUMAN1"}, {"B1"}) + assert adds == [] + assert removes == [] + + +def test_changes_in_sync_is_a_no_op(): + assert _dependency_changes({"B1"}, {"B1"}, {"B1"}) == ([], []) + + +# --- sync_dependencies (step behavior) ------------------------------------ + +def _issue(number, key): + return {"number": number, "title": f"[{key}] t", "url": f"https://github.com/o/r/issues/{number}"} + + +def _harness(cards): + """card_for keyed by issue number; cards: {number: card_dict_or_None}.""" + return lambda issue: cards.get(issue["number"]) + + +def _run(issues, blocked_by, cards, managed, reads, blocker_cards=None): + """Run sync_dependencies with agileplace fully faked; return recorded write calls. + blocker_cards defaults to every issue's card (the _blocker_cards contract); pass it + explicitly to model retired blockers that resolve outside the active-issue set.""" + calls = {"create": [], "delete": [], "reads": []} + + def fake_read(cfg, cid): + calls["reads"].append(cid) + return reads.get(cid) + + if blocker_cards is None: + blocker_cards = {i["number"]: cards[i["number"]] + for i in issues if cards.get(i["number"])} + with patch("sync.agileplace.card_dependencies", side_effect=fake_read), \ + patch("sync.agileplace.incoming_dependency_ids", + side_effect=lambda entries: {e["cardId"] for e in entries + if e.get("direction") == "incoming"}), \ + patch("sync.agileplace.create_dependencies", + side_effect=lambda cfg, apply, cid, ids: calls["create"].append((cid, sorted(ids)))), \ + patch("sync.agileplace.delete_dependencies", + side_effect=lambda cfg, apply, cid, ids: calls["delete"].append((cid, sorted(ids)))): + sync_dependencies({}, True, issues, blocked_by, blocker_cards, _harness(cards), managed) + return calls + + +def test_creates_missing_dependency_between_managed_cards(): + issues = [_issue(1, "A"), _issue(2, "B")] + cards = {1: {"id": "C1"}, 2: {"id": "C2"}} + calls = _run(issues, {1: [2]}, cards, {"C1", "C2"}, reads={"C1": [], "C2": []}) + assert calls["create"] == [("C1", ["C2"])] + assert calls["delete"] == [] + + +def test_removes_stale_managed_dependency_but_never_a_non_managed_one(): + issues = [_issue(1, "A")] + cards = {1: {"id": "C1"}} + reads = {"C1": [{"direction": "incoming", "cardId": "C9"}, # managed, no GH edge -> remove + {"direction": "incoming", "cardId": "HUMAN"}]} # not managed -> untouchable + calls = _run(issues, {1: []}, cards, {"C1", "C9"}, reads) + assert calls["create"] == [] + assert calls["delete"] == [("C1", ["C9"])] + + +def test_failed_read_skips_that_card_but_not_others(capsys): + issues = [_issue(1, "A"), _issue(2, "B"), _issue(3, "C")] + cards = {1: {"id": "C1"}, 2: {"id": "C2"}, 3: {"id": "C3"}} + reads = {"C1": None, "C2": [], "C3": []} # C1's read fails + calls = _run(issues, {1: [2], 2: [3]}, cards, {"C1", "C2", "C3"}, reads) + assert calls["create"] == [("C2", ["C3"])] # C1's desired add was NOT attempted + assert "WARN" in capsys.readouterr().out + + +def test_plan_only_card_creates_all_desired_without_a_read(): + issues = [_issue(1, "A"), _issue(2, "B")] + cards = {1: {"id": "planned-card:x", "_planOnly": True}, 2: {"id": "C2"}} + calls = _run(issues, {1: [2]}, cards, {"planned-card:x", "C2"}, reads={"C2": []}) + assert calls["create"] == [("planned-card:x", ["C2"])] + assert "planned-card:x" not in calls["reads"] # a plan-only id never crosses a read boundary + + +def test_blocker_without_a_card_is_excluded_from_desired(): + issues = [_issue(1, "A"), _issue(2, "B")] + cards = {1: {"id": "C1"}, 2: None} # blocker issue 2 has no card + calls = _run(issues, {1: [2]}, cards, {"C1"}, reads={"C1": []}) + assert calls["create"] == [] + assert calls["delete"] == [] + + +def test_edge_to_retired_done_blocker_is_preserved_not_deleted(): + """P1 from the gpt-5.6-sol adversarial review: an active issue blocked by a RETIRED + (NOT_PLANNED/DUPLICATE) issue whose URL-owned card survives must keep its native + dependency. Blockers resolving through active issues only dropped the edge from + `desired` while the retired card stayed managed -- deleting the valid dependency as + stale on every run.""" + issues = [_issue(1, "A")] # only the active issue is syncable + cards = {1: {"id": "C1"}} + blocker_cards = {1: {"id": "C1"}, 9: {"id": "R9"}} # 9 = retired blocker, card R9 + reads = {"C1": [{"direction": "incoming", "cardId": "R9"}]} # edge already on the board + calls = _run(issues, {1: [9]}, cards, {"C1", "R9"}, reads, blocker_cards=blocker_cards) + assert calls["delete"] == [] # the edge is desired -- NOT stale + assert calls["create"] == [] # and already present -- nothing to add + + +def test_sync_blocker_cards_helper_includes_retired_url_owned_cards(): + from sync import _blocker_cards + by_number = {1: {"number": 1, "url": "u1"}} + retired = [{"number": 9, "url": "u9"}, {"number": 8, "url": "u8-no-card"}] + resolved = _blocker_cards(by_number, lambda i: {"id": f"C{i['number']}"}, + retired, {"u9": {"id": "R9"}}) + assert resolved == {1: {"id": "C1"}, 9: {"id": "R9"}} # 8 has no card -> excluded + + +def test_every_edge_is_mirrored_not_only_incomplete_blockers(): + """blocked_by carries raw edges; sync_dependencies must not filter by stage/completion -- + an edge whose blocker is Done is still desired (the Blocked flag differs deliberately).""" + issues = [_issue(1, "A"), _issue(2, "DONE-BLOCKER")] + cards = {1: {"id": "C1"}, 2: {"id": "C2"}} + calls = _run(issues, {1: [2]}, cards, {"C1", "C2"}, reads={"C1": [], "C2": []}) + assert calls["create"] == [("C1", ["C2"])] diff --git a/tests/test_sync_main.py b/tests/test_sync_main.py index 9167982..8866a80 100644 --- a/tests/test_sync_main.py +++ b/tests/test_sync_main.py @@ -90,6 +90,7 @@ def _mock_io(card, items_and_raw_return, field_meta_return, open_pr_return=_UNSE stack.enter_context(patch("agileplace.board_layout", return_value=list(lanes_return))) cards = [card] if existing_cards is _UNSET else list(existing_cards) stack.enter_context(patch("agileplace.list_cards", return_value=cards)) + stack.enter_context(patch("agileplace.card_dependencies", return_value=[])) patch_card_mock = stack.enter_context(patch("agileplace.patch_card")) create_card_mock = stack.enter_context(patch("agileplace.create_card", return_value={})) return stack, run_mock, patch_card_mock, create_card_mock