diff --git a/.harness/docs/ARCHITECTURE.md b/.harness/docs/ARCHITECTURE.md index b2085db..6649dbc 100644 --- a/.harness/docs/ARCHITECTURE.md +++ b/.harness/docs/ARCHITECTURE.md @@ -1867,11 +1867,11 @@ story-105 is where the two halves of the outbox pair finally reach this deployme **The repair had to come first, because the board block was unreachable on the one invocation that needed it.** The script searched for an existing issue and, where it found one, echoed the URL and exited above the board block — so the sequence that block's own comment described, a board call that failed being retried by the next sweep, could not happen. An entry whose issue was created and whose board call failed would be found by the next invocation, answered from the search, and reported as landed with nothing on the board. Adding a Status call in the same place would have inherited the same hole. The two paths now **converge on one `url`**: the found path assigns and falls through, the create path assigns from what it created, and there is one `echo` and one `exit 0` at the end of the file, below the board block. The already-set case is a condition around the write rather than an early return, so that single exit stays single. -**Everything below `gh issue create` succeeding is transient, and that is a statement about what the two artifacts are.** The issue is the record; the board is a view of it. A board that was briefly unreachable must not cost the item, so every failure after the creation exits 75, the entry stays pending, and a later sweep re-runs the whole command — which finds the issue rather than creating a second one, on the idempotency contract "One command files a story" above states. The failures this covers are now several: the item-add, the item listing, the field and option lookups, and the edit itself. **A name that resolves to no id is transient like the rest** — a misconfigured field or option name costs a pending entry, and the alternative costs the item. +**Everything below `gh issue create` succeeding is transient, and that is a statement about what the two artifacts are.** The issue is the record; the board is a view of it. A board that was briefly unreachable must not cost the item, so every failure after the creation exits 75, the entry stays pending, and a later sweep re-runs the whole command — which finds the issue rather than creating a second one, on the idempotency contract "One command files a story" above states. The failures this covers are now several: the item-add, the read of the item's own field values, the field and option lookups, and the edit itself. **A name that resolves to no id is transient like the rest** — a misconfigured field or option name costs a pending entry, and the alternative costs the item. -**The board block adds the item keeping its id, reads what the board already says, and writes only into an empty Status.** (story-110 restructured that block so its reads happen once per filing and five classification fields are written beside Status; the Status write's own behaviour, including its transient exit on a name that resolves to no id, is unchanged. See "A filed brief carries its classification" below.) `gh project item-add --format json` reports the existing item for an issue already on the board rather than adding a second, which is what makes the retry safe, and its id is what `item-edit` takes — that command deals in ids and not in names. Where a Status option is configured, the item's current value is read out of the project's item listing, and the write happens **only where that value is empty**. A sweep re-running over an entry that landed long ago finds a value and leaves it: this script puts an item into a column once and never moves it between columns, and transitions at other invocation points are a separate question. Where the write is needed, the project's node id, the field's id and the option's id are resolved by the configured names through `project view` and `field-list`. +**The board block adds the item keeping its id, reads what the board already says, and writes only into an empty Status.** (story-110 restructured that block so its reads happen once per filing and five classification fields are written beside Status; the Status write's own behaviour, including its transient exit on a name that resolves to no id, is unchanged. See "A filed brief carries its classification" below.) `gh project item-add --format json` reports the existing item for an issue already on the board rather than adding a second, which is what makes the retry safe, and its id is what `item-edit` takes — that command deals in ids and not in names. Where a Status option is configured, the item's current value is read — out of a listing of the whole project until story-128 replaced that with a read of the item's own node id, see "One item's Status is read by its own id" below — and the write happens **only where that value is empty**. A sweep re-running over an entry that landed long ago finds a value and leaves it: this script puts an item into a column once and never moves it between columns, and transitions at other invocation points are a separate question. Where the write is needed, the project's node id, the field's id and the option's id are resolved by the configured names through `project view` and `field-list`. -**An item the listing did not report is a failure to know, never an empty Status.** Those two are the same absence read two ways, and reading it the wrong way moves an item out of the column a human put it in — a silent, unattributable regression in somebody else's board. So a listing that does not contain the item exits 75 and says so, naming the bound it read to. `ITEM_LIST_LIMIT` is what bounds that read, and it is deliberately **not** an `L5_SYNC_` constant: it is a mechanic rather than something this target files against, identical in both copies, and a bound that was too small costs a pending entry rather than an overwritten column. +**An answer that did not describe the item is a failure to know, never an empty Status.** Those two are the same absence read two ways, and reading it the wrong way moves an item out of the column a human put it in — a silent, unattributable regression in somebody else's board. So a read that does not report the item exits 75 and says so. story-105 obtained that answer by selecting the item out of a listing of the whole project, bounded by an `ITEM_LIST_LIMIT` that was deliberately **not** an `L5_SYNC_` constant; story-128 retired both the listing and the bound, and the guard itself is unchanged — see "One item's Status is read by its own id" below. **The split is the design: the mechanics are generic and the values are the target's.** `templates/sync/github.sh` gained `PROJECT_OWNER`, `STATUS_FIELD` and `STATUS_OPTION` in the `${L5_SYNC_...:-default}` form its two existing constants already used, and it names **no project number and no Status option value** — a template carrying one would file another repository's briefs onto this board. An empty `STATUS_OPTION` means the item is added and its Status left at the project's own default, which is what a target whose board has no such field gets, and a target that configures no project at all files exactly as it did before. `.harness/sync/github.sh` sets the two values this deployment files against and differs from its template in nothing else, which is asserted by comparing the pair and requiring every differing line to be a constant assignment — with a control that plants a non-constant difference and requires it reported. @@ -1921,11 +1921,11 @@ story-110 puts a brief's classification where the backlog is worked. A brief has **The script enumerates no category, severity, confidence, effort or workflow.** It writes the value the entry carries, so the acceptable values stay `schemas/story-brief.schema.json`'s enums and the workflows the harness defines — and a category added to the schema later is a board option somebody adds rather than a second list in a shell script to keep in step. The tests are derived the same way, from the schema's enums and from `harness_config.workflow_names`, so a category added to the schema becomes a drive case with no edit to the test module; a helper asserts each derived axis is non-empty, so an enum that stopped resolving is a failing test rather than a parametrization over nothing. -**`CATEGORY_LABEL_PREFIX` defaults to something non-empty and the five field names default to empty**, which is the same split story-105 made and read off which kind of thing each is. A label prefix is a mechanic every target filing briefs wants; a field name is a claim about a particular board, and a template carrying one would write into a column of somebody else's project. An empty field name means that field is not written, so a target that configures nothing files exactly as it did before this story — asserted directly, by driving the template with nothing set and observing no field call and, with no project configured, no project call at all. `CATEGORY_LABEL_COLOR` is deliberately not an `L5_SYNC_` constant, beside `ITEM_LIST_LIMIT` and for its reason: it is a mechanic, and it exists so the `--force` create is genuinely a no-op — a create naming no colour gives the label a fresh random one on every filing. +**`CATEGORY_LABEL_PREFIX` defaults to something non-empty and the five field names default to empty**, which is the same split story-105 made and read off which kind of thing each is. A label prefix is a mechanic every target filing briefs wants; a field name is a claim about a particular board, and a template carrying one would write into a column of somebody else's project. An empty field name means that field is not written, so a target that configures nothing files exactly as it did before this story — asserted directly, by driving the template with nothing set and observing no field call and, with no project configured, no project call at all. `CATEGORY_LABEL_COLOR` is deliberately not an `L5_SYNC_` constant, for the reason `ITEM_LIST_LIMIT` stood beside it until story-128 retired it: it is a mechanic, and it exists so the `--force` create is genuinely a no-op — a create naming no colour gives the label a fresh random one on every filing. -**The reads are memoised rather than hoisted, and the difference is what a target with no fields configured pays.** Six writes resolving their own project id, field list and item listing would multiply the reads by six. Hoisting all three above the writes would make a target with a project and no fields configured issue `project view` and `field-list` calls it does not make today, which is exactly what the "files exactly as it does today" property forbids. So `read_the_project` and `read_the_item` memoise on the globals they fill and are called the first time something needs them: each is read at most once per filing and not at all when nothing is written. `board_value`, `field_id_for` and `option_id_for` are the three lookups all six writes share. +**The reads are memoised rather than hoisted, and the difference is what a target with no fields configured pays.** Six writes resolving their own project id, field list and item field values would multiply the reads by six. Hoisting all three above the writes would make a target with a project and no fields configured issue `project view` and `field-list` calls it does not make today, which is exactly what the "files exactly as it does today" property forbids. So `read_the_project` and `read_the_item` memoise on the globals they fill and are called the first time something needs them: each is read at most once per filing and not at all when nothing is written. `board_value`, `field_id_for` and `option_id_for` are the three lookups all six writes share. -**Nothing overwrites a value a person edited.** Each field is written only where the item listing reports it empty, which is the rule the Status write already kept and for the reason story-105 records: an item the listing did not report is a failure to know rather than a set of empty fields, so it exits 75 rather than writing over what a human put there. A retry that finds the existing issue falls through to both the label work and the field work and creates no second issue, which is the convergence-on-one-`url` repair story-105 made, now carrying two more blocks below it. +**Nothing overwrites a value a person edited.** Each field is written only where the read of the item reports it empty, which is the rule the Status write already kept and for the reason story-105 records: an answer that did not describe the item is a failure to know rather than a set of empty fields, so it exits 75 rather than writing over what a human put there. A retry that finds the existing issue falls through to both the label work and the field work and creates no second issue, which is the convergence-on-one-`url` repair story-105 made, now carrying two more blocks below it. **Every failure after the issue exists is still transient**, so the label create, the label add, and each project call the field work makes exit 75 with the entry left pending and the next sweep re-running the whole command. The one thing that is not a failure is a name that resolves to no id, and that is the skip above rather than an exception to this. @@ -2035,6 +2035,18 @@ story-126 closes the last gap in the loop the brief sections above opened. The I **What this story deliberately does not do**, and the list is the design rather than an omission: it changes no sync command, no filed-query command, no outbox entry and no transport; a publish enters no queue and is not a filing; nothing marks a story planned in the repository from what a tracker says; and no status is sent, read or inferred. +## One item's Status is read by its own id + +story-128 changes how the board block learns what a board already says about the item it just added. It asked for the whole project and selected one object out of the listing; it now asks for that one item, by the node id `item-add` already handed it, through a single `gh api graphql` read of the node's `fieldValues`. Both sync copies carry the change and nothing else moved: the writes, the transient discipline, the write-only-into-an-empty-field rule and every marker are as they were. + +**A listing has two ways of not containing the item, and both were paid.** The listing was bounded, and a newly added item sits at the *end* of a board's order, so past the bound the one item this invocation just added is exactly the one the listing cannot report — and the entry it left pending is retried by every later sweep against the same bound, so it can never succeed. Nothing in the queue tells that apart from a board that was briefly unreachable, which is the failure mode the transient exit was designed for and this one is not. The second way is cheaper to hit and was hit first: this repository's own filing on 2026-09-07 added an issue to a 49-item board and got a listing that did not yet contain it, and a re-run moments later set every field on the first attempt. **An index that lags behind a write is not a size problem**, so raising the bound would have answered neither. Asking for the item by id answers both at once: it is consistent by id rather than read against an index, and it has no size to outgrow. + +**The guard the read exists to serve is unchanged, and that is the load-bearing part.** An answer that was not obtained stays distinct from a field the board reports empty, and the distinction is carried by the `jq` reduction rather than by a second test: an answer with no such node, and an answer carrying `errors` instead of data, both reduce to *nothing at all*, while an item that exists carrying no values reduces to `{}`. So the one emptiness test that used to catch an item missing from the listing now catches an unobtainable read, exits 75 and makes no `item-edit` call, while an item with no values is accepted and every field reads as empty — the only case written into. **What changed about the guard is its message**: it names the item and the project and no count, because there is no longer a bound to name and telling a developer about one was telling them the wrong cause. `ITEM_LIST_LIMIT` and the comment block describing its cost as "a pending entry and never an overwritten value" are deleted, along with the sentence that explained `CATEGORY_LABEL_COLOR` by reference to it. + +**Only single-select values are selected**, because those are the only ones this script writes; a field of any other type contributes no key and therefore reads as empty, which is the answer the listing gave for a field it reported no value for. `board_value` now removes spaces and lowers case on **both** sides of the comparison rather than only on the requested name, because the listing keyed a field by its name with spaces removed and the graphql answer keys it by the name the board declares — the tolerance is what makes a board whose field names carry spaces resolve as it did before. The read is memoised on the same global as before, so it is made at most once per filing however many of the six fields are written, and not at all when nothing is written. + +**The stub `gh` in `tests/test_filed_query.py` grew the read and lost the listing**, and the assertions moved with it: the ledger records the graphql call as it records the `project` subcommands, `OMIT_VARIABLE` now makes the node read report no such item rather than making the listing report nothing, and the read failing is driven beside it so both routes to *unobtainable* are exercised. The assertions new in kind rather than repointed are that a filing writing every field records no project item-list call at all, that the transient stderr carries no number, and that a board seeded past the retired bound files an item added at the end of its order. A scan holds both sync scripts to mentioning neither `gh project item-list` nor `ITEM_LIST_LIMIT`, with a control that plants a listing back into a script and requires it reported, so the retirement is held by something that reddens rather than by this paragraph. + ## Run directory anatomy .harness/runs/story-001/ diff --git a/.harness/history/execution-log.jsonl b/.harness/history/execution-log.jsonl index ed624dd..19b90cc 100644 --- a/.harness/history/execution-log.jsonl +++ b/.harness/history/execution-log.jsonl @@ -54,3 +54,4 @@ {"story_id": "story-118", "status": "completed", "retry_count": 0, "timestamp": "2026-09-08 20:13:01"} {"story_id": "story-123", "status": "completed", "retry_count": 0, "timestamp": "2026-09-08 21:27:35"} {"story_id": "story-126", "status": "completed", "retry_count": 0, "timestamp": "2026-09-09 01:42:55"} +{"story_id": "story-128", "status": "completed", "retry_count": 0, "timestamp": "2026-09-09 08:32:13"} diff --git a/.harness/history/inspection-log.jsonl b/.harness/history/inspection-log.jsonl index bf70993..e781fcd 100644 --- a/.harness/history/inspection-log.jsonl +++ b/.harness/history/inspection-log.jsonl @@ -18,3 +18,4 @@ {"story_id": "story-118", "mode": "narrow", "cost_usd": 2.9625245, "scope_files": 60, "invocations": 1, "timestamp": "2026-09-08 20:20:27", "findings": 3, "filed": 3, "dropped": 0} {"story_id": "story-123", "mode": "narrow", "cost_usd": 1.7901849999999995, "scope_files": 60, "invocations": 1, "timestamp": "2026-09-08 21:32:25", "findings": 3, "filed": 3, "dropped": 0} {"story_id": "story-126", "mode": "narrow", "cost_usd": 1.9372045000000002, "scope_files": 60, "invocations": 1, "timestamp": "2026-09-09 01:47:09", "findings": 3, "filed": 3, "dropped": 0} +{"story_id": "story-128", "mode": "narrow", "cost_usd": 1.633331, "scope_files": 60, "invocations": 1, "timestamp": "2026-09-09 08:37:45", "findings": 2, "filed": 2, "dropped": 0} diff --git a/.harness/history/mandate-log.jsonl b/.harness/history/mandate-log.jsonl index 9ce18d3..0815887 100644 --- a/.harness/history/mandate-log.jsonl +++ b/.harness/history/mandate-log.jsonl @@ -32,3 +32,4 @@ {"story_id": "story-119", "timestamp": "2026-09-08 15:53:25", "conferred_by": "jerod.wilkerson <30474318+jerodw@users.noreply.github.com>", "source_kind": "human", "recorded_by": "l5-plan", "discarded_session_block": false} {"story_id": "story-123", "timestamp": "2026-09-08 20:30:12", "conferred_by": "jerod.wilkerson <30474318+jerodw@users.noreply.github.com>", "source_kind": "human", "recorded_by": "l5-plan", "discarded_session_block": false} {"story_id": "story-126", "timestamp": "2026-09-09 00:38:53", "conferred_by": "jerod.wilkerson <30474318+jerodw@users.noreply.github.com>", "source_kind": "human", "recorded_by": "l5-plan", "discarded_session_block": false} +{"story_id": "story-128", "timestamp": "2026-09-09 08:02:40", "conferred_by": "jerod.wilkerson <30474318+jerodw@users.noreply.github.com>", "source_kind": "human", "recorded_by": "l5-plan", "discarded_session_block": false} diff --git a/.harness/stories/story-128.yaml b/.harness/stories/story-128.yaml new file mode 100644 index 0000000..3cac459 --- /dev/null +++ b/.harness/stories/story-128.yaml @@ -0,0 +1,133 @@ +story: + id: story-128 + title: One item's Status is read by its own id, so filing survives a board of any size + description: | + Planned from the brief filed under key 118. + + To learn what a board already says about the item it just added, the sync + script lists the whole project and selects one object out of the listing. + Two things follow from that, and both have been paid. + + The listing is bounded at 5000 items, and a newly added item sits at the + end of the board's order, so once a board grows past the bound the item + this invocation just added is exactly the one the listing cannot report. + The script then exits transiently, correctly, on the reading that a + listing which did not mention the item is a failure to know rather than an + empty Status -- but the entry it leaves pending is retried on every sweep + and can never succeed, because the next sweep reads the same bound against + the same board. The comment describing that cost as "a pending entry and + never an overwritten value" reads as bounded; the real cost is that the + deployment stops filing anything, permanently, with no state telling it + apart from a board that is briefly unreachable. + + The bound is not the only way this fails, and it was not the first one met. + On 2026-09-07 a brief's issue was created and added to project 1, and the + listing that followed did not contain the item -- on a board of 49 items. + The add had been accepted; the listing had not caught up. The entry + deferred saying the item "was not in the first 5000 items", which named a + bound that had nothing to do with it, and a re-run moments later found the + item and set every field on the first attempt. + + Both are answered by asking for the one item rather than for the board. + Its node id is already in hand, so a graphql read of that node's field + values answers the same question in one request: consistent by id rather + than against an index that lags, and with no size to outgrow. The read is + shared by the Status write and the five classification writes, so all six + become bound-free and race-free together, and the bound and the comment + that misdescribes it are deleted. + + What the fix must not disturb is the reading the guard rests on: an answer + that was not obtained stays distinct from a field the board reports empty, + and the script still writes only into a field it read as empty. What + changes about that guard is its message, which stops naming a bound + because there is no longer one to name. + + The shipped reference implementation is templates/sync/github.sh, and this + repository files its own briefs through the copy installed at + .harness/sync/github.sh, which a later harness change does not update. The + two carry identical mechanics and differ only in the constants a target + sets, so the fix lands in both. + workflow: story-workflow + +tasks: + - Replace the project listing in read_the_item in templates/sync/github.sh with a gh api graphql read of the item's own node id, returning that item's single-select field values keyed by the field names the board declares. + - Delete ITEM_LIST_LIMIT and the comment block that describes its cost, since neither a bound nor its cost survives the change. + - Rewrite the transient message for an item whose fields could not be obtained so that it names the item and the project and no item count. + - Make the same mechanical change in the installed copy .harness/sync/github.sh, leaving its own L5_SYNC_ constant values as they are. + - Adapt tests/test_filed_query.py so its stub gh answers a graphql read of an item node, and so the assertions that were written against the listing are made against the read that replaces it. + - Add assertions that a filing to a board makes no project item-list invocation at all, and that the transient answer for an unobtainable item mentions no item count. + +acceptance_criteria: + - A filing that sets Status and all five classification fields on a board completes with no gh project item-list invocation recorded by the stub tracker. + - The item's field values are obtained through a single gh api graphql read that carries the item's node id, and that read is made at most once per filing however many fields are written. + - An item whose field values cannot be obtained -- the read failing, or answering with no such node -- exits 75, makes no item-edit call, and leaves every field on the board as it was. + - The stderr of that transient answer names the item and the project and contains no number of items, so a developer is not told about a bound as the cause. + - A board holding more items than the retired bound files correctly, including an item added at the end of that board's order. + - A field the board reports no value for is still read as empty and is the only case written into; a field the board already carries a value for is left alone, for Status and for each classification field. + - ITEM_LIST_LIMIT appears in neither templates/sync/github.sh nor .harness/sync/github.sh, and neither file invokes gh project item-list. + - templates/sync/github.sh and .harness/sync/github.sh differ only in the values of the editable constant assignments at the top, which is what the existing comparison in tests/test_filed_query.py already asserts. + - The existing suite passes, including the marker-agreement checks that hold the sync, query and item scripts to the same strings. + +technical_plan: + implementation_steps: + - Read read_the_item and board_value in templates/sync/github.sh, and the calls into them from the Status write and from set_board_field, so the replacement keeps the same contract - a shell variable holding this item's field values, read at most once, empty where the board reports nothing. + - Replace the gh project item-list call with gh api graphql, passing the item's node id as a variable, selecting the item node's fieldValues and taking the single-select values with the name of the field each belongs to. Only single-select values are needed, because those are the only ones this script writes. + - Reduce that answer with jq to one object mapping field name to value, dropping any entry missing either half, so a field the board reports no value for contributes no key and reads as empty exactly as it does today. + - Keep board_value tolerant of both spellings - match the requested field name against the keys with spaces removed and case lowered on both sides - so a board whose field names carry spaces resolves the same way it did under the listing. + - Answer transiently where the read fails and where it resolves to no such node, with a message naming the item and the project and no count, since a bound is no longer among the reasons. + - Delete ITEM_LIST_LIMIT and the comment block above it, and check the file's remaining header prose for any sentence that describes the read as bounded. + - Apply the identical mechanical change to .harness/sync/github.sh, changing none of its constant values. + - In tests/test_filed_query.py, teach the stub gh to answer an api graphql invocation by looking the item up by its node id across the projects it holds and reporting that item's field values in the shape the real graphql answer has, recording the call in the ledger as the project subcommands already are. + - Repoint the variable that made the old listing report nothing so that it makes the node read report no such item, and keep the test built on it - the claim it makes, that a failure to know is not an empty Status, is unchanged and is the guard the story preserves. + - Add the new assertions - no item-list invocation is made during a filing, the transient stderr carries no item count, and a board seeded with more items than the retired bound still files. + likely_file_changes: + - file: templates/sync/github.sh + stage: implementer + reason: The shipped reference implementation carrying the bounded listing, the ITEM_LIST_LIMIT constant and the comment that misdescribes its cost. + - file: .harness/sync/github.sh + stage: implementer + reason: This repository's installed copy of the same mechanics, which a change to the template does not reach and which is the copy that filed the entry that failed. + - file: tests/test_filed_query.py + stage: tester + reason: Holds the stub tracker and every assertion written against the project listing, so it grows graphql support and the assertions that the listing is no longer read. + +scope: + modify: + - templates/sync/github.sh + - .harness/sync/github.sh + - tests/test_filed_query.py + do_not_modify: + - templates/query/github.sh + - templates/item/github.sh + - .harness/query/github.sh + - .harness/item/github.sh + - orchestration/ + - schemas/ + - templates/config.yaml + - .harness/config.yaml + +verification_requirements: + - Confirm that neither sync script invokes gh project item-list and that neither mentions ITEM_LIST_LIMIT. + - Confirm from the stub tracker's ledger that a complete filing records no item-list call and exactly one graphql read of the item. + - Confirm that an item whose field values cannot be obtained still exits 75, still makes no item-edit call, and leaves the board's fields untouched. + - Confirm that the transient message for that case contains no number of items. + - Confirm the write-only-into-an-empty-field rule still holds for Status and for each of the five classification fields, in both directions - written where the board reports empty, left alone where the board reports a value. + - Confirm that a board holding more items than the retired bound files an item added at the end of its order. + - Confirm the two sync copies still differ only in the values of their editable constants. + - Confirm the whole suite passes with the test command the harness configuration names. + +constraints: + - An answer that was not obtained must stay distinct from a field the board reports empty. The transient exit for an unobtainable item is the correct reading and survives the change. + - The rule that a value is written only into a field the board reports empty stays exactly as it is, for Status and for the classification fields alike. + - The Status write keeps its standing as the one write this script may not quietly decline to make, and the classification writes keep theirs of being skipped with a note on stderr. + - The key marker, the per-path markers and the payload marker are untouched, so filing, dedupe and brief fetch keep finding what this script writes. + - The sync command commits nothing to the repository. + - The template carries mechanics and no target's values; the installed copy keeps its own values and gains no mechanics of its own. + - The script keeps requiring only gh and jq, and reaches for no additional tool. + +mandate: + source: + kind: human + conferred_at: 2026-09-09 08:02:40 + conferred_by: jerod.wilkerson <30474318+jerodw@users.noreply.github.com> + recorded_by: l5-plan diff --git a/.harness/sync/github.sh b/.harness/sync/github.sh index 79a6e9b..b5e68a5 100755 --- a/.harness/sync/github.sh +++ b/.harness/sync/github.sh @@ -135,21 +135,13 @@ WORKFLOW_FIELD="${L5_SYNC_WORKFLOW_FIELD:-Workflow}" # wants — and a payload carrying no category applies no label at all. CATEGORY_LABEL_PREFIX="${L5_SYNC_CATEGORY_LABEL_PREFIX:-l5-}" -# The colour a category label is created with. Not an L5_SYNC_ constant, for the -# reason ITEM_LIST_LIMIT below is not: it is a mechanic rather than something -# this target files against. It exists so the create is idempotent — gh's +# The colour a category label is created with. Not an L5_SYNC_ constant: it is a +# mechanic rather than something this target files against. It exists so the +# create is idempotent — gh's # --force updates a label that already exists rather than failing on it, and a # create naming no colour would give the label a fresh random one every filing. CATEGORY_LABEL_COLOR="ededed" -# How much of the project's item listing is read when looking for the item this -# invocation just added. Not an L5_SYNC_ constant, because it is a mechanic -# rather than something this target files against: it bounds a read, and an -# item the listing did not report is answered transiently rather than read as a -# set of empty fields, so a bound that was too small costs a pending entry and -# never an overwritten value. -ITEM_LIST_LIMIT=5000 - # The searchable marker written once per path the payload carries. # templates/query/github.sh searches for exactly this prefix, and a test reads # this line out of both files and asserts the two strings are the same, so the @@ -285,8 +277,8 @@ if [ -n "$PROJECT" ]; then || fail_transient "the issue was added to project ${PROJECT} but the item id could not be read" [ -n "$item_id" ] || fail_transient "the issue was added to project ${PROJECT} but it named no item" - # The project's id, its field list and its item listing are each read at most - # once per filing rather than once per field, so the writes below do not + # The project's id, its field list and this item's own field values are each + # read at most once per filing rather than once per field, so the writes below do not # multiply the reads. Each is read the first time something needs it, so a # filing with nothing to write makes none of these calls. project_id="" @@ -306,27 +298,56 @@ if [ -n "$PROJECT" ]; then read_the_item() { [ -z "$item" ] || return 0 - # What the board already says about this item. An item the listing did not - # report is a failure to know rather than a set of empty fields: writing on - # the strength of a listing that did not mention the item would overwrite - # values a human put there. - local listed - listed="$(gh project item-list "$PROJECT" --owner "$PROJECT_OWNER" \ - --limit "$ITEM_LIST_LIMIT" --format json 2>/dev/null)" \ - || fail_transient "the project ${PROJECT} listing failed, so the item's fields are unknown" - item="$(printf '%s' "$listed" \ - | jq -c --arg id "$item_id" '[.items[]? | select(.id == $id)] | .[0] // empty')" \ - || fail_transient "the project ${PROJECT} listing could not be read" + # What the board already says about this item, asked for by the item's own + # node id rather than selected out of a listing of the whole project. The id + # is already in hand, so one graphql read answers the same question + # consistently: it is not read against an index that lags behind an add, and + # it has no size to outgrow as the board grows. + # + # An item whose field values could not be obtained -- the read failing, or + # answering with no such node -- is a failure to know rather than a set of + # empty fields: writing on the strength of an answer that did not describe + # the item would overwrite values a human put there. + local answered + answered="$(gh api graphql -f item="$item_id" -f query=' + query($item: ID!) { + node(id: $item) { + ... on ProjectV2Item { + fieldValues(first: 100) { + nodes { + ... on ProjectV2ItemFieldSingleSelectValue { + name + field { ... on ProjectV2FieldCommon { name } } + } + } + } + } + } + }' 2>/dev/null)" \ + || fail_transient "the field values of item ${item_id} in project ${PROJECT} could not be read, so its fields are unknown" + # One object mapping field name to value. Only single-select values are + # selected, because those are the only ones this script writes, and an entry + # missing either half is dropped -- so a field the board reports no value for + # contributes no key and reads as empty, exactly as it did under the listing. + # An answer carrying no such node yields nothing at all rather than an empty + # object, which is what makes it distinguishable from an item with no values. + item="$(printf '%s' "$answered" | jq -c ' + (.data.node.fieldValues.nodes? // empty) + | [ .[] | select((.name? != null) and (.field?.name? != null)) + | {key: .field.name, value: .name} ] + | from_entries')" \ + || fail_transient "the field values of item ${item_id} in project ${PROJECT} could not be read, so its fields are unknown" [ -n "$item" ] \ - || fail_transient "item ${item_id} was not in the first ${ITEM_LIST_LIMIT} items of project ${PROJECT}, so its fields are unknown" + || fail_transient "the field values of item ${item_id} in project ${PROJECT} could not be obtained, so its fields are unknown" } - # What the listing reports this item's named field as, empty where the board - # reports none. gh names a field's key after the field itself, so the name is - # matched with its spaces removed and its case ignored. + # What the read reports this item's named field as, empty where the board + # reports none. The name is matched with its spaces removed and its case + # ignored on both sides, so a board whose field names carry spaces resolves + # the same way however the key is spelled. board_value() { printf '%s' "$item" | jq -r --arg name "$1" \ - '[to_entries[] | select((.key | ascii_downcase) == ($name | gsub(" "; "") | ascii_downcase)) | .value] | .[0] // "" | tostring' + '[to_entries[] | select((.key | gsub(" "; "") | ascii_downcase) == ($name | gsub(" "; "") | ascii_downcase)) | .value] | .[0] // "" | tostring' } field_id_for() { diff --git a/templates/sync/github.sh b/templates/sync/github.sh index ae7c49f..969e762 100644 --- a/templates/sync/github.sh +++ b/templates/sync/github.sh @@ -135,21 +135,13 @@ WORKFLOW_FIELD="${L5_SYNC_WORKFLOW_FIELD:-}" # wants — and a payload carrying no category applies no label at all. CATEGORY_LABEL_PREFIX="${L5_SYNC_CATEGORY_LABEL_PREFIX:-l5-}" -# The colour a category label is created with. Not an L5_SYNC_ constant, for the -# reason ITEM_LIST_LIMIT below is not: it is a mechanic rather than something -# this target files against. It exists so the create is idempotent — gh's +# The colour a category label is created with. Not an L5_SYNC_ constant: it is a +# mechanic rather than something this target files against. It exists so the +# create is idempotent — gh's # --force updates a label that already exists rather than failing on it, and a # create naming no colour would give the label a fresh random one every filing. CATEGORY_LABEL_COLOR="ededed" -# How much of the project's item listing is read when looking for the item this -# invocation just added. Not an L5_SYNC_ constant, because it is a mechanic -# rather than something this target files against: it bounds a read, and an -# item the listing did not report is answered transiently rather than read as a -# set of empty fields, so a bound that was too small costs a pending entry and -# never an overwritten value. -ITEM_LIST_LIMIT=5000 - # The searchable marker written once per path the payload carries. # templates/query/github.sh searches for exactly this prefix, and a test reads # this line out of both files and asserts the two strings are the same, so the @@ -285,8 +277,8 @@ if [ -n "$PROJECT" ]; then || fail_transient "the issue was added to project ${PROJECT} but the item id could not be read" [ -n "$item_id" ] || fail_transient "the issue was added to project ${PROJECT} but it named no item" - # The project's id, its field list and its item listing are each read at most - # once per filing rather than once per field, so the writes below do not + # The project's id, its field list and this item's own field values are each + # read at most once per filing rather than once per field, so the writes below do not # multiply the reads. Each is read the first time something needs it, so a # filing with nothing to write makes none of these calls. project_id="" @@ -306,27 +298,56 @@ if [ -n "$PROJECT" ]; then read_the_item() { [ -z "$item" ] || return 0 - # What the board already says about this item. An item the listing did not - # report is a failure to know rather than a set of empty fields: writing on - # the strength of a listing that did not mention the item would overwrite - # values a human put there. - local listed - listed="$(gh project item-list "$PROJECT" --owner "$PROJECT_OWNER" \ - --limit "$ITEM_LIST_LIMIT" --format json 2>/dev/null)" \ - || fail_transient "the project ${PROJECT} listing failed, so the item's fields are unknown" - item="$(printf '%s' "$listed" \ - | jq -c --arg id "$item_id" '[.items[]? | select(.id == $id)] | .[0] // empty')" \ - || fail_transient "the project ${PROJECT} listing could not be read" + # What the board already says about this item, asked for by the item's own + # node id rather than selected out of a listing of the whole project. The id + # is already in hand, so one graphql read answers the same question + # consistently: it is not read against an index that lags behind an add, and + # it has no size to outgrow as the board grows. + # + # An item whose field values could not be obtained -- the read failing, or + # answering with no such node -- is a failure to know rather than a set of + # empty fields: writing on the strength of an answer that did not describe + # the item would overwrite values a human put there. + local answered + answered="$(gh api graphql -f item="$item_id" -f query=' + query($item: ID!) { + node(id: $item) { + ... on ProjectV2Item { + fieldValues(first: 100) { + nodes { + ... on ProjectV2ItemFieldSingleSelectValue { + name + field { ... on ProjectV2FieldCommon { name } } + } + } + } + } + } + }' 2>/dev/null)" \ + || fail_transient "the field values of item ${item_id} in project ${PROJECT} could not be read, so its fields are unknown" + # One object mapping field name to value. Only single-select values are + # selected, because those are the only ones this script writes, and an entry + # missing either half is dropped -- so a field the board reports no value for + # contributes no key and reads as empty, exactly as it did under the listing. + # An answer carrying no such node yields nothing at all rather than an empty + # object, which is what makes it distinguishable from an item with no values. + item="$(printf '%s' "$answered" | jq -c ' + (.data.node.fieldValues.nodes? // empty) + | [ .[] | select((.name? != null) and (.field?.name? != null)) + | {key: .field.name, value: .name} ] + | from_entries')" \ + || fail_transient "the field values of item ${item_id} in project ${PROJECT} could not be read, so its fields are unknown" [ -n "$item" ] \ - || fail_transient "item ${item_id} was not in the first ${ITEM_LIST_LIMIT} items of project ${PROJECT}, so its fields are unknown" + || fail_transient "the field values of item ${item_id} in project ${PROJECT} could not be obtained, so its fields are unknown" } - # What the listing reports this item's named field as, empty where the board - # reports none. gh names a field's key after the field itself, so the name is - # matched with its spaces removed and its case ignored. + # What the read reports this item's named field as, empty where the board + # reports none. The name is matched with its spaces removed and its case + # ignored on both sides, so a board whose field names carry spaces resolves + # the same way however the key is spelled. board_value() { printf '%s' "$item" | jq -r --arg name "$1" \ - '[to_entries[] | select((.key | ascii_downcase) == ($name | gsub(" "; "") | ascii_downcase)) | .value] | .[0] // "" | tostring' + '[to_entries[] | select((.key | gsub(" "; "") | ascii_downcase) == ($name | gsub(" "; "") | ascii_downcase)) | .value] | .[0] // "" | tostring' } field_id_for() { diff --git a/tests/test_filed_query.py b/tests/test_filed_query.py index 69a1629..b6d812d 100644 --- a/tests/test_filed_query.py +++ b/tests/test_filed_query.py @@ -1289,8 +1289,8 @@ def reference_script(directory: str) -> str: #: How a test tells the stub to break on purpose. Two variables rather than #: one, because the two failures they cause are different claims: a call that -#: fails is a tracker the sync cannot write to, and a listing that reports -#: nothing is a board the sync cannot *read* — which the script must not +#: fails is a tracker the sync cannot write to, and a read that answers with no +#: such node is a board the sync cannot *read* — which the script must not #: mistake for an item whose Status is empty. #: #: What the first names is a call rather than a project subcommand: the label @@ -1304,7 +1304,18 @@ def reference_script(directory: str) -> str: #: `item-edit`. LABEL_CREATE_CALL = "label-create" LABEL_ADD_CALL = "issue-edit" -OMIT_VARIABLE = "L5_STUB_ITEM_LIST_REPORTS_NOTHING" + +#: How the ledger names the read the sync makes of one item's own field values. +#: The project subcommands are recorded under their own names; this one is not +#: a project subcommand, so — like the two label calls above — it carries a +#: name of the stub's rather than one taken from an argument. +GRAPHQL_CALL = "api-graphql" + +#: What makes that read answer with no such node while the item is in fact on +#: the board. That is what a tracker whose read has not caught up with an add +#: looks like, and it is the case the script must answer transiently rather +#: than read as a set of empty fields. +OMIT_VARIABLE = "L5_STUB_ITEM_READ_REPORTS_NOTHING" STUB_GH = '''#!INTERPRETER """A stub `gh`, standing in for a tracker and its project board. It reaches no @@ -1335,11 +1346,17 @@ def reference_script(directory: str) -> str: project item-add adds the url to a project, or reports the item already there rather than adding a second one, which is the behaviour the sync script's retry depends on. - project item-list the project's items. A field an item has no value for - carries no key at all, which is how gh reports one, so a - script that read a missing key as an empty string and a - script that could not tell them apart are distinguishable - here. + api graphql one item's own field values, looked up by the node id the + invocation carries. A field the item has no value for + contributes no node at all, which is how the real answer + reports one, and an empty node stands beside them for a + value of a type the query's fragment does not match — so + a script that read a missing field as an empty string and + a script that could not tell them apart are + distinguishable here. An item the read cannot resolve is + answered with a null node rather than with an empty list + of values, which is the distinction the sync script's + transient exit rests on. project view the project's node id. project field-list the project's fields and their options, by name. project item-edit sets one single-select field on one item, by ids. The @@ -1349,10 +1366,13 @@ def reference_script(directory: str) -> str: field by field rather than as one column. The ledger holds the issues, the repository's labels, the projects and every -project invocation that was made, so a test can assert on a call that was *not* -made as well as on one that was. FAIL_VARIABLE names calls that must exit -non-zero, and OMIT_VARIABLE makes `item-list` report a project with no items in -it. +board invocation that was made — the project subcommands under their own names +and the item read under GRAPHQL_CALL — so a test can assert on a call that was +*not* made as well as on one that was. A subcommand this stub does not +implement is recorded before it is refused, so an invocation the sync script +must no longer make is visible in the ledger rather than only in its exit +status. FAIL_VARIABLE names calls that must exit non-zero, and OMIT_VARIABLE +makes the item read answer with no such node. """ import json import os @@ -1473,6 +1493,37 @@ def record_issue_call(command): else: print(json.dumps([{name: issue.get(name) for name in fields} for issue in matched])) +elif argv[:2] == ["api", "graphql"]: + # One item's own field values, by the node id the invocation carries. + # Recorded in the same ledger the project subcommands are, so "the read was + # made once" and "the listing was not made at all" are both readable there. + state["calls"].append({"command": "GRAPHQL_CALL", "argv": argv}) + save() + if told_to_fail("GRAPHQL_CALL"): + refuse("the stub was told to fail at api graphql") + variables = dict(pair.split("=", 1) + for index, pair in enumerate(argv) + if index and argv[index - 1] == "-f" and "=" in pair) + wanted = variables.get("item") + holding = [project for project in projects.values() + for one in project["items"] if one["id"] == wanted] + if os.environ.get("OMIT_VARIABLE") or not holding: + # No such node: a well formed answer that describes no item. It is + # deliberately not an item carrying an empty list of values, because + # those two are what the sync script must tell apart. + print(json.dumps({"data": {"node": None}})) + else: + project = holding[0] + item = [one for one in project["items"] if one["id"] == wanted][0] + # An empty node stands for a field value of a type the query's inline + # fragment does not match, which is what the real answer carries for + # every value that is not a single select. + nodes = [{}] + for field in project["fields"]: + held = item.get(board_key(field["name"])) + if held: + nodes.append({"name": held, "field": {"name": field["name"]}}) + print(json.dumps({"data": {"node": {"fieldValues": {"nodes": nodes}}}})) elif argv[:1] == ["project"]: subcommand = argv[1] # Recorded before the refusal below, so a call a test told the stub to fail @@ -1522,20 +1573,6 @@ def record_issue_call(command): save() print(json.dumps({"id": item["id"], "type": "Issue", "url": item["url"]})) - elif subcommand == "item-list": - limit = int(flag(argv, "--limit", "30")) - reported = ([] if os.environ.get("OMIT_VARIABLE") - else project["items"][:limit]) - listed = [] - for one in reported: - shown = {"id": one["id"], - "content": {"type": "Issue", "url": one["url"]}} - for field in project["fields"]: - key = board_key(field["name"]) - if one.get(key): - shown[key] = one[key] - listed.append(shown) - print(json.dumps({"items": listed})) else: refuse("the stub was asked for something it does not do: %s" % " ".join(argv)) @@ -1615,6 +1652,7 @@ def stub_tracker(tmp_path: Path) -> tuple[dict, Path]: .replace("LEDGER_VARIABLE", LEDGER_VARIABLE) .replace("FAIL_VARIABLE", FAIL_VARIABLE) .replace("OMIT_VARIABLE", OMIT_VARIABLE) + .replace("GRAPHQL_CALL", GRAPHQL_CALL) .replace("LABEL_CREATE_CALL", LABEL_CREATE_CALL) .replace("LABEL_ADD_CALL", LABEL_ADD_CALL)) ledger.write_text(json.dumps( @@ -1673,7 +1711,12 @@ def board_items(ledger: Path) -> list[dict]: def project_calls(ledger: Path, command: str | None = None) -> list[dict]: - """Every project invocation the stub was made, optionally by subcommand.""" + """Every board invocation the stub was made, optionally by name. + + The project subcommands are recorded under their own names and the read of + one item's field values under `GRAPHQL_CALL`, so an assertion that a + particular call was *not* made is made of the same ledger either way. + """ return [call for call in ledger_state(ledger)["calls"] if command is None or call["command"] == command] @@ -1821,6 +1864,18 @@ def test_the_query_script_answers_nothing_known_when_its_search_fails(tmp_path): PROJECT_SUBCOMMANDS = sorted(set(re.findall( r"gh project ([a-z-]+)", TEMPLATE_SYNC.read_text(encoding="utf-8")))) +#: How the script asks for one item's own field values, read off the script for +#: the same reason. The read is not a project subcommand, so it is not among the +#: names above and would otherwise drop out of the sweep of every call made +#: after the issue exists. +GRAPHQL_INVOCATIONS = re.findall( + r"gh api graphql", TEMPLATE_SYNC.read_text(encoding="utf-8")) + +#: Every board call the script makes, under the names the ledger records them +#: by. The project subcommands answer to their own names and the item read +#: answers to the stub's. +BOARD_CALLS = tuple(PROJECT_SUBCOMMANDS) + (GRAPHQL_CALL,) + #: What the transport reads as "the entry stays pending and a later sweep #: retries it". Named rather than written as a bare 75 beside each assertion. TRANSIENT_EXIT = 75 @@ -1879,9 +1934,11 @@ def sync_to_the_board(script: Path, tmp_path: Path, environment: dict, *, breaking: dict | None = None): """One invocation of `script` against the stub's board. - `breaking` is whatever the stub is to be broken with for this invocation - alone, so a test can drive the same key twice with the board failing the - first time and answering the second. + `breaking` is whatever this invocation alone is driven with beside the + board values — most often what the stub is to be broken with, so a test can + drive the same key twice with the board failing the first time and + answering the second, and sometimes a board value overridden for one + invocation. """ return run_the_sync( script, tmp_path, environment, key=key, payload=payload or AN_ENTRY, @@ -1904,6 +1961,9 @@ def test_the_template_declares_the_constants_the_board_tests_override(): for name in OVERRIDDEN_CONSTANTS: assert TEMPLATE_CONSTANTS[name][0].startswith(SYNC_VARIABLE_PREFIX), name assert PROJECT_SUBCOMMANDS, "the script invokes no project subcommand" + assert GRAPHQL_INVOCATIONS, \ + "the script asks for no item's field values, so the sweep over the " \ + "calls it makes after the issue exists would not cover that read" def test_every_axis_the_classification_is_written_over_carries_values(): @@ -1944,16 +2004,18 @@ def test_an_entry_filed_with_a_project_configured_lands_on_the_board( @needs_jq @pytest.mark.parametrize("script", BOTH_SYNC_COPIES) -@pytest.mark.parametrize("subcommand", PROJECT_SUBCOMMANDS) +@pytest.mark.parametrize("subcommand", BOARD_CALLS) def test_every_failure_after_the_issue_exists_is_transient( subcommand, script, tmp_path): """The issue is the record and the board is a view of it. - Each project call the script makes is failed in turn, and each must exit 75 - rather than 0 or 1: a zero would report an entry as landed with the board - call lost, and a non-zero that is not 75 would fail the entry terminally - and lose it. The issue is filed either way, which is what makes the retry - the next sweep performs find it rather than create a second one. + Each board call the script makes is failed in turn — the project + subcommands and the read of the item's own field values alike — and each + must exit 75 rather than 0 or 1: a zero would report an entry as landed + with the board call lost, and a non-zero that is not 75 would fail the + entry terminally and lose it. The issue is filed either way, which is what + makes the retry the next sweep performs find it rather than create a second + one. """ environment, ledger = stub_tracker(tmp_path) result = sync_to_the_board(script, tmp_path, environment, key="k-fails", @@ -1962,7 +2024,7 @@ def test_every_failure_after_the_issue_exists_is_transient( assert result.returncode == TRANSIENT_EXIT, (result.returncode, result.stderr) assert len(ledger_state(ledger)["issues"]) == 1 assert project_calls(ledger, subcommand), \ - f"the script never invoked project {subcommand}" + f"the script never made the {subcommand} call" @needs_jq @@ -2055,35 +2117,98 @@ def test_an_item_whose_status_the_board_reports_is_left_where_it_is( assert project_calls(ledger, "item-edit") == [] -@needs_jq -@pytest.mark.parametrize("script", BOTH_SYNC_COPIES) -def test_an_item_the_listing_did_not_report_is_a_failure_to_know( - script, tmp_path): - """A listing that did not return the item is not an empty Status. +#: The two ways the item's own field values can fail to be obtained, as the +#: stub is told to produce them: a read that answers with no such node while +#: the item is in fact there, and a read that fails outright. Both are a +#: failure to know rather than a set of empty fields, and the script must +#: answer both the same way. +WAYS_OF_NOT_KNOWING = [ + pytest.param({OMIT_VARIABLE: "1"}, id="no-such-node"), + pytest.param({FAIL_VARIABLE: GRAPHQL_CALL}, id="read-failed"), +] - The stub reports a project with no items in it while the item is in fact - there, which is what a listing bounded too short or a tracker answering - partially looks like. Read as an empty Status it would be overwritten; read - as a failure to know it is answered transiently and left alone. - The control is the same drive with the listing answering, below the - assertion: there the `item-edit` is made and the Status is written, so the +@needs_jq +@pytest.mark.parametrize("script", BOTH_SYNC_COPIES) +@pytest.mark.parametrize("breaking", WAYS_OF_NOT_KNOWING) +def test_an_item_whose_field_values_were_not_obtained_is_a_failure_to_know( + breaking, script, tmp_path): + """A read that did not describe the item is not an empty Status. + + The item is on the board and the read either fails or answers with no such + node — which is what a tracker whose read has not caught up with the add + looks like. Read as an empty Status the item would be written into; read as + a failure to know it is answered transiently and left exactly as it was. + + The control is the same drive with the read answering, below the + assertions: there the `item-edit` is made and the Status is written, so the absence here is the guard and not a write that never happens. """ environment, ledger = stub_tracker(tmp_path) - blind = sync_to_the_board(script, tmp_path, environment, key="k-unlisted", - breaking={OMIT_VARIABLE: "1"}) + brief = a_filed_brief() + blind = sync_to_the_board(script, tmp_path, environment, key="k-unread", + payload=brief, breaking=breaking) assert blind.returncode == TRANSIENT_EXIT, (blind.returncode, blind.stderr) items = board_items(ledger) assert len(items) == 1, items assert "status" not in items[0], items[0] + for axis in CLASSIFICATION: + assert board_field_value(ledger, axis.field_name) == "", axis.field_name assert project_calls(ledger, "item-edit") == [] - seeing = sync_to_the_board(script, tmp_path, environment, key="k-unlisted") + seeing = sync_to_the_board(script, tmp_path, environment, key="k-unread", + payload=brief) assert seeing.returncode == 0, seeing.stderr assert board_items(ledger)[0]["status"] == THIS_TARGETS_STATUS_OPTION - assert len(project_calls(ledger, "item-edit")) == 1 + for axis in CLASSIFICATION: + assert board_field_value(ledger, axis.field_name) == \ + str(brief[axis.payload_field]), axis.field_name + assert len(project_calls(ledger, "item-edit")) == 1 + len(CLASSIFICATION) + + +def digits_beside(message: str, *names: str) -> str: + """Every digit in `message` that is not part of one of `names`. + + The transient message is required to name the item and the project, both of + which are spelled with digits on this board, so "and no count of items" is + asserted of what is left once those two are taken out. + """ + for name in sorted(names, key=len, reverse=True): + message = message.replace(name, " ") + return "".join(character for character in message if character.isdigit()) + + +@needs_jq +@pytest.mark.parametrize("script", BOTH_SYNC_COPIES) +@pytest.mark.parametrize("breaking", WAYS_OF_NOT_KNOWING) +def test_the_transient_answer_names_the_item_and_the_project_and_no_count( + breaking, script, tmp_path): + """A developer reading that line is not told about a bound. + + The message the script used to give named the size of a listing, which was + the wrong cause on every board smaller than it — so what is asserted is + that the item and the project are named and that no other number is. + + The absence of a number is controlled beside itself: the same reduction + over the message this one replaced, constructed here rather than read out + of the tree, does report a number. + """ + environment, ledger = stub_tracker(tmp_path) + blind = sync_to_the_board(script, tmp_path, environment, key="k-unread", + breaking=breaking) + assert blind.returncode == TRANSIENT_EXIT, blind.stderr + + item_id = board_items(ledger)[0]["id"] + said = blind.stderr.strip() + assert item_id in said, said + assert THIS_TARGETS_PROJECT in said, said + assert digits_beside(said, item_id, THIS_TARGETS_PROJECT) == "", said + + superseded = (f"item {item_id} was not in the first 5000 items of project " + f"{THIS_TARGETS_PROJECT}, so its fields are unknown") + assert digits_beside(superseded, item_id, THIS_TARGETS_PROJECT) != "", \ + "the reduction reports no number in a message that names a bound" @needs_jq @@ -2498,23 +2623,164 @@ def test_a_field_whose_options_lack_the_value_costs_that_field_alone( @needs_jq @pytest.mark.parametrize("script", BOTH_SYNC_COPIES) -def test_the_project_the_fields_and_the_listing_are_read_once_per_filing( +def test_the_project_the_fields_and_the_item_are_read_once_per_filing( script, tmp_path): """Six writes rather than six reads each. Each of the three reads is asserted to have been made exactly once: an upper bound alone would pass a script that made none of them, and this - filing needs all three, so the equality carries both halves. + filing needs all three, so the equality carries both halves. The third is + the read of this item's own field values, which every one of the six writes + consults and which none of them may repeat. + + The item-edit count beside them is what makes "six writes" the premise + rather than an assumption. """ environment, ledger = stub_tracker(tmp_path) result = sync_to_the_board(script, tmp_path, environment, key="k-read-once", payload=a_filed_brief()) assert result.returncode == 0, result.stderr - for subcommand in ("view", "field-list", "item-list"): + assert len(project_calls(ledger, "item-edit")) == 1 + len(CLASSIFICATION) + for subcommand in ("view", "field-list", GRAPHQL_CALL): assert len(project_calls(ledger, subcommand)) == 1, \ (subcommand, project_calls(ledger, subcommand)) + read = project_calls(ledger, GRAPHQL_CALL)[0] + assert board_items(ledger)[0]["id"] in " ".join(read["argv"]), read + + +#: A field name with a space in it. The board this module seeds has none, and +#: the two spellings of such a name — as the board declares it and as the +#: listing used to key it — are what the lookup has to reconcile, so a board +#: carrying one is built here rather than assumed absent. +A_FIELD_NAME_CARRYING_A_SPACE = "Some Category" + + +@needs_jq +@pytest.mark.parametrize("script", BOTH_SYNC_COPIES) +def test_a_field_whose_name_carries_a_space_is_written_once_and_then_left( + script, tmp_path): + """A board naming a field with a space resolves as one naming it without. + + The read keys a value by the field's name as the board declares it, and the + listing it replaces keyed the same value by that name with the spaces taken + out — so a lookup tolerant of only one of those spellings writes such a + field on every sweep for ever, moving a value a person set. + + Both halves are driven: the first filing finds the field empty and writes + it, and the second over the same entry reads back what it wrote and makes + no edit at all. The write is what controls the absence. + """ + environment, ledger = stub_tracker(tmp_path) + axis = CLASSIFICATION[0] + brief = a_filed_brief() + + def rename(project): + for field in project["fields"]: + if field["name"] == axis.field_name: + field["name"] = A_FIELD_NAME_CARRYING_A_SPACE + rewrite_the_board(ledger, rename) + spaced = {TEMPLATE_CONSTANTS[axis.constant][0]: + A_FIELD_NAME_CARRYING_A_SPACE} + + first = sync_to_the_board(script, tmp_path, environment, key="k-spaced", + payload=brief, breaking=spaced) + assert first.returncode == 0, first.stderr + written = board_field_value(ledger, A_FIELD_NAME_CARRYING_A_SPACE) + assert written == str(brief[axis.payload_field]), written + + state = ledger_state(ledger) + state["calls"] = [] + ledger.write_text(json.dumps(state), encoding="utf-8") + + again = sync_to_the_board(script, tmp_path, environment, key="k-spaced", + payload=brief, breaking=spaced) + assert again.returncode == 0, again.stderr + assert board_field_value(ledger, A_FIELD_NAME_CARRYING_A_SPACE) == written + assert project_calls(ledger, "item-edit") == [], \ + "a field the board already reports was written over" + + +#: The subcommand that listed a whole project to find one item in it. Written +#: here rather than derived, because what is asserted is that no script and no +#: filing names it any more — a name derived from the scripts would be the +#: empty string and the assertion would hold of everything. +THE_RETIRED_LISTING = "item-list" + + +@needs_jq +@pytest.mark.parametrize("script", BOTH_SYNC_COPIES) +def test_a_filing_that_writes_every_field_lists_no_project(script, tmp_path): + """The board is never listed, however many fields are written. + + A filing that sets the Status and all five classification fields is driven, + and the ledger must hold no listing at all. The absence is controlled twice + over: the read that replaces the listing *is* in the same ledger, asserted + above; and the stub is then invoked with the listing directly, which the + ledger does record — so silence here is the script and not a ledger that + cannot see a listing. + """ + environment, ledger = stub_tracker(tmp_path) + result = sync_to_the_board(script, tmp_path, environment, key="k-no-list", + payload=a_filed_brief()) + assert result.returncode == 0, result.stderr + + assert project_calls(ledger, THE_RETIRED_LISTING) == [] + assert [call for call in project_calls(ledger) + if THE_RETIRED_LISTING in call["argv"]] == [] + assert project_calls(ledger, GRAPHQL_CALL), \ + "the ledger holds no board read at all, so it saw nothing either way" + + subprocess.run( + [sys.executable, str(tmp_path / "stub-bin" / "gh"), "project", + THE_RETIRED_LISTING, THIS_TARGETS_PROJECT, + "--owner", THIS_TARGETS_PROJECT_OWNER, "--format", "json"], + capture_output=True, text=True, timeout=60, env=environment) + assert project_calls(ledger, THE_RETIRED_LISTING), \ + "the ledger does not record a listing that was in fact made" + + +#: A board with more items on it than the listing the scripts used to make was +#: ever bounded at. Not derived from either copy: the bound this story retired +#: is gone from both of them, so this is the size of the board the test builds +#: rather than a number read off anything. +A_BOARD_LARGER_THAN_THE_RETIRED_BOUND = 5001 + + +@needs_jq +@pytest.mark.parametrize("script", BOTH_SYNC_COPIES) +def test_a_board_larger_than_the_retired_bound_files_the_item_added_last( + script, tmp_path): + """The board a bounded listing could never have reported this item on. + + The item a filing adds sits at the end of the board's order, so on a board + past the retired bound it was exactly the item the listing could not + report, and the entry stayed pending for ever. Asked for by its own node id + there is no size to outgrow, so the filing lands. + """ + environment, ledger = stub_tracker(tmp_path) + + def crowd(project): + project["items"] = [ + {"id": "PVTI_%d" % (number + 1), + "url": "https://tracker.invalid/issues/already-%d" % (number + 1)} + for number in range(A_BOARD_LARGER_THAN_THE_RETIRED_BOUND)] + rewrite_the_board(ledger, crowd) + + result = sync_to_the_board(script, tmp_path, environment, key="k-crowded", + payload=a_filed_brief()) + assert result.returncode == 0, result.stderr + + items = board_items(ledger) + assert len(items) == A_BOARD_LARGER_THAN_THE_RETIRED_BOUND + 1 + filed = items[-1] + assert filed["url"] == result.stdout.strip().splitlines()[-1] + assert filed["status"] == THIS_TARGETS_STATUS_OPTION + for axis in CLASSIFICATION: + assert filed.get(axis.field_name.replace(" ", "").lower()) == \ + str(a_filed_brief()[axis.payload_field]), axis.field_name + @needs_jq @pytest.mark.parametrize("script", BOTH_SYNC_COPIES) @@ -2709,6 +2975,52 @@ def test_that_comparison_reports_a_difference_that_is_not_a_constant(tmp_path): assert any("fail_transient_renamed" in line for line in reported), reported +# -------------------------------------------------------------------------- +# Neither copy lists a project, and neither carries a bound on doing so +# -------------------------------------------------------------------------- + + +#: The constant that bounded the listing, and the invocation it bounded. Both +#: are written here rather than derived: the whole claim is that neither +#: appears, and a name derived from the files being scanned would be the empty +#: string, which appears in every file there is. +THE_RETIRED_BOUND = "ITEM_LIST_LIMIT" +THE_RETIRED_INVOCATION = f"gh project {THE_RETIRED_LISTING}" + + +def listing_mentions(text: str) -> list[str]: + """Every line naming the retired bound or the listing it bounded.""" + return [line for line in text.splitlines() + if THE_RETIRED_BOUND in line or THE_RETIRED_INVOCATION in line] + + +@pytest.mark.parametrize("script", BOTH_SYNC_COPIES) +def test_no_sync_script_lists_a_project_or_bounds_a_listing(script): + """A shipped artifact and the subject. + + Neither copy may name the bound or make the invocation, and the two are + scanned for together because deleting one without the other leaves a + constant nothing reads or a listing nothing bounds. The control is below: + the same scan over a rendering of the same script with the listing put back + reports both lines, so silence here is the file rather than a scan that + matches nothing. + """ + assert listing_mentions(script.read_text(encoding="utf-8")) == [] + + +def test_that_scan_reports_a_listing_put_back_into_a_sync_script(): + """The control, on a rendering rather than on the tree.""" + restored = TEMPLATE_SYNC.read_text(encoding="utf-8").replace( + 'echo "$url"', + f'{THE_RETIRED_BOUND}=5000\n' + f'{THE_RETIRED_INVOCATION} "$PROJECT" --limit "${THE_RETIRED_BOUND}"\n' + 'echo "$url"') + reported = listing_mentions(restored) + + assert any(THE_RETIRED_BOUND in line for line in reported), reported + assert any(THE_RETIRED_INVOCATION in line for line in reported), reported + + # -------------------------------------------------------------------------- # A sync command must not commit # --------------------------------------------------------------------------