diff --git a/.github/workflows/foc-board-mechanical-rules.yml b/.github/workflows/foc-board-mechanical-rules.yml index 3c6070c..b6e7099 100644 --- a/.github/workflows/foc-board-mechanical-rules.yml +++ b/.github/workflows/foc-board-mechanical-rules.yml @@ -14,6 +14,15 @@ on: - 'true' - 'false' +# Serialize runs rather than letting them overlap: two concurrent runs would +# both restore the same mutation-log cache snapshot, could both move the same +# item, and the later cache save could silently drop the other run's +# mutations. cancel-in-progress stays false so a run in flight (which may +# already be mid-mutation) always finishes rather than being cut off. +concurrency: + group: foc-board-mechanical-rules + cancel-in-progress: false + jobs: apply-rules: runs-on: ubuntu-latest diff --git a/foc-board-rules/field-completeness.md b/foc-board-rules/field-completeness.md index 124a0ba..566e7f1 100644 --- a/foc-board-rules/field-completeness.md +++ b/foc-board-rules/field-completeness.md @@ -164,3 +164,13 @@ If no reasonable inference can be made, **leave Cycle Theme blank** — do not i **How the removal check works, and its limit:** see [`foc-mechanical-rules`'s "Mutation log" section](../foc-mechanical-rules/README.md#mutation-log) for why GitHub can't answer this directly and how the tool tracks it instead. The short version: this rule can only detect a removal it itself witnessed. A cycle a human cleared *before* this rule ever set it (or before that history existed) won't be caught — the item will just look like any other item missing a Cycle and will get (re-)assigned. **Relationship to [R-FC-006](#r-fc-006-in-flight-prs-without-a-cycle-should-be-in-the-current-cycle) / [R-FC-009](#r-fc-009-in-flight-items-in-active-milestones-should-have-a-cycle):** Those rules are status/milestone-gated and remain the sweep's judgment-call fallback for items this rule's 3-day activity window doesn't reach (e.g. an in-flight item that's gone quiet for a week). This rule is the broader, simpler, purely time-based mechanical subset — any issue or PR, not just in-flight PRs or active-milestone items. **Why:** Items with recent activity and no Cycle are a planning gap — they're clearly live work but invisible in cycle views. A 3-day activity window catches this quickly without pulling in stale backlog the way an unconditional "everything gets a cycle" rule would. + +## R-FC-013: Open items in a past cycle should move to the current cycle + +**Enforced mechanically, hourly:** this rule runs automatically via [`foc-mechanical-rules`](../foc-mechanical-rules/) (see [its implementation](../foc-mechanical-rules/foc_mechanical_rules/rules/cycle.py)), scheduled by [`.github/workflows/foc-board-mechanical-rules.yml`](../.github/workflows/foc-board-mechanical-rules.yml). The prose below stays canonical for *what* and *why*; the linked module is canonical for exactly how it's evaluated. + +**When:** Any **issue or PR** on the board (any status except "🎉 Done") has its Cycle set to an iteration whose date range has already ended (a "past cycle"). +**Action:** Set the Cycle to the current active cycle (the iteration whose date range contains today). +**Scope:** Only applies when the item's Cycle is a *past* iteration. An item with no Cycle at all is [R-FC-012](#r-fc-012-recently-active-items-without-a-cycle-get-the-current-cycle)'s concern, not this one's. An item whose Cycle is a *future* iteration (deliberately planned ahead) is left alone. +**Skip if a human has since moved it back:** If this rule (or any mutation this tool made under this rule's id) previously moved the item's Cycle *away from* the past-cycle value it currently holds, do not re-apply — flag for human review instead. A human moving an item back to a past cycle after this tool moved it forward is a deliberate signal (e.g. correcting a mistaken auto-move, or intentionally leaving it attributed to the cycle where the work actually happened) that automation shouldn't fight. See [`foc-mechanical-rules`'s "Mutation log" section](../foc-mechanical-rules/README.md#mutation-log) for how this is tracked and its limits (only reversions this tool's own history witnessed are caught). +**Why:** An item still open once its cycle has ended almost always means the cycle ended before the work did — the Cycle value is now stale and understates what's actually in flight this cycle. Leaving it in the old iteration hides the work from current cycle planning and reporting. diff --git a/foc-board-rules/future-ideas.md b/foc-board-rules/future-ideas.md index 8429138..5280440 100644 --- a/foc-board-rules/future-ideas.md +++ b/foc-board-rules/future-ideas.md @@ -4,7 +4,7 @@ Ideas for improving the FOC board tooling, collected during rule application ses ## Move the mechanical rules out of the LLM entirely -**Status: In progress.** [`foc-mechanical-rules`](../foc-mechanical-rules/) now runs hourly via GitHub Actions: R-PR-001 (unassigned PR -> author) and R-FC-012 (recently-active item with no Cycle -> current cycle). The rule-per-field, decision-table design is meant to grow: R-PR-002/003/004 (dependabot/release-PR theme+status) and R-PR-008/009 (merged/closed -> Done) are the natural next slice, following the same `Rule` pattern (`select` + `apply_one`, registered in `registry.py`). +**Status: In progress.** [`foc-mechanical-rules`](../foc-mechanical-rules/) now runs hourly via GitHub Actions: R-PR-001 (unassigned PR -> author), R-FC-012 (recently-active item with no Cycle -> current cycle), and R-FC-013 (open item in a past cycle -> current cycle). The rule-per-field, decision-table design is meant to grow: R-PR-002/003/004 (dependabot/release-PR theme+status) and R-PR-008/009 (merged/closed -> Done) are the natural next slice, following the same `Rule` pattern (`select` + `apply_one`, registered in `registry.py`). **New sub-problem this surfaced:** rules that need their own mutation history (see [`foc-mechanical-rules`'s "Mutation log" section](../foc-mechanical-rules/README.md#mutation-log) for why) currently get it persisted via GitHub Actions cache, which isn't a truly durable store (eviction after ~7 days unused, no cross-repo/cross-workflow access). That's an acceptable v1 tradeoff since the library itself doesn't assume any particular persistence mechanism — only the CLI/workflow-level wiring would need to change. If more rules end up depending on this history, or cache eviction ever causes a visible miss, worth moving it somewhere durable: a small persisted store the REST API server owns, for example. @@ -16,6 +16,14 @@ Roughly 60% of the 2026-07-07 sweep's ~395 mutations (dependabot theme/status/cy **Triggered by:** agent feedback after the 2026-07-07 full sweep (~395 mutations across 6 stages). +## Batch mutations in foc-mechanical-rules + +**Status: Not started.** `github_projects_client`'s `set_field_value_bulk` already batches GraphQL mutations 25-at-a-time via aliased queries — but every rule calls `set_field_value`, a thin wrapper that always passes a 1-item list, so the batching path never actually batches anything today. A live R-FC-013 dry-run against the real board (2026-08-22) found 179 items needing a mutation; at 1 GraphQL request per item that's 179 round trips this run alone, versus ~8 if they were batched. + +**Why it's not just a drop-in fix:** the shared `Rule.run()`/`apply_one` contract (see README.md's "Design" section) interleaves per-item decision logic (skip/flag/error, mutation-log guards) with the actual mutation call, one item at a time. Batching means splitting that into two phases — decide which items to mutate (unchanged, still per-item), then mutate all of them in one `set_field_value_bulk` call — which is a change to the shared base class every rule (current and future) goes through, not something scoped to one rule. `AssigneeRule` doesn't call `set_field_value` at all (it's REST issue/PR endpoints, not board-field mutations), so it gets no benefit but needs to keep working under whatever the new shape is. + +**Triggered by:** implementing R-FC-013 and auditing its API call pattern (see README.md's "API call pattern per rule" section) after a live dry-run. + ## Sweep journal for incremental sweeps Each sweep re-derives the whole board from scratch. Persist the final item-state snapshot at the end of each sweep (one JSON file per sweep, ~200 bytes/item: ref, status, cycle, theme, assignee, board `updated`, GitHub `updatedAt`, flags raised). The next sweep can then run incrementally: only items whose GitHub/board `updated` changed since the snapshot need evaluation. diff --git a/foc-mechanical-rules/README.md b/foc-mechanical-rules/README.md index 8512cef..8c151a9 100644 --- a/foc-mechanical-rules/README.md +++ b/foc-mechanical-rules/README.md @@ -19,7 +19,7 @@ Every `applied`, `flagged`, or `error` outcome is written to the shared [`action ## Mutation log -Some rules need to know what this tool has already done to an item — R-FC-012 (cycle) is the first example: it won't re-add a cycle it previously set if the item now has no cycle, because that's a human's deliberate signal to descope it, not something to silently override. Getting that from GitHub directly isn't possible: GitHub has no change-history API for Projects v2 custom fields other than Status (confirmed by GraphQL schema introspection — `ProjectV2ItemStatusChangedEvent` is the only such timeline event that exists; a separate, similarly-named `IssueFieldChangedEvent` family turned out to belong to an unrelated "Issue Fields" GitHub feature and returned nothing when checked against a real item with a multi-cycle history). +Some rules need to know what this tool has already done to an item — R-FC-012 (cycle) is the first example: it won't re-add a cycle it previously set if the item now has no cycle, because that's a human's deliberate signal to descope it, not something to silently override. R-FC-013 uses the same log the other direction: it won't re-move an item off a past cycle if this tool already moved it off that exact cycle once and the item is back there now — that's a human's deliberate signal to leave it, not something to fight. Getting that from GitHub directly isn't possible: GitHub has no change-history API for Projects v2 custom fields other than Status (confirmed by GraphQL schema introspection — `ProjectV2ItemStatusChangedEvent` is the only such timeline event that exists; a separate, similarly-named `IssueFieldChangedEvent` family turned out to belong to an unrelated "Issue Fields" GitHub feature and returned nothing when checked against a real item with a multi-cycle history). So this tool keeps its own record instead, and treats it explicitly as an *input* to rules rather than an assumption baked into how they run: `mutation_log.py` defines `MutationLog`, an item -> mutations multimap (`for_item(item_ref)` for O(1) lookup, `record(...)` to append). The base `Rule.run()` builds one record per real (non-dry-run) `applied` outcome automatically and adds it to whatever `MutationLog` it's given; any rule's `apply_one` can read it back via `mutation_log.for_item(...)` (see `rules/cycle.py`). **It is not guaranteed comprehensive** — it only knows what was fed in plus what's happened this run — so a rule using it should treat a miss as "no known history," not as proof nothing happened. @@ -31,6 +31,23 @@ Neither `mutation_log.py` nor any rule module knows or cares how the log survive | --- | --- | --- | | [R-PR-001](../foc-board-rules/pr-hygiene.md#r-pr-001-unassigned-prs-should-be-assigned-to-their-author) | assignee | Unassigned open PRs are assigned to their author (skipping bots, with a merged-release-PR carve-out, and skipping PRs where a human explicitly removed the assignee) | | [R-FC-012](../foc-board-rules/field-completeness.md#r-fc-012-recently-active-items-without-a-cycle-get-the-current-cycle) | cycle | Issues/PRs updated in the last 3 days with no Cycle get the current cycle, unless this tool previously set that item's Cycle to the current one and a human has since cleared it | +| [R-FC-013](../foc-board-rules/field-completeness.md#r-fc-013-open-items-in-a-past-cycle-should-move-to-the-current-cycle) | cycle | Open issues/PRs whose Cycle is a past iteration move to the current cycle, unless this tool previously moved that item off the same past cycle and a human has since moved it back | + +## API call pattern per rule + +Board size (~180 open items and growing) makes it easy for a rule to accidentally turn an O(1)-per-run cost into an O(items) one. Three conventions keep that in check, and every rule should follow them: + +1. **`select()` issues exactly one board query**, paginated via `list_items`'s cursor — never a query per candidate. +2. **Anything that's the same for the whole run (e.g. "what's the current cycle?") is resolved once and memoized** on the rule instance (see `CycleRule`/`PastCycleRule`'s `_resolve*` methods), not refetched in every `apply_one` call. +3. **Mutations pass the item's node ID** (`item.get("_node_id")` — `list_items` always includes it, regardless of the requested `fields`), not an `"owner/repo#number"` ref. `github_projects_client`'s `set_field_value`/`set_field_value_bulk` silently does an extra `get_item` read per plain ref to resolve it to a node ID; a node ID skips that lookup entirely and also gets its `old_value` from a batched API read instead of whatever `select()` saw earlier (which can be stale by the time the mutation runs). + +| Rule | `select()` (once per run) | Per-run, memoized | Per-item reads | Per-item writes | +| --- | --- | --- | --- | --- | +| R-PR-001 (assignee) | 1 paginated board query | — | 1 REST `GET` (PR metadata) per candidate; **+1 REST `GET`** (issue events, paginated) unless skipped as a bot author | 1 REST `POST` per applied item | +| R-FC-012 (cycle) | 1 paginated board query | 1 GraphQL query (iterations) | none | 1 GraphQL mutation per applied item (node ID) | +| R-FC-013 (cycle) | 1 paginated board query (Cycle field included, so no separate read is needed to know an item's current cycle) | 1 GraphQL query (iterations, shared helper with R-FC-012) | none | 1 GraphQL mutation per applied item (node ID) | + +**Known gap, not yet worth fixing:** R-PR-001's 1-2 REST reads per candidate PR are real per-item calls with no batched equivalent used today, unlike the two cycle rules. At current volume (dozens of candidates per hourly run, most REST GETs) it's well within GitHub's rate limits and not worth the complexity, but if candidate volume grows a lot, `github_projects_client`'s `nodes(ids: [ID!]!)` batching pattern (used by `set_field_value_bulk`'s old-value fetch) generalizes: a GraphQL `nodes()` query keyed by PR node IDs could fetch author + merge state for many PRs in one call, cutting the metadata `GET` to near-zero; issue-events (used only to detect a human `unassigned` event) would need a similar `timelineItems` batch to fully close the gap. ## Usage @@ -38,6 +55,8 @@ Neither `mutation_log.py` nor any rule module knows or cares how the log survive uv run foc-mechanical-rules --dry-run # preview, no mutations uv run foc-mechanical-rules # apply uv run foc-mechanical-rules -o "$GITHUB_STEP_SUMMARY" +uv run foc-mechanical-rules --dry-run --rule R-FC-013 # only this rule +uv run foc-mechanical-rules --dry-run --rule R-FC-013 --rule R-PR-001 # or a few ``` Requires a `GITHUB_TOKEN` (or `--token`) with `read:project` (board reads) and issue/PR write access (`repo` scope, or fine-grained `Issues: write` + `Pull requests: write`) on the blessed orgs. CI uses the org's `FILOZZY_CI_ADD_TO_PROJECT` secret (also used by [`add-issues-and-prs-to-fs-project-board.yml`](../.github/workflows/add-issues-and-prs-to-fs-project-board.yml)). diff --git a/foc-mechanical-rules/foc_mechanical_rules/cli.py b/foc-mechanical-rules/foc_mechanical_rules/cli.py index 668c471..e0e0fb1 100644 --- a/foc-mechanical-rules/foc_mechanical_rules/cli.py +++ b/foc-mechanical-rules/foc_mechanical_rules/cli.py @@ -62,6 +62,13 @@ def main() -> None: help="Path to the persisted mutation-history TSV, read at start and " "written at end (default: %(default)s)", ) + parser.add_argument( + "--rule", + action="append", + metavar="RULE_ID", + help="Only run this rule (e.g. R-FC-013). Repeatable to run several. " + "Default: run every registered rule.", + ) args = parser.parse_args() logging.basicConfig( @@ -77,6 +84,19 @@ def main() -> None: session = build_session(token) rules = default_rules() + + if args.rule: + known_ids = {rule.id for rule in rules} + unknown = sorted(set(args.rule) - known_ids) + if unknown: + print( + f"Error: unknown rule id(s): {', '.join(unknown)}. " + f"Known rules: {', '.join(sorted(known_ids))}", + file=sys.stderr, + ) + sys.exit(1) + rules = [rule for rule in rules if rule.id in args.rule] + for rule in rules: rule.org = args.org rule.project_number = args.project_number diff --git a/foc-mechanical-rules/foc_mechanical_rules/registry.py b/foc-mechanical-rules/foc_mechanical_rules/registry.py index a41b8d1..11eac03 100644 --- a/foc-mechanical-rules/foc_mechanical_rules/registry.py +++ b/foc-mechanical-rules/foc_mechanical_rules/registry.py @@ -9,8 +9,8 @@ from .rule import Rule from .rules.assignee import AssigneeRule -from .rules.cycle import CycleRule +from .rules.cycle import CycleRule, PastCycleRule def default_rules() -> List[Rule]: - return [AssigneeRule(), CycleRule()] + return [AssigneeRule(), CycleRule(), PastCycleRule()] diff --git a/foc-mechanical-rules/foc_mechanical_rules/rules/assignee.py b/foc-mechanical-rules/foc_mechanical_rules/rules/assignee.py index c7731f4..727939c 100644 --- a/foc-mechanical-rules/foc_mechanical_rules/rules/assignee.py +++ b/foc-mechanical-rules/foc_mechanical_rules/rules/assignee.py @@ -6,6 +6,9 @@ This module is that rule's canonical implementation — the markdown links back here, and this docstring links back to the markdown, so the two stay in sync instead of drifting apart silently. + +API call pattern: see README.md's "API call pattern per rule" table. If you +change what this rule reads or writes per item, update that table too. """ from __future__ import annotations diff --git a/foc-mechanical-rules/foc_mechanical_rules/rules/cycle.py b/foc-mechanical-rules/foc_mechanical_rules/rules/cycle.py index ce4e928..69525c6 100644 --- a/foc-mechanical-rules/foc_mechanical_rules/rules/cycle.py +++ b/foc-mechanical-rules/foc_mechanical_rules/rules/cycle.py @@ -1,14 +1,18 @@ -"""R-FC-012: recently-active items without a Cycle get the current cycle. +"""R-FC-012 / R-FC-013: keep an item's Cycle pointed at the current iteration. -Canonical English rule: -foc-board-rules/field-completeness.md#r-fc-012-recently-active-items-without-a-cycle-get-the-current-cycle +Canonical English rules: +- foc-board-rules/field-completeness.md#r-fc-012-recently-active-items-without-a-cycle-get-the-current-cycle +- foc-board-rules/field-completeness.md#r-fc-013-open-items-in-a-past-cycle-should-move-to-the-current-cycle This module is that rule's canonical implementation — see rules/assignee.py's docstring for why the markdown and this module link back to each other. -The "don't re-add a cycle a human removed" guard relies on the mutation +The "don't re-add/re-move a cycle a human undid" guards rely on the mutation log (see mutation_log.py and README.md's "Mutation log" section for why that's necessary and how it works) rather than anything GitHub-provided. + +API call pattern: see README.md's "API call pattern per rule" table. If you +change what either rule reads or writes per item, update that table too. """ from __future__ import annotations @@ -31,6 +35,7 @@ ... on ProjectV2IterationField { configuration { iterations { title startDate duration } + completedIterations { title startDate duration } } } } @@ -40,6 +45,28 @@ """ +def _fetch_iterations( + session: requests.Session, *, org: str, project_number: int +) -> List[Dict[str, Any]]: + """Every iteration GitHub knows about for the Cycle field, current/future and completed. + + ``configuration.iterations`` alone only returns the current and future + iterations -- completed ones live under the separate + ``completedIterations`` field and won't appear otherwise. R-FC-012 only + needs the iteration containing today, so this omission didn't matter + there, but R-FC-013 needs the full history to tell a past cycle from an + unknown one, so both iteration lists are combined here. + """ + data = graphql_query( + session, CURRENT_CYCLE_QUERY, {"org": org, "number": project_number} + ) + field = ((data.get("organization") or {}).get("projectV2") or {}).get("field") or {} + configuration = field.get("configuration") or {} + return (configuration.get("iterations") or []) + ( + configuration.get("completedIterations") or [] + ) + + def get_current_cycle_title( session: requests.Session, *, @@ -51,11 +78,7 @@ def get_current_cycle_title( Returns None if today falls in a gap between iterations (no active cycle). """ - data = graphql_query( - session, CURRENT_CYCLE_QUERY, {"org": org, "number": project_number} - ) - field = ((data.get("organization") or {}).get("projectV2") or {}).get("field") or {} - iterations = (field.get("configuration") or {}).get("iterations") or [] + iterations = _fetch_iterations(session, org=org, project_number=project_number) today = today or date.today() for it in iterations: @@ -66,6 +89,33 @@ def get_current_cycle_title( return None +def get_current_and_past_cycle_titles( + session: requests.Session, + *, + org: str, + project_number: int, + today: Optional[date] = None, +) -> tuple[Optional[str], set[str]]: + """Return (current cycle title, set of past cycle titles). + + A cycle is "past" if its date range ended before today. The current + cycle (if any) is never included in the past set, even on a boundary. + """ + iterations = _fetch_iterations(session, org=org, project_number=project_number) + + today = today or date.today() + current: Optional[str] = None + past: set[str] = set() + for it in iterations: + start = date.fromisoformat(it["startDate"]) + end = start + timedelta(days=it["duration"] - 1) + if start <= today <= end: + current = it["title"] + elif end < today: + past.add(it["title"]) + return current, past + + class CycleRule(Rule): id = "R-FC-012" field_name = "cycle" @@ -118,6 +168,7 @@ def apply_one( number = str(item.get("Id", "")) title = item.get("Title", "") item_ref = f"{repository}#{number}" + node_id = item.get("_node_id", "") current_cycle = self._resolve_current_cycle(session) if not current_cycle: @@ -157,7 +208,10 @@ def apply_one( session, org=self.org, project_number=self.project_number, - item_ref=item_ref, + # Node ID, not "owner/repo#number" -- see the matching comment in + # PastCycleRule.apply_one for why (skips set_field_value_bulk's + # per-item get_item lookup). + item_ref=node_id or item_ref, field_name="Cycle", value=current_cycle, ) @@ -176,3 +230,152 @@ def apply_one( old_value=result.get("old_value", ""), new_value=current_cycle, ) + + +class PastCycleRule(Rule): + id = "R-FC-013" + field_name = "cycle" + doc_url = ( + "https://github.com/FilOzone/tpm-utils/blob/master/foc-board-rules/" + "field-completeness.md#r-fc-013-open-items-in-a-past-cycle-should-move-to-the-current-cycle" + ) + + def __init__(self, org: str = FILOZ_ORG, project_number: int = PROJECT_NUMBER): + self.org = org + self.project_number = project_number + self._current_cycle: Optional[str] = None + self._past_cycles: Optional[set] = None + self._resolved = False + + def _resolve(self, session: requests.Session) -> tuple[Optional[str], set]: + if not self._resolved: + self._current_cycle, self._past_cycles = get_current_and_past_cycle_titles( + session, org=self.org, project_number=self.project_number + ) + self._resolved = True + return self._current_cycle, self._past_cycles or set() + + def select(self, session: requests.Session) -> List[Dict[str, Any]]: + items: List[Dict[str, Any]] = [] + cursor: Optional[str] = None + while True: + result = list_items( + session, + org=self.org, + project_number=self.project_number, + query='-status:"🎉 Done" has:cycle', + fields=["Repository", "Id", "Title", "url", "Cycle"], + cursor=cursor, + ) + items.extend(result["items"]) + if not result["has_more"]: + break + cursor = result["next_cursor"] + return items + + def apply_one( + self, + session: requests.Session, + item: Dict[str, Any], + *, + dry_run: bool, + mutation_log: MutationLog, + ) -> ActionResult: + repository = item.get("Repository", "") + number = str(item.get("Id", "")) + title = item.get("Title", "") + item_ref = f"{repository}#{number}" + item_cycle = item.get("Cycle", "") + node_id = item.get("_node_id", "") + + if not repository or not number: + # Draft notes (project items with no linked repo issue/PR) match + # `has:cycle` too but have no repository/number to build a + # mutable item_ref from -- R-FC-013 is scoped to issues and PRs. + return ActionResult( + item_ref=item_ref, + title=title, + status="skipped", + reason="not an issue or PR (likely a draft note) -- no repository/number", + ) + + current_cycle, past_cycles = self._resolve(session) + if not current_cycle: + return ActionResult( + item_ref=item_ref, + title=title, + status="error", + reason="no active cycle for today (gap between iterations)", + ) + + if item_cycle == current_cycle: + return ActionResult( + item_ref=item_ref, + title=title, + status="skipped", + reason="already in the current cycle", + ) + + if item_cycle not in past_cycles: + return ActionResult( + item_ref=item_ref, + title=title, + status="skipped", + reason=f"cycle '{item_cycle}' is not a past iteration (future or unknown)", + ) + + reverted_by_human = any( + m.rule == self.id + and m.field == self.field_name + and m.old_value == item_cycle + for m in mutation_log.for_item(item_ref) + ) + if reverted_by_human: + return ActionResult( + item_ref=item_ref, + title=title, + status="flagged", + reason=( + f"{self.id} previously moved this item off cycle '{item_cycle}' " + "and a human has since moved it back -- not re-applying" + ), + ) + + if dry_run: + return ActionResult( + item_ref=item_ref, + title=title, + status="applied", + reason="dry-run: would move", + old_value=item_cycle, + new_value=current_cycle, + ) + + result = set_field_value( + session, + org=self.org, + project_number=self.project_number, + # Pass the node ID (from select()'s list_items call) instead of + # the "owner/repo#number" ref: set_field_value_bulk skips its + # per-item get_item lookup for a raw node ID and instead + # batch-fetches old_value, which is also more accurate (the API's + # current value, not the value we saw at selection time). + item_ref=node_id or item_ref, + field_name="Cycle", + value=current_cycle, + ) + if not result.get("success"): + return ActionResult( + item_ref=item_ref, + title=title, + status="error", + reason=f"failed to set cycle: {result.get('error')}", + ) + + return ActionResult( + item_ref=item_ref, + title=title, + status="applied", + old_value=result.get("old_value", item_cycle), + new_value=current_cycle, + ) diff --git a/foc-mechanical-rules/tests/test_cli.py b/foc-mechanical-rules/tests/test_cli.py new file mode 100644 index 0000000..703f5bb --- /dev/null +++ b/foc-mechanical-rules/tests/test_cli.py @@ -0,0 +1,63 @@ +"""Unit tests for the --rule CLI flag — mocked GitHub API, no live calls.""" + +from __future__ import annotations + +import sys +from unittest.mock import MagicMock, patch + +import pytest + +from foc_mechanical_rules import cli + + +def _run_cli(argv, run_all_mock): + with ( + patch.object(sys, "argv", ["foc-mechanical-rules", *argv]), + patch("foc_mechanical_rules.cli.build_session"), + patch("foc_mechanical_rules.cli.run_all", run_all_mock), + patch("foc_mechanical_rules.cli.render_summary", return_value=""), + patch("foc_mechanical_rules.cli.read_tsv", return_value=[]), + patch("foc_mechanical_rules.cli.write_tsv"), + ): + cli.main() + + +def test_no_rule_flag_runs_every_registered_rule(capsys): + run_all_mock = MagicMock(return_value=[]) + + _run_cli(["--dry-run", "--token", "x"], run_all_mock) + + ran_ids = {rule.id for rule in run_all_mock.call_args.args[1]} + assert ran_ids == {"R-PR-001", "R-FC-012", "R-FC-013"} + + +def test_rule_flag_filters_to_the_named_rule(capsys): + run_all_mock = MagicMock(return_value=[]) + + _run_cli(["--dry-run", "--token", "x", "--rule", "R-FC-013"], run_all_mock) + + ran_ids = {rule.id for rule in run_all_mock.call_args.args[1]} + assert ran_ids == {"R-FC-013"} + + +def test_rule_flag_is_repeatable(capsys): + run_all_mock = MagicMock(return_value=[]) + + _run_cli( + ["--dry-run", "--token", "x", "--rule", "R-FC-013", "--rule", "R-PR-001"], + run_all_mock, + ) + + ran_ids = {rule.id for rule in run_all_mock.call_args.args[1]} + assert ran_ids == {"R-FC-013", "R-PR-001"} + + +def test_unknown_rule_id_exits_without_running_anything(capsys): + run_all_mock = MagicMock(return_value=[]) + + with pytest.raises(SystemExit) as exc_info: + _run_cli(["--dry-run", "--token", "x", "--rule", "R-NOPE"], run_all_mock) + + assert exc_info.value.code == 1 + run_all_mock.assert_not_called() + assert "R-NOPE" in capsys.readouterr().err diff --git a/foc-mechanical-rules/tests/test_cycle_rule.py b/foc-mechanical-rules/tests/test_cycle_rule.py index 04d53f4..da6d214 100644 --- a/foc-mechanical-rules/tests/test_cycle_rule.py +++ b/foc-mechanical-rules/tests/test_cycle_rule.py @@ -14,6 +14,7 @@ "Id": "458", "Title": "fix: something", "url": "https://github.com/FilOzone/dealbot/pull/458", + "_node_id": "PVTI_abc123", } ITERATIONS = { @@ -76,6 +77,9 @@ def test_item_without_prior_history_gets_current_cycle(mock_get_cycle, mock_set) assert result.status == "applied" assert result.new_value == "202608-2" mock_set.assert_called_once() + # Passes the node ID, not "owner/repo#number" -- skips set_field_value's + # internal per-item get_item lookup. + assert mock_set.call_args.kwargs["item_ref"] == "PVTI_abc123" @patch("foc_mechanical_rules.rules.cycle.set_field_value") diff --git a/foc-mechanical-rules/tests/test_past_cycle_rule.py b/foc-mechanical-rules/tests/test_past_cycle_rule.py new file mode 100644 index 0000000..e68067e --- /dev/null +++ b/foc-mechanical-rules/tests/test_past_cycle_rule.py @@ -0,0 +1,234 @@ +"""Unit tests for R-FC-013 (past cycle) — mocked GitHub API, no live calls.""" + +from __future__ import annotations + +from datetime import date +from unittest.mock import MagicMock, patch + +from foc_mechanical_rules.mutation_log import MutationLog, MutationRecord +from foc_mechanical_rules.rule import Rule +from foc_mechanical_rules.rules.cycle import ( + PastCycleRule, + get_current_and_past_cycle_titles, +) + +ITEM = { + "Repository": "FilOzone/dealbot", + "Id": "458", + "Title": "fix: something", + "url": "https://github.com/FilOzone/dealbot/pull/458", + "Cycle": "202607-2", + "_node_id": "PVTI_abc123", +} + +ITERATIONS = { + "organization": { + "projectV2": { + "field": { + "configuration": { + # GitHub's GraphQL schema splits current/future iterations + # from completed ones -- ``iterations`` alone never + # includes a truly past cycle. Mirror that split here so + # a regression back to reading only ``iterations`` fails + # this test instead of passing by accident. + "iterations": [ + { + "title": "202608-2", + "startDate": "2026-08-17", + "duration": 14, + }, + { + "title": "202608-3", + "startDate": "2026-08-31", + "duration": 14, + }, + ], + "completedIterations": [ + { + "title": "202608-1", + "startDate": "2026-08-03", + "duration": 14, + }, + { + "title": "202607-2", + "startDate": "2026-07-20", + "duration": 14, + }, + ], + } + } + } + } +} + + +def test_get_current_and_past_cycle_titles(): + session = MagicMock() + with patch( + "foc_mechanical_rules.rules.cycle.graphql_query", return_value=ITERATIONS + ): + current, past = get_current_and_past_cycle_titles( + session, org="FilOzone", project_number=14, today=date(2026, 8, 20) + ) + assert current == "202608-2" + assert past == {"202607-2", "202608-1"} + + +@patch("foc_mechanical_rules.rules.cycle.set_field_value") +@patch( + "foc_mechanical_rules.rules.cycle.get_current_and_past_cycle_titles", + return_value=("202608-2", {"202607-2", "202608-1"}), +) +def test_open_item_in_past_cycle_moves_to_current(mock_get, mock_set): + # The API-observed old_value can differ from what select() saw (a human + # could have edited it in between) -- the result should reflect that, + # not the stale value read at selection time. + mock_set.return_value = {"success": True, "old_value": "202607-2"} + + result = PastCycleRule().apply_one( + MagicMock(), ITEM, dry_run=False, mutation_log=MutationLog() + ) + + assert result.status == "applied" + assert result.old_value == "202607-2" + assert result.new_value == "202608-2" + mock_set.assert_called_once() + # Passes the node ID, not "owner/repo#number" -- skips set_field_value's + # internal per-item get_item lookup. + assert mock_set.call_args.kwargs["item_ref"] == "PVTI_abc123" + + +@patch("foc_mechanical_rules.rules.cycle.set_field_value") +@patch( + "foc_mechanical_rules.rules.cycle.get_current_and_past_cycle_titles", + return_value=("202608-2", {"202607-2", "202608-1"}), +) +def test_draft_note_with_no_repository_is_skipped(mock_get, mock_set): + # A draft note (board item with no linked repo issue/PR) matches + # `has:cycle` too but has no Repository/Id to build a mutable ref from. + item = {**ITEM, "Repository": "", "Id": ""} + + result = PastCycleRule().apply_one( + MagicMock(), item, dry_run=False, mutation_log=MutationLog() + ) + + assert result.status == "skipped" + mock_set.assert_not_called() + + +@patch("foc_mechanical_rules.rules.cycle.set_field_value") +@patch( + "foc_mechanical_rules.rules.cycle.get_current_and_past_cycle_titles", + return_value=("202608-2", {"202607-2", "202608-1"}), +) +def test_item_already_in_current_cycle_is_skipped(mock_get, mock_set): + item = {**ITEM, "Cycle": "202608-2"} + + result = PastCycleRule().apply_one( + MagicMock(), item, dry_run=False, mutation_log=MutationLog() + ) + + assert result.status == "skipped" + mock_set.assert_not_called() + + +@patch("foc_mechanical_rules.rules.cycle.set_field_value") +@patch( + "foc_mechanical_rules.rules.cycle.get_current_and_past_cycle_titles", + return_value=("202608-2", {"202607-2", "202608-1"}), +) +def test_item_in_future_cycle_is_skipped(mock_get, mock_set): + item = {**ITEM, "Cycle": "202608-3"} + + result = PastCycleRule().apply_one( + MagicMock(), item, dry_run=False, mutation_log=MutationLog() + ) + + assert result.status == "skipped" + mock_set.assert_not_called() + + +@patch("foc_mechanical_rules.rules.cycle.set_field_value") +@patch( + "foc_mechanical_rules.rules.cycle.get_current_and_past_cycle_titles", + return_value=("202608-2", {"202607-2", "202608-1"}), +) +def test_item_human_reverted_is_flagged_not_reset(mock_get, mock_set): + log = MutationLog( + [ + MutationRecord( + timestamp="2026-08-18T00:00:00+00:00", + rule="R-FC-013", + item="FilOzone/dealbot#458", + field="cycle", + old_value="202607-2", + new_value="202608-2", + ) + ] + ) + + result = PastCycleRule().apply_one( + MagicMock(), ITEM, dry_run=False, mutation_log=log + ) + + assert result.status == "flagged" + mock_set.assert_not_called() + + +@patch("foc_mechanical_rules.rules.cycle.set_field_value") +@patch( + "foc_mechanical_rules.rules.cycle.get_current_and_past_cycle_titles", + return_value=("202608-2", {"202607-2", "202608-1"}), +) +def test_prior_history_for_a_different_cycle_does_not_block(mock_get, mock_set): + # We previously moved this item off a *different* past cycle; that's not + # the reversion signal -- only a prior move off *this exact* cycle counts. + log = MutationLog( + [ + MutationRecord( + timestamp="2026-07-01T00:00:00+00:00", + rule="R-FC-013", + item="FilOzone/dealbot#458", + field="cycle", + old_value="202606-2", + new_value="202607-2", + ) + ] + ) + mock_set.return_value = {"success": True, "old_value": "202607-2"} + + result = PastCycleRule().apply_one( + MagicMock(), ITEM, dry_run=False, mutation_log=log + ) + + assert result.status == "applied" + + +@patch( + "foc_mechanical_rules.rules.cycle.get_current_and_past_cycle_titles", + return_value=(None, set()), +) +def test_no_active_cycle_is_an_error(mock_get): + result = PastCycleRule().apply_one( + MagicMock(), ITEM, dry_run=False, mutation_log=MutationLog() + ) + assert result.status == "error" + + +@patch("foc_mechanical_rules.rules.cycle.set_field_value") +@patch( + "foc_mechanical_rules.rules.cycle.get_current_and_past_cycle_titles", + return_value=("202608-2", {"202607-2", "202608-1"}), +) +def test_dry_run_does_not_mutate(mock_get, mock_set): + result = PastCycleRule().apply_one( + MagicMock(), ITEM, dry_run=True, mutation_log=MutationLog() + ) + + assert result.status == "applied" + assert result.new_value == "202608-2" + mock_set.assert_not_called() + + +def test_past_cycle_rule_is_a_rule(): + assert isinstance(PastCycleRule(), Rule)