diff --git a/.lockstep/lockstep.py b/.lockstep/lockstep.py index 90dd0a0..ed9d0e2 100644 --- a/.lockstep/lockstep.py +++ b/.lockstep/lockstep.py @@ -687,4 +687,11 @@ judge_workflows.register() # `review/all-lenses`: the required check, as one fan-out over every lens the bound adapter # declares (GATE-REVIEW-5, GATE-COST-6). `lockstep.yml` invokes it and nothing else. +# +# No `gating=`, and that is a decision rather than an omission. Since #449 a repository can +# name which of its bound lenses gate a pull request, leaving the rest bound and reachable +# from `/review ` on the thread but off the required check. THIS repository may not: +# O10 exists so the four shipped lenses are not things we ask adopters to trust on our word, +# and `intent`, `performance` and `tests` already spent a release shipped-but-never-run once +# (#241). `GATE-REVIEW-9` fails the build if an argument appears here. review_workflows.register() diff --git a/design/gates.md b/design/gates.md index 1bd3155..eebca78 100644 --- a/design/gates.md +++ b/design/gates.md @@ -242,10 +242,11 @@ one **offers** it, and a line somebody wrote is what puts it in force. | `GATE-REVIEW-2` | none | held | A `/fix` or `/implement` asked for on a pull request runs against the ticket that pull request was opened for, or refuses by name. The number a comment carries is resolved in Python. The trampoline passes it through untouched, so the decision is not an `if:` expression, where being wrong reads exactly like "nobody asked". Resolution reads the record `change_body` wrote and falls back to the branch only when the branch's shape is unambiguous, so it can decline to resolve a real ticket but never resolve the wrong one. A change request that records no ticket raises `NoTicketForChange` naming it, rather than proceeding with a pull request's own number and failing two steps later with a true sentence about the wrong thing. A host that numbers issues and change requests separately is not guessed at: `shared_numbering` is a fact about the host, and where it is false the key is left alone. | | `GATE-REVIEW-3` | none | held | The lens a review is asked for — by a `/review` comment or by `--aspect` at a terminal — is resolved in Python, against the lenses the BOUND adapter has, before any credential is read or any run id exists. A name that is no lens is refused with the set that does exist, reads no key, and appends no ledger record — which matters because the adapter's own refusal arrives after `_run_id`, and `blocked` sits inside `failure_rate`'s denominator, so a stream of typos from anyone who can comment would deflate the repository's own failure rate. The flag took a different road until #275: it reached the adapter, and a typo at the terminal wrote a `blocked` record, the framework's word for a control stopping a run, spent on nothing stopping anything. One function, `chatops.resolve_aspect`, serves both spellings now, so they cannot drift apart again. The two part only on a bound adapter that is not `Inspectable` and so states no set at all: the flag's name passes through to the adapter's own refusal, because a set nobody stated is not one to refuse against and the shipped four would be a guess about a stranger's adapter; a comment is refused by name, because a comment is anyone's and the closed set is the whole promise. The closed set kept an aspect out of `report.marker`, which builds an HTML comment by interpolation, and since #275 the marker escapes its argument as well — a `-->` inside the kind closed the comment early and orphaned the anchor the next run finds its own comment by — because a defence that is only an ordering is one reordering away from gone. The escape is injective and leaves every shipped kind byte-identical, and its pattern is defined beside the writer and imported by `comment`, whose own copy excluded `-` and refused the body a hyphenated lens had written as unanchored. Resolution cannot be delegated to the trampoline in any case: GitHub's expression language has twelve functions and none of them splits a string. | | `GATE-REVIEW-4` | none | held | A finding names coordinates the change actually touched, or they are refused rather than reported. `_in_the_change` checks each finding's `path` against `Diff.paths` and its `line` against `Diff.hunks`, over the diff **as sent to the model** — not a fresh `git diff`, because a model cannot name a file it was not shown, so the context is the tighter test and a review that reached the model must not then fail on a ref resolving. The two halves are refused differently, and the difference is the argument: a path the change does not touch drops the **finding**, because there is nothing there to say anything about; a line outside every hunk drops only the **line**, because a model that noticed something real about the file and pointed one line off is still worth reading. Per finding, so one bad path does not discard three real findings beside it, and both are **counted** — `review.path_not_in_diff` and `review.line_not_in_hunk` name what was refused, because a lens that quietly drops half its output reads as a lens that found half as much. Both sides of the diff count as touched: a deleted file appears only under `--- a/`, and deleting one is a fine thing to have an opinion about. Coordinates are never rewritten to make them match — `./x` is normalised, `a/x` is not, since `a/` is a legal directory name and rewriting until something matches is the guessing O1 refuses next door. Three places fail open rather than refuse, each tested: a diff with no parsable path (a pure mode change), a file with no new side (a deletion), and a finding claiming no line at all — in each, *cannot be checked* is a different fact from *is wrong*, and enforcing the rule on input that cannot answer it would drop real output. Closed by #234. | -| `GATE-REVIEW-5` | none | held | The required review check exercises every lens the BOUND adapter declares. `review` is a required status check on `main`, so every pull request here was already gated on a model reading the diff -- through one lens of four. `intent`, `performance` and `tests` shipped, composed, and sat in the characterization corpus without ever having run on a real pull request in the repository whose whole argument is that it runs on itself, which is O10's definition of asking adopters to trust us on our word. It bounded O5 as well: this check records and harvests, so a single-lens check made a single-lens corpus and #163's loop would have had evidence about a quarter of what ships. Compared against `_review_lenses` over this repository's loaded module rather than against a list of four written in the test, because a gate that hardcodes the shipped set goes stale the moment somebody adds a fifth -- which is the failure it exists to prevent one layer down, and is asserted by adding one. Both directions: a bound lens the loop never runs, and a name in the loop no adapter declares, the second because `GATE-REVIEW-3`'s refusal is loud but arrives after the job has installed the provider extra and minted a credential. **Since Phase 5 (PR-13)** the check is `review/all-lenses`, a shipped workflow whose branches are declared from the bound adapter's `compositions()` and fanned out under one budget, one tape and one kill switch (`GATE-COST-6`, `GATE-ASYNC-3b`), so `lockstep.yml` can name no lens at all and the list that could go stale is gone. The file flipped one merge after the module registered the workflow, not in the same pull request: configuration loads from the trusted ref while a pull request's workflow file comes from the merge ref, so the pull request that registered it ran a check naming a workflow the base branch's module did not know (run 34171482674). The gate is asserted over the workflow with a stub adapter declaring five lenses, one of them nothing the framework ships, in both directions, and over the file in whichever spelling it carries: the legacy step must list every bound lens, the fan-out step must list none. Four lenses are defensible on a REQUIRED check because findings are non-blocking warnings -- a lens that finds something still exits 0, so nothing here turns a note about performance into a merge block. Scoped to this repository and not to what `init` scaffolds: exercising every lens is a decision about money, and quadrupling a stranger's first bill is not ours to make. The scaffold names the other three in a comment instead. Filed as #241. | +| `GATE-REVIEW-5` | none | held | The required review check exercises every lens the BOUND adapter declares. `review` is a required status check on `main`, so every pull request here was already gated on a model reading the diff -- through one lens of four. `intent`, `performance` and `tests` shipped, composed, and sat in the characterization corpus without ever having run on a real pull request in the repository whose whole argument is that it runs on itself, which is O10's definition of asking adopters to trust us on our word. It bounded O5 as well: this check records and harvests, so a single-lens check made a single-lens corpus and #163's loop would have had evidence about a quarter of what ships. Compared against `_review_lenses` over this repository's loaded module rather than against a list of four written in the test, because a gate that hardcodes the shipped set goes stale the moment somebody adds a fifth -- which is the failure it exists to prevent one layer down, and is asserted by adding one. Both directions: a bound lens the loop never runs, and a name in the loop no adapter declares, the second because `GATE-REVIEW-3`'s refusal is loud but arrives after the job has installed the provider extra and minted a credential. **Since Phase 5 (PR-13)** the check is `review/all-lenses`, a shipped workflow whose branches are declared from the bound adapter's `compositions()` and fanned out under one budget, one tape and one kill switch (`GATE-COST-6`, `GATE-ASYNC-3b`), so `lockstep.yml` can name no lens at all and the list that could go stale is gone. The file flipped one merge after the module registered the workflow, not in the same pull request: configuration loads from the trusted ref while a pull request's workflow file comes from the merge ref, so the pull request that registered it ran a check naming a workflow the base branch's module did not know (run 34171482674). The gate is asserted over the workflow with a stub adapter declaring five lenses, one of them nothing the framework ships, in both directions, and over the file in whichever spelling it carries: the legacy step must list every bound lens, the fan-out step must list none. Four lenses are defensible on a REQUIRED check because findings are non-blocking warnings -- a lens that finds something still exits 0, so nothing here turns a note about performance into a merge block. Scoped to this repository and not to what `init` scaffolds: exercising every lens is a decision about money, and quadrupling a stranger's first bill is not ours to make. The scaffold names the other three in a comment instead. Filed as #241. **Since #449** the claim is scoped, because it had quietly been two: the check exercises every lens the bound adapter declares UNLESS the module narrows it with `review_workflows.register(gating=...)`, and what holds without qualification is that the check runs exactly the gating set and that the set defaults to all of them. Here nothing narrows it and nothing may, which is `GATE-REVIEW-9`'s second half rather than this row's -- and this row's own tests still read the bound adapter, in both directions, so a lens added to the framework is still exercised the day it lands. | | `GATE-REVIEW-6` | none | held | Every lens the required check runs says what it found **on the pull request**, as its own sticky comment. Four lenses had read every pull request here since #241 and said nothing on any of them: their verdicts and findings went to the job log, so the reader saw a green check and none of what a model had been paid to say -- and a later `/fix` on the ticket, which gathers what was said on the change request, gathered none of it either. `review` with several `--aspect`s and a `--comment-out` directory writes `.md` per lens, each anchored by its own `review:` marker, and refuses a path that says it is one file rather than overwriting it once per lens; the artifact carries the directory; and `comment --body-file ` posts every body in it under the marker inside it, from the `publish` job -- write token, no provider credential, the split `GATE-LEDGER-10` made -- checking every body for its marker before posting any. One comment per lens rather than one wall, because a reader telling `security` from `tests` should not have to scroll a table, and because a re-review then edits four comments in place. Asserted at the command (bodies, markers, the refusal, the all-or-nothing post) and over this repository's workflow and both scaffolds. `/review ` on request already posted this way through `lockstep-review.yml`; the required check now says as much as the lens somebody asked for. Closed by #345. | | `GATE-REVIEW-7` | none | held | A lens says what it made of the change, not only what it found wrong. On a change it had no findings about, a lens posted four characters -- `No findings.` -- which is a reviewer saying nothing and is indistinguishable from a reviewer that did not look. Both of #442's clean lenses rendered exactly that, at 48 and 111 output tokens: a real spend producing a sentence anybody could have written without reading the diff, on a pull request whose other two lenses found a real defect. **The path was half-built and entirely dead.** `REVIEW_SCHEMA` carried an OPTIONAL `verdict`, `review.py` parsed it onto `ReviewReport`, and the shared format skill asked for it in prose -- and nothing rendered it anywhere, so the model was asked for a judgement, sometimes gave one, and the framework discarded it. The shipped recording carrying none is the evidence that optional means absent. Third dead vocabulary found this way, after `ConfigRef.under_review` and `_CONTAINED`. **Required now, and renamed.** Required so structured output enforces it and a lens returning none is a `review.schema_mismatch` the adapter already refuses; `statement` rather than `verdict` because `verdict` means a pass/fail judgement everywhere else here (`TestVerdict`, `read_verdict`, `CriterionVerdict`) and a review has none to give -- findings are non-blocking by design, which is what makes four lenses defensible on a required check. **It rides as a NOTE finding, which is what puts it on the RECORD.** A step's record carries its findings and not its outcome's value, so a statement kept on the report would reach the pull-request comment and nothing else -- invisible to `report`, to `history --explain`, and to the improvement loop that reads recorded inferences to propose better prompts (O4, O5). Never blocking, bounded at `MAX_STATEMENT_CHARS` for the comment and at `Finding.MAX_RECORDED_MESSAGE` for the record, and partitioned out of the findings table like the injection signals beside it, because a paragraph in a column headed *finding* reads as one. Rendered between the verdict-and-cost line and the table, so a reader meets what the lens concluded before the list of what it objected to. **Cost:** requiring it changed the composed prompt, which invalidated the shipped review fixture -- cassettes are keyed on the whole composed prompt, and re-recording is a real model call. That was a person's decision, taken deliberately, and the re-recorded fixture is what ships. Discharged by `test_gate_review_7_a_clean_lens_says_what_it_concluded`, `test_gate_review_7_the_statement_sits_between_the_cost_line_and_the_findings`, `test_gate_review_7_the_schema_requires_a_statement` and `test_gate_review_7_a_statement_is_a_non_blocking_note`. | | `GATE-REVIEW-8` | none | held | A second `/implement` on a ticket that already has an open change request **this verb** opened force-pushes to that branch rather than opening a second pull request; `changes_for` matches the ticket segment and ignores the workflow one, so a `/fix` run's request for the same ticket is in its answer and is not implement's to write over. The changeset is built over HEAD, not the branch tip — the ordering property in `prepared` is not weakened (#422) — asserted on the remote against the prior attempt's own file, because the first spelling of that assertion compared two constants (`base` was never passed, so `kwargs.get("base", "") != branch` was `"" != "in-lockstep/…"`) and passed against an implementation deliberately rewritten to build over the tip. A branch carrying any commit without an `In-Lockstep-Run` trailer is refused before the push, naming the commits and saying what to do; **and a branch that cannot be READ is refused too**, because *could not tell* is not *nobody wrote anything here* and three separate paths spelled them the same — `hasattr(scm, "commits_between")` on an adapter where that method does not live (it is `GitLocal`'s, and the hosts proxy only `diff`, so the refusal was dead on both), a `HEAD..` range in a checkout that never fetched the branch, and `GitLocal.git` without `check=True` answering `""` on failure. The branch is fetched once and the sha that read returns is the sha the push is **leased** on (`--force-with-lease=:`, required rather than defaulted): the control and the write name one state, and a bare `--force-with-lease` is not the weaker version of that but a push git rejects outright as `stale info`, since the job that runs it has no remote-tracking ref. That lease is also what replaces the branch name's run id as the concurrency guarantee — `lockstep-implement.yml` groups on the number the comment was left on, so a round asked for on the issue and one asked for on the pull request are different groups and overlap, which was safe only while every run had its own branch. An update whose tests did not pass puts the change request back into **draft**: only this path can meet one that is already ready, and leaving a red change in a review queue while the comment calls it a draft is a false statement where a person reads it -- and `mark_draft` does not double the `Draft:` prefix, because the two constructors of a `ChangeRequest` disagree about whether a title carries one. The refusal's prose carries no git stderr: it is posted publicly on the ticket, and git echoes the remote URL, which IS a credential on a remote spelled `https://@host/...`; git's text goes to the job log, and `raise ... from e` keeps it for the traceback. When no open change request exists, the flow is unchanged; two open ones, the newest is updated and the comment names which. Asserted on BOTH hosts over real git, because `update_change` is two implementations of one property and O3 asks for the same process wherever a repository lives -- which is how GitLab's clamping of the commit subject as well as the title, so an update dropped a body the first attempt kept, was found. Filed as #443; the second half found reviewing the change that closed it. Discharged by `test_a_second_attempt_updates_the_existing_change_request`, `test_a_branch_with_a_persons_commit_is_refused_before_the_push`, `test_a_persons_commit_is_found_through_the_adapter_the_workflow_is_handed`, `test_a_branch_that_cannot_be_read_is_refused_rather_than_assumed_clean`, `test_the_changeset_is_built_over_head_not_the_branch_tip`, `test_the_sha_the_check_read_is_the_sha_the_push_is_leased_on`, `test_a_branch_that_moved_since_it_was_read_refuses_the_push_by_name`, `test_another_verbs_change_request_is_not_the_one_updated`, `test_a_host_that_cannot_list_its_changes_opens_a_new_one`, `test_the_refusal_a_person_reads_does_not_carry_git_stderr`, `test_mark_draft_puts_the_prefix_back_and_does_not_double_it`, `test_the_gitlab_adapter_updates_over_head_and_on_a_lease_too`, `test_an_update_keeps_the_commit_body_a_title_cannot_carry`, `test_a_red_second_attempt_puts_the_change_request_back_into_draft` and `test_no_existing_cr_opens_a_new_one_as_before`. | +| `GATE-REVIEW-9` | none | held | Which lenses a repository HAS and which lenses GATE a pull request are two decisions, and one map used to answer both. `AiReview(lenses=...)` is the adapter's declaration and is what `chatops.aspect_from` resolves `/review ` against (deliberately: a repository that replaced its lenses gets its own set, and one that added a lens can name it). So the only lever for keeping a lens off the required check was dropping it from that map -- which also put it out of reach of the comment asking for it, and the ordinary want, five lenses available and two gating, was not expressible. `review_workflows.register(gating=...)` is the second lever, and it is on the registration because it is a property of the CHECK: `lenses=` reads better at the call site and is the worse place semantically, since a second check over one adapter could not then choose differently. Not a `--arg`: `GATE-REVIEW-5` closed the question of a lens list in YAML, and `register` re-declares the workflow's signature rather than using `functools.wraps` for that exact reason -- `wraps` sets `__wrapped__`, `inspect.signature` follows it, and `in-lockstep run` builds its `--arg` surface by introspecting the registered callable, so a wrapped `gating` parameter would have been the trampoline lens list coming back in through the signature. Asserted structurally, not by reading the file. `functools.partial` is the other obvious spelling and is worse in a third way: a partial carries no `__qualname__`, and `_same_declaration` compares module and qualname, so a host that loads a `lockstep.py` twice in one process -- a long-lived worker, a test harness, two CLI invocations sharing an interpreter -- would get `DuplicateWorkflow` against itself on the second load. Re-registration is asserted in both directions, the no-argument call after a narrowing one first, because a selection that could not be taken back off is the worse failure. A non-gating lens does not run at all (by decision: `lenses=` is already the lever for what exists, so two levers answering "does this run" would be the confusion this row exists to remove) and is still nameable on the thread, which is the property the obvious implementation breaks and therefore has its own test from the chat-ops side. A gating name the bound adapter does not declare refuses the run by name, listing the unknown lens and the set that exists, with zero model calls -- resolved at run time rather than at registration, because a module may call `register` before it binds `Review` and there is no container yet; run time is still before the fan-out, which is the property that matters. `gating=()` is refused separately rather than read as "all of them": absent and empty are opposite intentions and the silent reading of the second is a required check that can never fail. Both refusals are `blocked`, which exits 3, so a misconfigured selection is a red check rather than a green check over nothing. **Here, all of them gate, by decision and not by default**: O10 exists so the shipped lenses are not things we ask adopters to trust on our word, and `intent`, `performance` and `tests` already spent a release shipped-but-never-run once (#241). Asserted by running the workflow this repository's own module registered over a stub declaring five lenses and requiring five branches, so a selection of any kind appearing in `.lockstep/lockstep.py` fails the build. Filed as #449. | | `GATE-DOGFOOD-1` | none | held | The loop has closed on this repository, and the published ledger and the host say so. O10 said "it runs on itself" while the framework had never opened a pull request here (#312): eight implement and fix runs cost $275 and the two whose work half succeeded died at `propose`. Nine `/fix` runs on #319 later (#333 to #342 each fixed what the previous run surfaced: a redaction that wrote `***` into a test, an image without git, a container without the venv, a capability-dropped user who could not write `.ruff_cache`, a uid no `getpwuid` knew, a session that scripted string replacements for want of an edit tool, a session that fixed inside its reproduce step), run 34158960476 staged a reproducer and a fix, proposed #343, and #343 merged on 2026-09-07 -- the first framework-authored change in this repository's history. `lockstep-improve.yml` ran twice (34069517686 dispatched, 34121918174 scheduled), each blocked by name, `improve.no_trend` then `improve.nothing_to_improve`, which is the loop refusing to propose on no evidence. Asserted by reading `origin/lockstep-history` for a succeeded fix or implement record and an improve record, and by asking the host for a merged pull request on a run branch; each half skips with a named reason where its source is out of reach, and `ci.yml` checks out every branch and hands `gh` a read-only token so on CI it runs. **The chat-ops clause closed on 2026-09-07:** `/review security` on #346 (run 34163733667) was the first comment that ever matched `lockstep-review.yml`'s gate, and it failed one step in -- this repository's file handed its reviewing step no `GH_TOKEN` while the scaffold did, and `change_refs` read "could not ask" as "not a pull request" (#347) -- and the second, on #348 (run 34167168362), ran gate, review and post through to the sticky comment. Asserted by asking the host for a succeeded `lockstep-review.yml` run. What that run showed next: its record was uploaded as `review-`, a name the sweep never lists, so it would have expired unabsorbed; every workflow that bundles a record now uploads it under the one name the sweep lists, and a test holds every file and scaffold to it. Two smaller facts the same issue found are closed beside it: a run id only a fixture mints (`triage-`, `-local`) is refused by every path that publishes -- push, reconcile, absorb -- naming it, while the four that already leaked stay pullable until the acknowledged rewrite that removes them, which is a person's act; and `doctor` warns (`DOC127`, `DOC128`) when `enforce_admins` is off or a bypass actor sits on the ruleset that requires the review check, because a `required` check one engineer can step around is enforced by choice, and here it is. Filed as #312. | | `GATE-LENS-1` | none | held | A review lens is a declared object keyed on the name it already has. `Lens` carries the prompt, an emphasis, a layer stack and its ceilings, and `review/` is a route key that wins over `review`; `AiReview` composes each lens from its own declaration, so `show-prompt` renders the emphasis and the stack a run would send, `ls` prints one guardrail chain per distinct stack rather than the first lens's as everybody's, and a route to a lens nothing binds is flagged beside it the way a route to a verb nothing serves already was. The key is the whole design and not a convenience: `review.security` is the finding id, the sticky-comment marker, the `Improvable` label and the census key at once, and an enhancement that renamed the lens would fork all four -- which is why #204's design panel refused a verb per lens (dispatch keys on the request type, so four verbs are four request types, four adapters and four binds, and the same again for every extender). A lens differs in what it **says**; a verb differs in what it **may do**, and `docs/extending.md` draws the line. Ceilings move one way each, and the reason is stated at the type: `max_turns` may only tighten, because the adapter's cap is already `min(its own, the policy floor)` and the floor is not recoverable from the result; `max_tokens` replaces, because `Policy` deliberately carries no floor for it. `Pack.emphasis` is the no-subclass form for a pack, and the bare prompt class stays a legal entry, so nothing written before this row changes meaning. Closed by #204. | | `GATE-EVAL-3` | none | held | A recording can always find itself — given the request as it was SENT, or as it was STORED. **The original wording said one thing and the fixture proved a weaker one.** It claimed a stored request hashes back to the key it is filed under; it does not, and cannot: the filing key hashes the raw request because a live lookup holds a raw request, while the entry holds `redact.text(...)` of it because a file on disk must not carry a credential. Both are right and the two hashes are simply different numbers. The assertion survived because every fixture recorded through a `Redact` with nothing registered and no structural pattern in the text, so the redactor was a no-op and the property could not fail however wrong it was — the same shape as `GATE-RECORD-1` clause (4), one file over. `Cassette.as_stored` is the index that reconciles them, derived at read time so no tape migrates, and `replay_provider` consults it only after a miss. Asserted in both directions, over a fixture that masks something: neutralising the fallback turns the stored lookup red while the live lookup stays green, and the test refuses a fixture with nothing to redact rather than passing over it. The older test that carried this gate's name while asserting the weaker property over a fixture that redacted nothing was renamed for what it proves at PR-20. Everything harvesting does rests on this: a stored request that could not be found produced cases that build, replay, miss, and report as unplayable for a reason nobody could act on — a code fault wearing the costume of a data problem. Harvest refuses rather than guesses on a recording made before requests were kept, invents no rubric, and never writes: a case holds a whole request and a whole answer, so the write is a redaction sink and belongs to a layer that may reach `privileged`. | diff --git a/design/objectives.md b/design/objectives.md index 049a723..51a1f44 100644 --- a/design/objectives.md +++ b/design/objectives.md @@ -94,8 +94,8 @@ to move without somebody standing in front of this table. | `O5` | The record is what teaches it | held | `GATE-IMPROVE-1`, `GATE-IMPROVE-2`, `GATE-IMPROVE-3`, `GATE-IMPROVE-4`, `GATE-IMPROVE-5`, `GATE-IMPROVE-6`, `GATE-IMPROVE-7`, `GATE-IMPROVE-8`, `GATE-EVAL-2`, `GATE-EVAL-4`, `GATE-EVIDENCE-1`, `GATE-OUT-2`, `GATE-LEDGER-12`, `GATE-JUDGE-1`, `GATE-JUDGE-3`, `GATE-LEDGER-2`, `GATE-LEDGER-4`, `GATE-EVAL-1`, `GATE-EVAL-3`, `GATE-REVIEW-7` | — | The reading half, the writing half and, since PR-18, the half that reads back: `improve` reads the ledger for a qualifying trend, drafts a change to the one declared body that trend is attributed to, measures the draft against the promoted corpus on both arms -- the bound judge settling the rubrics, a verdict kept beside its case -- opens the change with its scorecard, and `report --around` compares the runs after the merge with the runs before it on that body's subject (`GATE-LEDGER-2`). What this repository's own ledger can show of that last step was thin when the row moved -- the Phase 4 merge had two subject-carrying runs after it, because the required check's fan-out records carried its lenses as steps without a subject -- and the fix that followed stamps each lens step with its subject and its bill, so every check since is four comparable runs; the records before it stay thin and the window says so. On this repository the corpus is one promoted case with one kept verdict, so the loop refuses before spending until somebody tightens a case to what a correct answer would have said -- the labelling act it learns from. The reason the first dispatched `improve` run actually gives belongs in this row once it has run. | | `O6` | The model never holds a secret | held | `GATE-AUTH-1`, `GATE-AUTH-2`, `GATE-SANDBOX-1`, `GATE-SANDBOX-2`, `GATE-EGRESS-1`, `GATE-EGRESS-2`, `GATE-EGRESS-3`, `GATE-REDACT-1`, `GATE-REDACT-2`, `GATE-REDACT-3`, `GATE-GUARD-4`, `GATE-CFG-1`, `GATE-POLICY-2`, `GATE-CFG-2`, `GATE-RETRY-6`, `GATE-APPROVAL-1`, `GATE-GUARD-1`, `GATE-GUARD-2`, `GATE-DELEGATE-1`, `GATE-SEARCH-1`, `GATE-CFG-5` | — | The container a model-staged test needs is one whose image carries the suite's dependencies, and `init` names that image rather than deriving it, so an adopter's first `implement` or `fix` refuses `sandbox.host_fallback` until a person writes the line (`GATE-SANDBOX-2` says why a stack image will not do). This repository's own container mounts the `.venv` its runner built, which runs the suite wherever its packages import on linux -- everywhere, while they stay pure Python -- and fails at import, naming the module, the day one does not. Egress for the run as a whole stays opted out here and in the scaffold (`GATE-EGRESS-2`, #315). | | `O7` | Determinism first | held | `GATE-REVIEW-2`, `GATE-REVIEW-3`, `GATE-REVIEW-4`, `GATE-EVAL-4`, `GATE-COST-3`, `GATE-SHAPE-1`, `GATE-VERDICT-1`, `GATE-PROGRESS-1`, `GATE-OUT-3`, `GATE-JUDGE-2`, `GATE-WORKSPACE-1`, `GATE-VALIDATE-2`, `GATE-ASSESS-1` | — | — | -| `O8` | Extended without forking | held | `GATE-PACK-1`, `GATE-PACK-2`, `GATE-PACK-3`, `GATE-PACK-4`, `GATE-PACK-5`, `GATE-PLUGIN-1`, `GATE-PLUGIN-2`, `GATE-PLUGIN-3`, `GATE-DOCS-1`, `GATE-BODY-1`, `GATE-POLICY-1` | — | — | -| `O9` | New aspects on a verb that already exists | held | `GATE-REVIEW-3`, `GATE-PACK-5`, `GATE-REVIEW-5`, `GATE-LENS-1`, `GATE-BLAST-1` | — | — | +| `O8` | Extended without forking | held | `GATE-PACK-1`, `GATE-PACK-2`, `GATE-PACK-3`, `GATE-PACK-4`, `GATE-PACK-5`, `GATE-PLUGIN-1`, `GATE-PLUGIN-2`, `GATE-PLUGIN-3`, `GATE-DOCS-1`, `GATE-BODY-1`, `GATE-POLICY-1`, `GATE-REVIEW-9` | — | — | +| `O9` | New aspects on a verb that already exists | held | `GATE-REVIEW-3`, `GATE-PACK-5`, `GATE-REVIEW-5`, `GATE-REVIEW-9`, `GATE-LENS-1`, `GATE-BLAST-1` | — | — | | `O10` | It runs on itself | held | `GATE-CI-1`, `GATE-RECORD-1`, `GATE-TEST-3`, `GATE-REVIEW-5`, `GATE-CFG-3`, `GATE-CI-3`, `GATE-DOGFOOD-1`, `GATE-LEDGER-10`, `GATE-CI-5`, `GATE-VALIDATE-1` | — | Reviews on every pull request, a review asked for on a thread, a fix and measurement have closed here (`GATE-DOGFOOD-1` names the runs); an implementation has not, and neither has delegation: `.lockstep/lockstep.py` binds `TDD(delegation=True)`, so the next `/implement` here holds `delegate` (`GATE-DELEGATE-1`), and no run has used it yet. Deliberately not dogfooded on this repository's own runs, and each a separate decision: egress enforcement and the daily ceiling (off, documented), GitLab (no instance; O3 says so), and any provider but Anthropic (`GATE-COST-3` prices it; nothing here has routed to it). The required `review` check is enforced by choice -- `doctor` now says so (`DOC127`, `DOC128`) -- and stays that way while one engineer is the administrator. | | `O11` | The provider and the model are the adopter's, not ours | held | `GATE-AUTH-2`, `GATE-RESIDENCY-1`, `GATE-COST-4`, `GATE-LENS-1`, `GATE-MODEL-1`, `GATE-JUDGE-2`, `GATE-COST-3`, `GATE-ASSESS-2` | — | — | | `O12` | A second engineer is served, not obstructed | held | `GATE-TEAM-1`, `GATE-TEAM-2`, `GATE-LEDGER-10`, `GATE-LEDGER-11`, `GATE-LEDGER-12`, `GATE-REVIEW-6`, `GATE-OUT-6`, `GATE-LEDGER-1`, `GATE-LEDGER-3`, `GATE-LEDGER-7`, `GATE-LEDGER-8`, `GATE-LEDGER-9`, `GATE-REVIEW-1`, `GATE-VERDICT-2`, `GATE-RECORD-6`, `GATE-FORK-1`, `GATE-REPORT-1`, `GATE-CONTEXT-1`, `GATE-CFG-4`, `GATE-REVIEW-8` | — | — | diff --git a/docs/extending.md b/docs/extending.md index ada7860..2cb5d9d 100644 --- a/docs/extending.md +++ b/docs/extending.md @@ -159,6 +159,34 @@ aspect then reports the lenses *this adapter* has, not the ones that happen to s The map is copied at construction, in both directions: a later mutation of `LENSES` cannot reach an adapter you already bound, and an adapter cannot leak a lens back into the shipped map. +### Which of them gate a pull request + +That map says which lenses this repository **has**, and `/review ` on a thread resolves +against exactly it — so narrowing it to keep a lens off the required check would also put that lens +out of reach of the comment asking for it. Which lenses **gate** is a different decision, and it +sits on the registration of the check rather than on the adapter, because that is what it is a +property of: + +```python +from in_lockstep.workflows import review as review_workflows + +review_workflows.register(gating=("security", "tests")) +``` + +Five lenses bound, two named: the required check runs those two and pays for those two, and the +other three are still one `/review performance` away on any thread. Omit the argument — as this +repository does, deliberately — and every bound lens gates. + +A name the bound adapter does not declare refuses the run, listing it beside the set that exists, +before the fan-out and therefore before anything spends. `gating=()` is refused separately, because +"nothing gates" and "everything gates" are opposite intentions and the second is already what an +omitted argument means. Both refusals exit 3, so a selection with a typo in it is a red check +rather than a green one over nothing. + +There is deliberately no flag for this. The selection is executable configuration in the one file +you are meant to edit, never an argument in a workflow file: a lens list in YAML is precisely the +thing that goes stale, which is why the check stopped taking one. + ### Enhancing a shipped lens, without a subclass An entry in that map is either a prompt class or a `Lens`: the declared form, carrying the prompt diff --git a/src/in_lockstep/workflows/review.py b/src/in_lockstep/workflows/review.py index 1235ad9..26a80d7 100644 --- a/src/in_lockstep/workflows/review.py +++ b/src/in_lockstep/workflows/review.py @@ -1,4 +1,4 @@ -"""The required review check as one run: every lens the bound adapter declares, fanned out. +"""The required review check as one run: the lenses that gate a pull request, fanned out. `review/all-lenses` is what `lockstep.yml` invokes on every pull request here. Until Phase 5 the check was one `review` invocation looping four `--aspect` flags in sequence, and the file named @@ -9,12 +9,25 @@ branch's findings are the run's findings, and one record with four steps is written where four records were. +Since #449 those branches are the lenses that GATE, which is not the same set as the lenses that +EXIST. `AiReview(lenses=...)` answers the second and is what `/review ` on a thread resolves +against; `register(gating=...)` answers the first. One map used to answer both, so the only lever +for keeping a lens off the required check was dropping it -- which also put it out of reach of the +comment asking for it. The two are orthogonal now: a lens bound but not gating is available on the +thread and not run by the check. + +Here all four gate, by decision and not by default. O10 exists so the shipped lenses are not +things we ask adopters to trust on our word, and a subset in THIS repository would be that defect +returning (`GATE-REVIEW-9`). + A lens's comment body is still one file per lens under the directory the trampoline names, so -the `publish` job posts four sticky comments a reader can tell apart (`GATE-REVIEW-6`). +the `publish` job posts one sticky comment per gating lens a reader can tell apart +(`GATE-REVIEW-6`). """ from __future__ import annotations +from collections.abc import Collection from typing import Any from ..adapters.ai.review import Review @@ -39,32 +52,81 @@ def bound_lenses(ctx: RunContext) -> tuple[str, ...]: return tuple(sorted(str(label).rsplit("/", 1)[-1] for label in compositions())) +def _named(names: Collection[str]) -> str: + return ", ".join(sorted(names)) + + +def _refused(reason: str, message: str) -> Outcome[Any]: + """A configuration this check will not guess at, as the run's own outcome. + + `blocked` rather than `failed`, because nothing ran and nothing spent and that is §4.3's own + category. It is still red where it matters: `EXIT_BLOCKED` is 3, so a required check whose + lens selection is wrong fails rather than reading as a green check that reviewed nothing. + """ + return Outcome.blocked_by( + reason, + findings=(Finding(id=reason, message=message, severity=Severity.ERROR, blocking=True),), + ) + + async def review_all_lenses( - ctx: RunContext, base: str, head: str, comments: str = "", diff: str = "" + ctx: RunContext, + base: str, + head: str, + comments: str = "", + diff: str = "", + *, + gating: Collection[str] | None = None, ) -> Outcome[Any]: - """Every lens over one change, at once, as one run. + """Every lens that gates this change, at once, as one run. `comments` is a directory: one body per lens lands in it for the job that holds the write token to post. `diff` is for a replay whose tape was recorded against a diff git no longer has (the shipped fixture), and is otherwise empty. + + `gating` comes from `register(gating=...)` and from nowhere else. Keyword-only, and absent + from the callable that `register` actually registers, because `in-lockstep run` builds its + `--arg` surface by introspecting that callable's signature -- and a lens list reachable from + the trampoline is the question `GATE-REVIEW-5` closed. """ - lenses = bound_lenses(ctx) - if not lenses: - return Outcome.blocked_by( + declared = bound_lenses(ctx) + if not declared: + return _refused( "review.no_lenses", - findings=( - Finding( - id="review.no_lenses", - message=( - "no Review adapter declaring its lenses is bound; nothing was reviewed. " - "`in-lockstep run` binds the shipped AiReview when a module binds none, so " - "this means a module bound an adapter with no `compositions()`." - ), - severity=Severity.ERROR, - blocking=True, - ), - ), + "no Review adapter declaring its lenses is bound; nothing was reviewed. " + "`in-lockstep run` binds the shipped AiReview when a module binds none, so this " + "means a module bound an adapter with no `compositions()`.", ) + + lenses = declared + if gating is not None: + # Resolved against what the adapter declares rather than trusted, and resolved HERE -- + # after the binding exists, before the fan-out. `register` cannot do it: a module is free + # to call it before it binds `Review`, and there is no container at registration time. + asked = tuple(dict.fromkeys(gating)) + if not asked: + return _refused( + "review.gating_empty", + "`register(gating=...)` names no lens at all, so nothing would gate and this " + "check could not fail for any reason. Omit the argument to gate on every bound " + f"lens ({_named(declared)}), or name the ones that should.", + ) + unknown = tuple(name for name in asked if name not in declared) + if unknown: + return _refused( + "review.gating_unknown", + f"`register(gating=...)` names {_named(unknown)}, which the bound Review adapter " + f"does not declare. It declares {_named(declared)}. Refused rather than dropped: " + "a typo that quietly gates on fewer lenses than somebody meant is a check that " + "goes green for the wrong reason, which is the one failure a required check must " + "not have.", + ) + lenses = tuple(name for name in declared if name in set(asked)) + # Said out loud on every run, because the difference between the bound set and the gating + # set is invisible in the check's output otherwise -- and a reader looking at four sticky + # comments where five lenses are bound should not have to open `lockstep.py` to know why. + print(f"gating {len(lenses)} of {len(declared)} bound lenses: {_named(lenses)}") + join = await ctx.fan_out( branches={ lens: ctx.call(Review(base=base, head=head, aspect=lens, diff=diff), step=lens) for lens in lenses @@ -81,9 +143,38 @@ async def review_all_lenses( return join.as_outcome() -def register() -> None: - """Claim the `review/*` ids. Called by an adopter's `lockstep.py`, never on import.""" - workflow(id=ALL_LENSES)(review_all_lenses) +def register(*, gating: Collection[str] | None = None) -> None: + """Claim the `review/*` ids, and say which of the bound lenses gate a pull request. + + Called by an adopter's `lockstep.py`, never on import. + + `gating` lives here and not beside `AiReview(lenses=...)` because it is a property of the + CHECK rather than of the adapter. The adapter's map says which lenses this repository HAS, and + `chatops.aspect_from` resolves `/review ` against exactly that map -- so narrowing it to + keep a lens off the required check would also put that lens out of reach of the comment asking + for it. That is the lever #449 was filed about being the wrong one, and this is the right one. + + `None`, the default, gates on every bound lens: a repository that says nothing is unaffected. + A named lens the bound adapter does not declare refuses the run by name, at run time and + before the fan-out, so nothing has spent. + """ + selection = None if gating is None else tuple(gating) + + async def all_lenses( + ctx: RunContext, base: str, head: str, comments: str = "", diff: str = "" + ) -> Outcome[Any]: + return await review_all_lenses(ctx, base, head, comments, diff, gating=selection) + + # Re-declared rather than wrapped with `functools.wraps`. `wraps` sets `__wrapped__` and + # `inspect.signature` follows it, which would put `gating` among the arguments `in-lockstep + # run` accepts as `--arg` -- a lens list reachable from the trampoline YAML, i.e. exactly what + # `GATE-REVIEW-5` closed. The four parameters above are the whole callable surface. + # + # `__qualname__` is left as the closure's own: two calls read as ONE declaration to + # `_same_declaration` (same module, same qualname), so re-executing a `lockstep.py` reinstalls + # its selection rather than raising `DuplicateWorkflow` against itself. + all_lenses.__doc__ = review_all_lenses.__doc__ + workflow(id=ALL_LENSES)(all_lenses) __all__ = ["ALL_LENSES", "bound_lenses", "register", "review_all_lenses"] diff --git a/tests/in_lockstep/test_enforced_review.py b/tests/in_lockstep/test_enforced_review.py index 2bd9389..35593c1 100644 --- a/tests/in_lockstep/test_enforced_review.py +++ b/tests/in_lockstep/test_enforced_review.py @@ -27,14 +27,14 @@ import pytest import yaml -from in_lockstep.cli import _ensure_review_bound, _review_lenses +from in_lockstep.cli import _ensure_review_bound, _parameters, _review_lenses from in_lockstep.core.container import Container from in_lockstep.core.context import RepoInfo, RunContext from in_lockstep.core.outcome import Finding, Outcome, Status from in_lockstep.core.verbs import Capability, Verb -from in_lockstep.core.workflow import restore, snapshot +from in_lockstep.core.workflow import get, restore, snapshot from in_lockstep.loader import load -from in_lockstep.workflows.review import bound_lenses, review_all_lenses +from in_lockstep.workflows.review import ALL_LENSES, bound_lenses, register, review_all_lenses ROOT = Path(__file__).resolve().parents[2] WORKFLOW = ROOT / ".github" / "workflows" / "lockstep.yml" @@ -145,3 +145,195 @@ def test_this_repositorys_check_runs_the_fan_out_once_recording_on_one_tape(lens assert "--arg comments=review-comments" in run # The shipped four are among what this repository binds, so the workflow will run them. assert {"security", "intent", "performance", "tests"} <= lenses + + +# -- which lenses GATE, as distinct from which lenses EXIST (`GATE-REVIEW-9`, #449) ------------ + + +@pytest.fixture +def registry() -> Any: + """`register` writes to a process-global registry, so every test that calls it restores.""" + state = snapshot() + try: + yield + finally: + restore(state) + + +def _registered() -> Any: + entry = get(ALL_LENSES) + assert entry is not None, "register() did not claim the id" + return entry.fn + + +def test_gate_review_9_only_the_gating_lenses_run_and_the_rest_are_never_asked( + registry: None, tmp_path: Path +) -> None: + """`GATE-REVIEW-9`. Five lenses bound, two of them gating: the check runs exactly those two, + costs two calls rather than five, and writes two comment bodies. The other three are not + narrowed away from the adapter -- they are simply not this check's business.""" + adapter = _Lensed("security", "intent", "performance", "tests", "licensing") + ctx = _ctx(adapter) + # Named in the reverse of the order they run in, deliberately: the branches are ordered by + # what the ADAPTER declares, not by how the selection was typed, so the steps below assert an + # ordering that a `for name in asked` implementation would get wrong. + register(gating=("tests", "security")) + outcome = asyncio.run(_registered()(ctx, "main", "HEAD", comments=str(tmp_path / "bodies"))) + assert sorted(adapter.asked) == ["security", "tests"], "a lens that does not gate was paid for" + assert [s.step for s in ctx.steps] == ["security", "tests"] + assert sorted(p.name for p in (tmp_path / "bodies").glob("*.md")) == ["security.md", "tests.md"] + assert outcome.status is Status.SUCCEEDED + # Still every declared lens, unchanged: the selection is the check's, not the adapter's. + assert bound_lenses(ctx) == ("intent", "licensing", "performance", "security", "tests") + + +def test_gate_review_9_a_lens_that_does_not_gate_is_still_reachable_from_the_thread( + registry: None, +) -> None: + """`GATE-REVIEW-9`, and the property the obvious implementation breaks. + + The wrong fix for #449 is to narrow `AiReview(lenses=...)`, because `chatops.aspect_from` + resolves `/review ` against that same map -- so dropping a lens to keep it off the check + also puts it out of reach of the comment asking for it. Declaring the gating set must leave + what the adapter declares exactly as it was, which is what this asserts from the chat-ops side. + """ + from types import SimpleNamespace + + from in_lockstep.adapters.ai.review import Review + from in_lockstep.platform.chatops import aspect_from + + adapter = _Lensed("security", "intent", "performance", "tests", "licensing") + register(gating=("security", "tests")) + + # AFTER a gating run, over the same adapter object -- not over a fresh one. The narrowing this + # guards against is as easy to write at run time as at binding time, and a check made before + # the run would not see it. + asyncio.run(_registered()(_ctx(adapter), "main", "HEAD")) + assert sorted(adapter.asked) == ["security", "tests"] + + container = Container() + container.bind(Review, adapter) + known = _review_lenses(SimpleNamespace(container=container)) + assert known == ("intent", "licensing", "performance", "security", "tests"), ( + "the check narrowed what the adapter declares, which is the thread's set too" + ) + assert aspect_from("/review performance", known=known) == "performance", ( + "a bound lens that does not gate must still be nameable on a pull request" + ) + + +def test_gate_review_9_a_gating_lens_the_adapter_does_not_declare_refuses_before_the_fan_out( + registry: None, +) -> None: + """`GATE-REVIEW-9`. A typo gates on fewer lenses than somebody meant, and a check that goes + green for the wrong reason is the one failure a required check must not have. Refused by name, + naming both the unknown lens and the set that exists, with zero model calls -- and BLOCKED + exits 3, so the check is red rather than green-over-nothing.""" + adapter = _Lensed("security", "intent", "performance", "tests") + register(gating=("security", "test")) + outcome = asyncio.run(_registered()(_ctx(adapter), "main", "HEAD")) + assert outcome.status is Status.BLOCKED and outcome.reason == "review.gating_unknown" + message = outcome.findings[0].message + # Exact, not a substring search: "test" is a substring of "tests", so a loose assertion + # here would pass against a refusal that named the declared set and never the typo. + assert message.startswith("`register(gating=...)` names test, which"), message + assert "intent, performance, security, tests" in message, "the refusal lists what does exist" + assert adapter.asked == [], "a misconfigured gating set must not reach a model" + + # More than one at a time, because the singular case cannot tell a join from a bare name and + # somebody renaming lenses gets several wrong at once. + register(gating=("security", "test", "licencing")) + outcome = asyncio.run(_registered()(_ctx(adapter), "main", "HEAD")) + assert outcome.status is Status.BLOCKED and outcome.reason == "review.gating_unknown" + assert outcome.findings[0].message.startswith("`register(gating=...)` names licencing, test, which"), ( + outcome.findings[0].message + ) + assert adapter.asked == [] + + +def test_gate_review_9_registering_twice_replaces_the_selection_rather_than_conflicting( + registry: None, +) -> None: + """`GATE-REVIEW-9`. Any host that loads a `lockstep.py` more than once in one process + registers twice -- a long-lived worker, a test harness, two CLI invocations sharing an + interpreter -- and `_same_declaration` reads that as one declaration evaluated twice rather + than as two workflows claiming an id. It reads it that way by comparing module and + `__qualname__`, so the property survives here only because `register` leaves the closure's own + qualname alone, which was prose in a comment until this. Both directions, because a selection + that could not be taken back off would be the worse failure.""" + adapter = _Lensed("security", "intent", "tests") + + register(gating=("security",)) + register() + asyncio.run(_registered()(_ctx(adapter), "main", "HEAD")) + assert sorted(adapter.asked) == ["intent", "security", "tests"], "the narrowing was not undone" + + again = _Lensed("security", "intent", "tests") + register(gating=("security",)) + asyncio.run(_registered()(_ctx(again), "main", "HEAD")) + assert again.asked == ["security"], "the later selection did not win" + + +def test_gate_review_9_gating_on_nothing_is_refused_rather_than_read_as_gating_on_everything( + registry: None, +) -> None: + """`GATE-REVIEW-9`. `gating=()` and no `gating=` at all are opposite intentions, and the + silent reading of the first is a required check that can never fail. Absent means every bound + lens; empty means somebody wrote something that cannot mean anything, so it is refused.""" + adapter = _Lensed("security", "tests") + register(gating=()) + outcome = asyncio.run(_registered()(_ctx(adapter), "main", "HEAD")) + assert outcome.status is Status.BLOCKED and outcome.reason == "review.gating_empty" + assert "security, tests" in outcome.findings[0].message + assert adapter.asked == [] + + +def test_gate_review_9_the_registered_workflow_takes_no_gating_argument(registry: None) -> None: + """`GATE-REVIEW-9`, structurally. `in-lockstep run` builds its `--arg` surface by + introspecting the registered callable, so a `gating` parameter visible there would be a lens + list reachable from the trampoline YAML -- exactly the question `GATE-REVIEW-5` closed. This + is why `register` re-declares the signature instead of `functools.wraps`, which sets + `__wrapped__` and would make `inspect.signature` report the wrapped parameters.""" + import inspect + + register(gating=("security",)) + fn = _registered() + assert list(inspect.signature(fn).parameters) == ["ctx", "base", "head", "comments", "diff"] + assert not hasattr(fn, "__wrapped__") + assert "gating" not in _parameters(fn), "the selection became a --arg" + + +def test_gate_review_9_this_repository_gates_on_every_lens_it_binds(registry: None) -> None: + """`GATE-REVIEW-9`, the half that is about this repository rather than about adopters. + + An adopter chooses; here all of them gate, by decision. O10 exists so the shipped lenses are + not things we ask adopters to trust on our word, and `intent`, `performance` and `tests` had + already spent a release shipped-but-never-run once (#241). Asserted by running the workflow + THIS module registered over a stub declaring five lenses: a selection of any kind would show + up as fewer than five branches.""" + # Provenance first: what runs below has to be the closure THIS module installed, not one a + # neighbouring test left in the registry. `register` builds a fresh `Registered` per call, so + # an unchanged entry means `.lockstep/lockstep.py` never registered the check at all -- at + # which point every assertion after this would be about somebody else's registration. + before = get(ALL_LENSES) + module, _ref = load(str(ROOT)) + entry = get(ALL_LENSES) + assert entry is not None and entry is not before, ( + "this repository's module did not register review/all-lenses; the rest of this test would " + "be asserting over a leftover registration" + ) + + adapter = _Lensed("security", "intent", "performance", "tests", "licensing") + ctx = _ctx(adapter) + # `Workflow` is annotated `Callable[..., Awaitable[Any]]`, which `asyncio.run` will not + # take; the registry stores what it stores and the widening belongs here, not there. + registered_fn: Any = entry.fn + asyncio.run(registered_fn(ctx, "main", "HEAD")) + assert sorted(adapter.asked) == ["intent", "licensing", "performance", "security", "tests"], ( + "this repository's registration narrowed the required check" + ) + # Five over a stub says the registration narrows nothing; this says what it is not narrowing + # HERE, so the row's claim is about this repository's real lens set rather than about a stub. + _ensure_review_bound(module.lockstep) + known = _review_lenses(module.lockstep) + assert known is not None and {"intent", "performance", "security", "tests"} <= set(known)