Skip to content

Make the pin's cross-repository lockstep state measured against the aggregation, not declared alone - #1145

Merged
brettheap merged 32 commits into
mainfrom
change/assert-review-lane-lockstep-against-aggregation
Sep 23, 2026
Merged

brettheap merged 32 commits into
mainfrom
change/assert-review-lane-lockstep-against-aggregation

Conversation

@brettheap

@brettheap brettheap commented Sep 22, 2026 •

Copy link
Copy Markdown
Contributor

Lane: openxfactory-4 (openXfactory-4-openDox_extraction)

The remedy the pin file recorded for itself. Copilot raised a HIGH finding on #1136 — the lockstep flip at the fourth re-point ceremony, merge 7c49825e — at r4067694104, review 5273295522. The lane verified the premise and declined the remedy as outside that pull request's mandate (r4067703286), and the flip's own appended reason: paragraph recorded the remedy's shape as option (b):

"make the CROSS-REPOSITORY fact assertable at all — nothing today reads xFactory's three surfaces and compares them to this value, which is why a false status survives in EITHER direction until a human notices … (b) is the better fix, because (a) still leaves this file's truth depending on a lane remembering to write it. Both are OpenSpec-class acts on a realized capability, which is exactly why neither rides here."

This packet is that act. Status: draft — not ratified by the authoring lane.


The hazard, measured on main 2e222d98

fact measurement
the declaring field contracts/review-lane-pin.yaml:714 status: converged
the value it is about core_commit:60 b21f0100…
the aggregation's three surfaces opensoft/xFactory main: merge-master-approval.yml:152, council-convening-lane.yml:308, tests/test_merge_master_workflows.py's MIGRATION_PIN:72 — all three b21f0100…
what the repin lane may write WRITABLE_PATHS = four files; within the pin, only regex-anchored core_commit, floor_snapshot.sha256, floor_snapshot.entry_count
can it write the field? grep -c lockstep scripts/review_lane_repin.py → 0
does anything read the aggregation? grep -nE "opensoft/xFactory|MIGRATION_PIN|council-convening" scripts/review_lane_repin.py .github/workflows/review-lane-repin.yml → no output. MIGRATION_PIN appears in no file under scripts/ or .github/workflows/.
the guarding test test_the_pin_records_its_lockstep_state_with_the_aggregation_pin asserts the field against a literal in the test file — this repository's value against this repository's own constant

Beware the substring. A bare grep -c xFactory on those two files answers 13 and 32, and every hit is openxFactory. The measurement above is the one that means anything.

So the declaration is true today and nothing keeps it true.

It is not hypothetical, and it is live right now

It last went false on 2026-09-18, when the hourly lane's advance (#1122 → 38f826c2, 20:53:37-04:00) moved core_commit and left converged standing. The hand repair (#1123 → 8184d74a, 21:27:43-04:00) followed 34 minutes 6 seconds later, both first-parent on main. Thirty-four minutes because a human was watching that evening. Nothing bounds it.

And #1138 is OPEN as this is written — not a draft, platform auto-merge armed, proposing core_commit b21f0100… → 491fc54d…:

$ git diff main...origin/bot/review-lane-repin --stat
 .github/workflows/merge-master-approval.yml | 4 ++--
 .github/workflows/pytest-suite.yml          | 2 +-
 contracts/review-lane-floor-snapshot.yaml   | 9 ++++++---
 contracts/review-lane-pin.yaml              | 6 +++---
$ git diff main...origin/bot/review-lane-repin | grep -cE "^[-+].*(lockstep|converged|diverged)"
0

tests/review_lane_pin/test_review_lane_caller.py is not in that diff at all. The hour it merges, lockstep.status becomes false, the test still passes, and nothing reports it.

Why shape (b) and not shape (a) — decided by measurement

The lane cannot write the field honestly, because it has no reading of the aggregation to write from. Zero hits, in both files. Under (a) it would have exactly one thing it could write — diverged, on every advance — by inference from "MIGRATION_PIN does not move on a routine advance."

That inference is already falsified by this estate's own history. At the fourth ceremony (opensoft/xFactory #475 → c88d1fdd, 2026-09-21) the aggregation converged onto a commit this file had been pinned to since 2026-09-18 — three days apart, not a pairing at all, which the pin file records as the first time that happened. An advance landing where the aggregation already is leaves the pair converged. So (a) does not remove the false declaration; it changes which direction it is false in.

And the capability has already ruled on assertion-versus-measurement, one requirement over. An automated pin advance moves every pinned site in one commit or opens nothing closes: "what is added here is that an UNATTENDED author must prove the lockstep by measurement rather than assert it." Shape (a) would install inside that same capability precisely what that sentence forbids. Shape (b) extends its principle to the one lockstep it does not yet reach.

The capability is also already built for (b). Its realization's declared split — "the workflow reads the two repositories with gh … and hands this module PURE INPUTS … Every requirement's refusal is then a unit test with a fixture, not a workflow that has to be fired to be believed" — is exactly what the assertion needs: one more repository read where reads already happen, one more pure comparison where judgments already happen. No new architecture.

(a) is not forbidden by this requirement. If a later act teaches the lane to read the aggregation, it may then write the field — and it would be writing a measurement, which makes (a) a special case of (b) rather than an alternative to it.

The requirement

ONE ## ADDED, eight scenarios — an absent or foreign declaration, a stale converged, a stale diverged, surfaces disagreeing with each other, the check's own access failing, an unreadable aggregation, a candidate unread or its head moved, and the pure-function reproduction. The state is measured, not declared; the check runs at least at every proposed advance and its pull request, whoever opened it — in practice on every pull request that changes the pin, a fork's included — so the contradiction is reported on the same pull request that creates it rather than whenever a human next reads the field; it observes this repository's side of the pair, so a MIGRATION_PIN re-point in the aggregation is reported at the next pull request here that changes the pin (tasks.md § 6.7); the read lives in the workflow and the comparison is a pure function; it is symmetric (a false converged and a false diverged alike); and the declaration is not removed — it becomes the subject of the measurement rather than its only evidence.

The aggregation's surfaces are read AS A SET (r4075976905) at ONE resolved commit of it — never independently from a moving branch, because a re-point landing between two API calls returns a mixed set the check would report as INCONSISTENT, a state that never existed in the repository it was reading. They must agree with each other before either state is concluded; disagreement is its own outcome, INCONSISTENT, with each surface named by a digest and the commit they were read at named with the verdict. The outcomes are ORDERED and exactly one holds for any input a run judges (r4076152698, r4076557241, r4084891099, r4085195340) — the candidate unread FIRST (a read of this repository failing, for any reason but the pin's absence), then the declaration outside its vocabulary (a foreign state, a non-commit core_commit, an unparseable candidate, the pin absent, or a converged_with: other than the read plan's workflow members), before the aggregation is read at all, then the check's own access failing, then unreadable (a surface whose value is not a commit counts as unreadable, r4076989950), then disagreeing, then the comparison — because giving each outcome a conclusion is exactly the moment overlapping cases become a contradiction, and two of mine did. One partition classifies every failure to read, of either repository: a transient failure is UNDETERMINED, a 404 for a branch, a commit or a file is an absence, and every other failure is the check's own access failing, a 404 from the head's lookup (a read of the pull request) among them. A run that finds its head moved publishes no verdict. It is the one run that publishes none, because the push that moved the head starts a run of its own.

Each outcome carries a CONCLUSION and three fail:

outcome conclusion why
declaration absent or outside its vocabulary — a foreign state, a non-commit core_commit, an unparseable candidate, the pin absent at the verified head, a converged_with: other than the read plan's workflow members FAIL, naming the value found or the pin's absence this repository's own file, checked once the candidate is in hand and before the aggregation is read — a defect that makes every later question moot
the check's own access fails — binding unresolved, mint refused, either repository refusing its request, or any failure to read that is neither transient nor an absence FAIL, naming the failure this repository's own configuration or code; an UNDETERMINED here would stand on every run
declared state CONTRADICTED FAIL, naming core_commit and the agreed commit's digest this repository's own file states something false; the remedy is one edit
surfaces disagree (INCONSISTENT) NEUTRAL, visible another repository's defect
aggregation unreadable, or a read of the candidate failing transiently (UNDETERMINED) NEUTRAL, visible another repository's availability, or the host's
declared state agrees pass

That does not contradict the decision not to block a lawful divergence: what fails is not the divergence but the false declaration about it. And UNDETERMINED is forbidden as a STANDING state: the check's own access failing is its own FAIL, so it cannot stand as a permanent NEUTRAL. No aggregation value is published as it is: this repository and its checks are public and the aggregation is private, so a surface is published only by its classification, its relation to core_commit and a digest (design.md D20).

## ADDED and no ## MODIFIED is a decision (design.md D3): three active changes carry deltas on this capability and none names this requirement or is named by it, and the hazard lives in the gap between the advance this capability automates and a field no requirement obliges anyone to measure — not in any promoted requirement's text.

Boundaries this packet holds

  • The disposition mechanism is untouched — validate-openspec-cli-pin.py, reconcile() and contracts/openspec-cli-pin.yaml are not this packet's subject in any respect.
  • The pin file is not hand-patched. lockstep.status, converged_with: and the append-only reason: narrative do not move. A packet that edited the value it proposes to start checking would assert the very fact it exists to stop asserting.
  • WRITABLE_PATHS and PINNED_SITES gain no member.
  • No act in opensoft/xFactory is proposed. The aggregation is READ. MIGRATION_PIN advances only at a recorded ceremony and this packet neither performs nor requires one.
  • The obligation: "only at a ceremony" clause is NOT narrowed here — still the ratifier's act, registered at tasks.md § 6.1. This packet makes the state measurable, which is what lets that clause be narrowed on evidence rather than on argument.
  • Advance the pinned decision core to fe9a6f5b91886e4f07296dc6a264deb345b1618f #1138 is not this packet's to land, hold or amend. It is cited as the live reproduction.

Gate

  • OPENSPEC_TELEMETRY=0 openspec validate assert-review-lane-lockstep-against-aggregation --strict → valid — a narrowed claim, about this change alone.
  • validate-code-surface.py / validate-target-release.py / validate-sequenced-after.py → exit 0 each.
  • Corpus-wide, through the adjudicated entrypoint the gate itself runs — python3 scripts/validate-openspec-cli-pin.py --all --no-cache (.github/workflows/openspec-cli-pin-gate.yml:101) → exit 0: @fission-ai/openspec@1.12.0 verified against its content address, 111 passed, 1 failed (112 items), 0 undispositioned. The one failure is add-chain-attestation / signed-execution-chain/spec.md, INHERITED from main and not this packet's, applied as the disposition contracts/openspec-cli-pin.yaml carries for it (Brett Heap, 2026-09-05, "take exit 2"). The raw openspec validate --all --strict reports that same finding as a blocking error because it cannot read the disposition — which is why, under the now-ratified require-adjudicated-validation-entrypoint (landed on main as 2e222d98), a corpus-wide claim names the entrypoint and not the raw tool.
  • code_surface: openxFactory is not empty, so under release-realization this archives on merged plus green realization evidence — the check landed, all eight of its scenarios exercised by fixtures plus the positive cases, the negative control (review-lane-repin.yml carries no pull_request trigger) and the gate's own trigger test, and one real observation of it concluding — converged, diverged or a named contradiction, and not UNDETERMINED, because an observation of UNDETERMINED proves the access is missing rather than that the check works. Stated here rather than discovered at the archive gate.

Sequencing with the lane's other open packet

This and #1143 both added a row to the README "OpenSpec Records" block and to tests/sequenced_after/corpus-ledger.yaml. #1143 has landed, as a151e462, and this branch still merges cleanly with main: git merge-tree --write-tree origin/main HEAD exits 0. The collision was measured and removed before that landing. With both rows at the head of the list, #1145 conflicted in README.md once #1143 had landed, so #1145's row was moved after the lane's other 2026-09-22 row; both orders then merged cleanly to the SAME tree, the ledger auto-merging throughout. On that fully merged tree, all of these passed: the three front-matter validators, both packets' strict validation, #1143's carriage proof, and the adjudicated entrypoint (112 passed, 1 failed (113 items), the one failure the dispositioned add-chain-attestation). main's required-check rules are not strict (strict_required_status_checks_policy: false), so this branch needs no merge from main.

Review record — 35 Copilot reviews at 31 heads; 87 findings (73 as threads, 14 only in Previously missed blocks), plus eight summary-line themes; 0 unresolved threads

82 taken as fixes, 4 answered by measurement, 1 measured and registered. Of the eight summary-line themes, six were taken, one was answered by measurement and one is registered. The last review, 5294384086 at d05ce453, reports Findings: None.

  • Answered: r4076254100, raised against a README count that a gh pr edit --body had already corrected. The edit and the review carry the SAME timestamp, 20:39:17Z.
  • Answered: r4078998769, which held that a base-repository check run cannot attach to a fork's head commit. Measured, it can: every pull request's head is published in the base repository as refs/pull/<n>/head. On the public fork pull request cli/cli#14474, the base repository resolves the fork's head commit and carries 20 check runs on it.
  • Answered: r4085195422, which held that a 404 from the head's lookup had no outcome. The partition already made it the check's own access ("a 404 for the repository or the pull request itself"); what it did not say was that the head is looked up by reading the pull request, and d05ce453 says so and lists the fixture.
  • Answered: r4085195461, a stale count of seven scenarios in this description. The review's run began at 17:08:42Z, and the description had been corrected to eight by 17:10:30Z.
  • Registered: r4078145216, the approver race. It needs an approver that admits a pin-changing pull request, and none does today, so tasks.md § 6.5 registers the obligation for the two active changes that would admit one.

Every thread carries a reply and is resolved. Every review's Previously missed block was read rather than trusting a zero-unresolved count: fourteen findings came only from there, and two more appeared there before they appeared as threads.

The three that changed the packet most, each a defect in my own text:

  1. r4076152594 — the check had no way to read the repository it measures. Measured: opensoft/xFactory is PRIVATE (gh api repos/opensoft/xFactory --jq .visibility → private; an unauthenticated raw read of a surface → 404), and contracts/review-lane-repin-binding.template.yaml grants contents:read on codeXfactory/codexFactory and nothing on the aggregation. On today's credentials the check could never conclude: it would have answered UNDETERMINED on every run then, and would end every run in its ACCESS FAIL now — and a check that never concludes looks exactly like one that works. Three consequences are now written down: a declared read-only binding in the same shape as the existing grant (with the same never_grants: set, so the family's rule that neither lane reaches into the other's repository holds); the standing-UNDETERMINED prohibition above; and § 5.4's evidence becoming one observation of the check concluding, not merely running.
  2. r4076152698 — two outcomes overlapped, so one set of inputs demanded both FAIL and NEUTRAL. A requirement no implementation could satisfy, introduced by my own previous-round fix. Answered by ordering the outcomes rather than by patching one WHEN.
  3. r4076152473 / r4076152544 — the contract had no workflow representation. A step exits 0 or non-zero — a green pass or a red failure and nothing else — so "NEUTRAL, visible, not a pass" needs the check-run API and a test that asserts the published conclusion rather than the exit code, or it degrades silently to green. And a read failure has to be an input, never a fatal step, or the UNDETERMINED path is unreachable by the only route that reaches it.

Rounds 4 and 5 found four more, every one a consequence of an earlier fix of mine rather than of the original text — which is the honest way to describe them:

  • r4076254027 — the binding covered one run of two. The PR-side gate is a separate workflow run: it cannot reuse a minted App token any more than it can reuse step outputs, and GITHUB_TOKEN cannot read a private repository. It mints its own from the same App, under a binding of its own (next item).
  • r4076368784 — and the binding is structurally single-consumer (consumer.holder_ref :100, resolution.resolved_by :167, "never passed to a second workflow" :190), so "names both consumers" needed a representation. It was first answered with a second consumer entry, and r4076990082 found that wrong one level down: the template's privileges: and resolution.scoped_to are binding-wide and its floored_repository grants write, so a seated consumer would mint with the advance lane's write authority. The gate now gets a binding of its own — contents:read on xFactory only, one down-scoped mint — and the never-passed statement is carried unchanged on both, because nothing is passed.
  • r4076368722 then r4076474882 — and the second REVERSED my answer to the first, on a test this repository already runs. A fork pull request receives no secrets, so the gate would answer UNDETERMINED on every fork event; my fix was a plain pull_request gate scoped by head.repo.full_name. test_the_head_executing_trigger_is_absent refuses exactly that on the secret-bearing caller — "a plain pull_request trigger would run the head's copy of this file with the App credential that reads a private repository in scope — the exfiltration shape the base-branch rule prevents" — and a same-repository condition cannot fix it, because under pull_request the head's copy of the workflow runs and can delete the condition. The gate now takes the estate's worked shape: pull_request_target (the base's copy runs), no checkout of the PR head (costing nothing — this check reads two repositories over the API and needs no candidate code). It first carried an allowlist as well, as defence in depth, and the review at eb01c3d1 showed that this capability forbids exactly that (below).
  • the neutral publication had no write path — no workflow here grants checks: write (merge-master-approval.yml:453 grants checks: read), so the publish would 403 and the contract would degrade to the silent green pass by a second route. The gate's own job declares exactly contents: read, pull-requests: read and checks: write — r4077837498 found that checks: write alone would have left it unable to read the candidate pin — and the aggregation token stays read-only.

The three reviews at 918e30e3 found nine, all taken in c82c8f76, and a search-based sweep found the same defects standing in places no thread named:

  • the gate was still called pull_request-triggered after D14 reversed it (r4076845460, r4076990021, and D7) — now pull_request_target throughout, with the reason the two existing pin gates may run on pull_request: neither holds a secret;
  • the trigger test binds merge-master-approval.yml only (r4076845535) — § 5.1b now asserts the gate's own triggers;
  • D9 still held the superseded order (r4076845591);
  • a published check run does not change the job's own, and a pull_request_target job check lands on the PR head — measured on this very pull request (r4076902144, r4077054569) — so the verdict run's name matches no job id, and it hangs on the verified candidate head.sha, never github.sha;
  • agreeing surfaces could carry a non-commit into the comparison (r4076989950) — a surface is now read only as a 40-hex commit, or it is UNREADABLE;
  • the second consumer inherited the advance lane's write grant (r4076990082) — the gate gets a binding of its own;
  • two outcomes fail, not one (r4077054636, and D8, which undercounted one paragraph below its own table).

The reviews at c82c8f76 and 4570063f found four more, all taken in 16fdd730:

  • the candidate could have chosen what the gate reads (r4077898334). Its converged_with: is head-controlled, and my own D16 rule of naming a non-commit surface by its value would then have printed any private xFactory file into the verdict. The requirement now takes only core_commit and the declared state from the candidate, and every aggregation path from the base; head data is judged, never followed.
  • checks: write alone would have starved the gate (r4077837498): a declared permissions: block sets undeclared scopes to none, so the job declares contents: read, pull-requests: read and checks: write.
  • the advance side had no publication path (r4077837539). It now records its verdict in the pull-request body the lane already composes, and the gate carries the conclusion on the same head. A lane push starts the gate, measured on #1138.
  • the advance lane's grant had no mint (r4077898366): it gains a third, xFactory contents: read.

And the review at 16fdd730 found one, taken in cb97b7ae: the requirement resolved "the aggregation's branch" without saying WHICH (r4078009970). It now resolves the aggregation's DEFAULT BRANCH at run time, never a caller-supplied ref. That is the capability's own rule for the source repository, which review-lane-repin.yml:433-459 already realizes.

The reviews at cb97b7ae, 62a0945c and eb01c3d1 found eight more, six as threads and two only in Previously missed blocks:

  • every outcome needed a published conclusion, not only NEUTRAL (r4078058050, taken in 62a0945c). failure, neutral and success are now each mapped, and the test asserts each one.
  • the aggregation's constant needed a selector (review 5285753827's previously missed item, taken in eb01c3d1). Each value is now read at its selector, and a decoy-literal fixture proves it.
  • the third mint needed the tests that fix the binding at two mints to move with it (r4078098133, taken in eb01c3d1).
  • the triple named its base but not its author or head ref (r4078098160, taken in eb01c3d1, since superseded).
  • the head re-read needed a test that drives it (r4078098168, taken in eb01c3d1).
  • the allowlist itself was the defect (r4078145229, and review 5285808064's previously missed item, taken in 16dc4416). This capability's promoted An automated pin advance is judged by the freshness checks that already exist, with no exemption forbids any check that judges a pin advance to branch on the pull request's author, branch or origin. The gate now judges every pull request that changes the pin, a fork's included. Its safety rests on base code, inert candidate data and a read plan now fixed in its code (design.md D17, D19). That is the second defence of mine on this packet that turned out to be the defect.
  • the approver race (r4078145216) is measured absent today and registered at tasks.md § 6.5.

The reviews at 16dc4416 through d36cb68f found thirty-one more, over ten rounds. Each was taken in the round it was raised, and every reply names its fixing commit. By theme:

  • the trigger and the token: the paths: filter is exactly the pin (r4078252960). A skip for edits that move no judged value was added (r4078308120) and later withdrawn as a bypass of the ordered outcomes (r4078883755). The declaration is validated first (r4078425707).
  • what is read: the read plan is fixed in the gate's code, never taken from the pin file (r4078365046). It has two workflow members and one additional MIGRATION_PIN read (r4078528181, r4078528208, r4078528244).
  • what is published: no aggregation value is published as it is. A classification, a relation to core_commit and a digest are published instead (r4078468841, r4078835190).
  • untrusted input: the candidate is parsed strictly under a byte ceiling (r4078835237), and nothing derived from it reaches a script except through env: (r4078835258). A non-commit core_commit and an unparseable candidate each FAIL first (r4078528147, r4078425733).
  • outcomes: the check's own access failure is its own FAIL (review 5286349291). Every outcome is data to one always-run publisher (r4078594297). The comparison scenarios' WHENs are disjoint on their own text (reviews 5286144492, 5286193943).
  • the runs:
    • overlapping runs are serialized, with the guarantee bounded to what holds without atomicity (r4078468867, r4078883704);
    • the lane's body record is historical (r4078594266);
    • the gate's identity is named once (review 5285930613);
    • an approver approves only on success (r4078528254).
  • scope and records:
    • the check says it observes this repository's side of the pair (r4078648320);
    • the live literal test is retired (r4078883732);
    • the pure function takes structured read results (review 5286688301);
    • three inventories I left behind were each found by review: D17's two-value comparison, target_release's six scenarios, and this description's counts (r4078425764, r4078648354, review 5286688301).
  • the last round (d36cb68f):
    • the App installation grant on opensoft is named as the realization's first prerequisite, since a binding and a mint declare the read without provisioning it (r4078926291);
    • the requirement now states the host's 3,000-file path-filter exception itself rather than only registering it (r4078926263);
    • seven statements that still said a missing grant would leave the check UNDETERMINED now say it fails on its own access.
  • the reviews at 1c037be6:
    • one thread was answered by measurement (r4078998769, above);
    • review 5286923179 reported Findings: None, but its summary named three themes, and each held on measurement. All three were taken in d6787530:
      • the conclusion paragraph now names all six outcomes' conclusions, the one PASS included;
      • D5's rationale is brought onto the fixed plan;
      • the reproduction scenario is given the pure function's whole input.
  • the review at d6787530 found one, r4084891099, and its summary named three themes. The finding and all three themes were taken in b0f02bd9 (design.md D21):
    • the candidate's own reads had no outcome, so "exactly one holds for any input" was false. The order now begins at the candidate unread;
    • one partition now classifies every failure to read, of either repository, with the check's own access as the residual class;
    • a verified head is now defined, a moved head publishes no verdict, and "is a commit" means the grammar;
    • an eighth scenario was added and the five inventories moved with it. D8's stale count of two failing outcomes is corrected.
  • the review at b0f02bd9 found three. One was taken in d05ce453: the one-outcome claim is scoped to the inputs a run judges, since a moved head judges nothing (r4085195340). The other two were answered, above (r4085195422, r4085195461).
  • the review at d05ce453 (5294384086) reported Findings: None. Its summary named two themes. Both classes had already had their two head-moving rounds, so each was answered by measurement in one comment, and the head stays:
    • access classification — no defect found. The partition is total and disjoint, and every statement of the class agrees with it;
    • branch-resolution inputs — the reproduction scenario's input list names the resolved commit but not the branch that the verdict also names. tasks.md § 5.2 already takes both. Registered for the next act on spec.md.

Summary by Sourcery

Add a draft OpenSpec change that measures the review lane pin’s declared lockstep state against the aggregation repository instead of trusting the declaration alone.

New Features:

  • Add a draft OpenSpec requirement to measure the review lane pin’s cross-repository lockstep state against the aggregation repository’s pin surfaces.
  • Define ordered, symmetric outcomes for valid, contradictory, inconsistent, unreadable, and inaccessible lockstep measurements, with explicit check conclusions.
  • Specify advance- and pull-request-side realization requirements, including safe private-repository access, candidate validation, neutral check publication, and pure-function testing.

Enhancements:

  • Document the measured hazard, its historical occurrence, design decisions, boundaries, sequencing, and realization obligations for the proposed lockstep assertion.
  • Register the change in the OpenSpec records and sequencing ledger without modifying the pin declaration or widening the repin lane’s writable surface.

Documentation:

  • Add a detailed proposal, design, specification, and task plan for the draft lockstep measurement capability.
  • Update the README active-changes record with the proposal’s scope, rationale, scenarios, and realization status.

Tests:

  • Define fixture coverage for the eight lockstep scenarios, positive agreement cases, trigger safety, candidate parsing, access failures, and pure-function reproduction as future realization work.

Chores:

  • Record the change as an active sole modifier sequenced after mirror-floor-regeneration-automation and leave ratification and implementation tasks outstanding.

🤖 Generated with Claude Code

…ggregation, not declared alone

Copilot finding r4067694104 (review 5273295522) against #1136, the lockstep flip
at the fourth re-point ceremony. The lane VERIFIED the premise and DECLINED the
remedy as outside that pull request's mandate (r4067703286), and the flip's own
appended reason: paragraph recorded the remedy's shape as option (b): "make the
CROSS-REPOSITORY fact assertable at all -- nothing today reads xFactory's three
surfaces and compares them to this value ... (b) is the better fix, because (a)
still leaves this file's truth depending on a lane remembering to write it. Both
are OpenSpec-class acts on a realized capability, which is exactly why neither
rides here." This packet is that act.

THE HAZARD, MEASURED ON MAIN 2e222d9. contracts/review-lane-pin.yaml:714
declares status: converged about core_commit:60 and the aggregation's
MIGRATION_PIN. WRITABLE_PATHS is four files and within the pin the lane rewrites
only regex-anchored sites; the string lockstep occurs ZERO times in
scripts/review_lane_repin.py; and opensoft/xFactory, MIGRATION_PIN and
council-convening together return ZERO hits across the driver and its workflow,
with MIGRATION_PIN appearing in no file under scripts/ or .github/workflows/ at
all. The guarding test asserts the field against a LITERAL in the test file, so
it compares this repository's value to this repository's own constant and can
never see the other repository.

Beware the substring: a bare grep -c xFactory on those two files answers 13 and
32, and every hit is openxFactory.

IT IS NOT HYPOTHETICAL AND IT IS LIVE. The declaration last went false on
2026-09-18 for 34 minutes 6 seconds (#1122 -> 38f826c at 20:53:37-04:00,
repaired by #1123 -> 8184d74 at 21:27:43-04:00, both first-parent on main) --
bounded by a human happening to watch, and by nothing else. And #1138 is OPEN
right now, not a draft, with platform auto-merge armed, proposing core_commit
b21f0100 -> 491fc54d in a four-file diff whose lockstep/converged/diverged line
count is ZERO.

SHAPE (b) AND NOT (a), DECIDED BY MEASUREMENT. The lane has no reading of the
aggregation, so under (a) it could only write diverged by INFERENCE from
"MIGRATION_PIN does not move on a routine advance" -- an inference this estate
has already falsified: at the fourth ceremony (xFactory #475 -> c88d1fdd) the
aggregation converged ONTO a commit this file had been pinned to for three days,
so an advance landing where the aggregation already is leaves the pair CONVERGED.
(a) does not remove the false declaration; it changes which direction it is false
in. And the capability has already ruled on this one requirement over: "an
UNATTENDED author must prove the lockstep by measurement rather than assert it."
The capability is also already built for (b): the workflow reads repositories and
hands the module pure inputs, so every verdict is a unit test with a fixture.

BOUNDARIES HELD. The disposition mechanism is not touched. The pin file is not
hand-patched -- a packet that edited the value it proposes to start checking
would assert the very fact it exists to stop asserting. WRITABLE_PATHS and
PINNED_SITES gain no member. No act in opensoft/xFactory is proposed; the
aggregation is READ. The obligation: field's "only at a ceremony" clause is NOT
narrowed here and is registered at tasks.md 6.1.

ADDED and not MODIFIED, which is a decision: three active changes carry deltas on
this capability and none names this requirement or is named by it, and the hazard
is in the GAP between the automated advance and a field no requirement obliges
anyone to measure, not in any promoted requirement's text.

Lane: openxfactory-4
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings September 22, 2026 20:02
@sourcery-ai

sourcery-ai Bot commented Sep 22, 2026

Copy link
Copy Markdown

Reviewer's Guide

This draft OpenSpec packet proposes measuring the pin’s cross-repository lockstep state against xFactory’s aggregation surfaces rather than trusting a locally declared status, with symmetric contradiction reporting, explicit unreadable-state handling, and implementation deferred until ratification.

Flow diagram for symmetric lockstep checking

flowchart TD
    S[Proposed pin advance] --> I[Read aggregation surfaces]
    I --> U{Aggregation readable?}
    U -->|No| D[Report UNDETERMINED]
    U -->|Yes| C[Compare core_commit with aggregation values]
    C --> M{Values match declared status?}
    M -->|Yes| P[Report measured state agrees]
    M -->|No| X[Report contradiction]
    X --> V[Detect stale converged or stale diverged]
Loading

File-Level Changes

Change Details Files
Adds an OpenSpec proposal to make cross-repository lockstep status an observed assertion against xFactory’s aggregation pins.
  • Defines one added requirement with symmetric converged/diverged contradiction reporting.
  • Specifies checks on proposed pin advances and their pull requests, with workflow reads and a pure comparison function.
  • Defines UNDETERMINED handling for unreadable aggregation surfaces and preserves the pin declaration as the measured subject.
openspec/changes/assert-review-lane-lockstep-against-aggregation/proposal.md
openspec/changes/assert-review-lane-lockstep-against-aggregation/design.md
openspec/changes/assert-review-lane-lockstep-against-aggregation/specs/review-lane-floor-mirror/spec.md
Records the proposal’s provenance, design decisions, sequencing, validation status, and deferred realization work.
  • Adds ad hoc origin metadata, related pull requests, target-release and sequencing declarations.
  • Tracks ratification, implementation, realization evidence, and inherited obligations in the task checklist.
  • Documents validation commands and the expected merged-plus-realization archive gate.
openspec/changes/assert-review-lane-lockstep-against-aggregation/.openspec.yaml
openspec/changes/assert-review-lane-lockstep-against-aggregation/tasks.md
Documents the draft change and measured lockstep hazard in the repository’s active OpenSpec records.
  • Adds a detailed README active-change entry covering the finding, live reproduction, remedy choice, boundaries, and archive condition.
README.md

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

One row, by the sanctioned seeder with --moved-by '#1145': active, sole,
declares [mirror-floor-regeneration-automation], depth 2. Sole rather than
co-modifier because this packet's only delta is a ## ADDED requirement no other
change in the corpus writes.

Lane: openxfactory-4
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Unresolved moderate findings leave authorization, deterministic outcomes, PR enforcement, and positive test coverage incomplete.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 2 Medium severity · 1 Low severity

Open (3)
What changed in this PR

This draft OpenSpec packet proposes measuring the review-lane pin’s lockstep state against xFactory’s aggregation.

Changes:

  • Adds the lockstep measurement requirement and scenarios.
  • Documents design, realization tasks, evidence, origin, and sequencing.
  • Registers the active change in the README and sequencing ledger.
File Summary and final review notes
tests/​sequenced_after/​corpus-ledger.yaml Records sequencing provenance.
README.md Adds the active OpenSpec record.
openspec/​changes/​assert-review-lane-lockstep-against-aggregation/​tasks.md Defines realization work. Findings: missing PR-triggered gate/wiring (moderate, 2 votes); inconsistent task 4.4 state (nit, 1 vote); missing positive fixtures (moderate, 1 vote).
openspec/​changes/​assert-review-lane-lockstep-against-aggregation/​specs/​review-lane-floor-mirror/​spec.md Defines lockstep measurement. Findings: unspecified neutral check behavior (moderate, 1 vote), unresolved disagreement among aggregation surfaces (moderate, 3 votes), missing unreadable-result channel (moderate, 1 vote), and missing advance-PR enforcement (moderate, 1 vote).
openspec/​changes/​assert-review-lane-lockstep-against-aggregation/​proposal.md Documents scope and rationale. Findings: aggregation-read identity and permissions are undefined (moderate, 1 vote); incorrect task cross-reference (nit, 3 votes).
openspec/​changes/​assert-review-lane-lockstep-against-aggregation/​design.md Records implementation decisions.
openspec/​changes/​assert-review-lane-lockstep-against-aggregation/​.openspec.yaml Declares origin and related changes.

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread openspec/changes/assert-review-lane-lockstep-against-aggregation/tasks.md Outdated
Comment thread openspec/changes/assert-review-lane-lockstep-against-aggregation/proposal.md Outdated
… nowhere to run: both taken

Copilot round 1 on #1145. Three findings, all FACT, all taken.

r4075976905 -- THE MEASUREMENT WAS NOT DETERMINISTIC. The requirement said "the
commit the aggregation's judging surfaces name" while design.md D5 had the
implementation reading THREE values, and never said what happens if they differ
or which is authoritative. An implementation could pick one surface and claim
compliance. The surfaces are now read AS A SET and must agree before either state
is concluded; disagreement gets its own outcome -- INCONSISTENT, each surface
named with the value it carried, concluding neither converged nor diverged, with
its own scenario. A check permitted to pick one surface is choosing its own
answer, which is the same defect in miniature as the declaration this packet
exists to stop trusting. The text names the SET (the pin's converged_with:
members plus the aggregation's constant that holds them identical) rather than a
count, because converged_with: is a list the pin owner may extend.

r4075976980 -- THE PULL-REQUEST CLAUSE HAD NOWHERE TO RUN. Measured:
review-lane-repin.yml's on: keys parse to exactly ['schedule',
'repository_dispatch', 'workflow_dispatch'] -- no pull_request. The only
pull_request string in that file is a comment at :672, which is why a
line-oriented grep for it answers misleadingly. So task 5.1 alone left the
requirement's "and the pull request that carries it" clause unrealized: the
lane's own advance could open a pull request with no check running on it. The
realization now adds a pull_request-triggered gate of its own, on the estate's
existing pin-gate pattern (openspec-cli-pin-gate.yml,
openreposhape-pin-gate.yml), and review-lane-repin.yml deliberately does NOT gain
a pull_request trigger -- that would fire the advance logic on every pull request
in the repository. Task 5.1b makes the exclusion a TESTED negative control rather
than a convention. The declared code_surface names the new gate.

Also taken from the same finding's neighbourhood: task 5.3 now owes POSITIVE
fixtures -- a true converged and a true diverged each reported as agreeing -- so
the check is proved to accept a correct declaration and not only to refuse a
wrong one.

r4075977034 -- the proposal's cross-reference pointed a reader at task 5.1, the
workflow-read implementation task, for an obligation registered at 6.1. Corrected.

Recorded at design.md D6 and D7. Scenario count four to five, in the delta and in
the README row.

Lane: openxfactory-4
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings September 22, 2026 20:12

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Unresolved moderate and nit findings remain in the specification, proposal, and task definitions.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 2 Low severity

Open (2)
Resolved since last review (3)
Previously missed (2)

In code that hasn't changed since last review

Medium severity Define neutral workflow handling for UNDETERMINED results

openspec/​changes/​assert-review-lane-lockstep-against-aggregation/​specs/​review-lane-floor-mirror/​spec.md:54

The requirement defines an UNDETERMINED semantic state but not the workflow/check conclusion for it. That leaves an implementation free either to fail the gate (turning an xFactory outage into this repository's red build, which D4 rejects) or to pass without a visible neutral result; specify that the workflow publishes a neutral, non-failing report while not confirming either lockstep state.

Low severity Mark task 4.4 complete for the recorded ledger move

openspec/​changes/​assert-review-lane-lockstep-against-aggregation/​tasks.md:71

Task 4.4 is still unchecked even though the changed ledger row already carries moved_by: "#1145" and moved_on: "2026-09-22", which is exactly its stated condition once this pull request has a number. Leaving the box open makes the task state contradict the ledger; tick it with the recorded measurement.

Comment thread openspec/changes/assert-review-lane-lockstep-against-aggregation/proposal.md Outdated
Comment thread openspec/changes/assert-review-lane-lockstep-against-aggregation/tasks.md Outdated
Copilot round 2 on #1145. Four findings -- two open, two in the PREVIOUSLY MISSED
block, which is why that block is read rather than trusting a zero-unresolved
count. All FACT, all taken.

THE PREVIOUSLY-MISSED ONE IS THE SUBSTANTIVE ONE. The requirement defined an
UNDETERMINED semantic state and never said what the CHECK concludes, leaving an
implementation free to fail the gate on an xFactory outage -- which design.md D4
rejects -- or to pass without a visible neutral result. Taken, and it forced the
missing half of D2:

  declared state CONTRADICTED  -> FAIL, naming both values. This repository's own
                                  contract file states something false about
                                  another repository; the remedy is one edit.
  surfaces disagree            -> NEUTRAL, visible. Another repository's defect.
  aggregation unreadable       -> NEUTRAL, visible. Another repository's
                                  availability, which is D4's whole point.

A NEUTRAL conclusion is not a pass and silence does not stand in for it: a state
a check computes and does not publish is a state nobody acts on, which is the
same defect as a declaration nobody measures.

THIS DOES NOT CONTRADICT D2. D2 refuses a gate that BLOCKS AN ADVANCE for leaving
the aggregation behind -- divergence is a lawful state the pin file has carried
through four cycles. What fails here is not the divergence but the FALSE
DECLARATION about it. The advance stays lawful; only the sentence claiming
something untrue is refused. And the asymmetry is deliberate: the one outcome
that fails is the only one whose subject is THIS repository's own file.

The other three. Task 4.4 ticked -- the ledger row was seeded and leaving the box
open contradicted the artifact; tasks.md now states which boxes stay open on
purpose and who owes them a tick. The target_release archive evidence said four
scenarios while the code surface and task 5.3 require five plus positives -- it
now names all five, the positives and the negative control, so the archive gate
cannot be satisfied against an incomplete fixture set. Task 2.1's scenario count
four to five.

Recorded at design.md D8, and in the README row.

Lane: openxfactory-4
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings September 22, 2026 20:22
@brettheap

Copy link
Copy Markdown
Contributor Author

Lane: openxfactory-4 (openXfactory-4-openDox_extraction)

Answering the review's Previously missed (2) block, which carries no threads of its own — read because a zero-unresolved count is not evidence.

Correct, and fixed in 6184d18d. This is the substantive finding of the round, and it came out of the previously missed block rather than the open list — which is exactly why that block is read and a zero-unresolved count is not evidence.

You are right that the requirement named a state and never said what the check CONCLUDES, leaving both wrong answers available. It forced the missing half of D2.

outcome conclusion why
declared state CONTRADICTED by the measurement FAIL, naming both values this repository's own contract file states something false about another repository; the remedy is one edit to the field
surfaces disagree with each other (INCONSISTENT) NEUTRAL, visible another repository's defect, not this repository's claim to answer
aggregation unreadable (UNDETERMINED) NEUTRAL, visible another repository's availability — D4's whole point
declared state agrees pass

And a NEUTRAL conclusion is not a pass, and silence does not stand in for it — your second horn. A state a check computes and does not publish is a state nobody acts on, which is the same defect as a declaration nobody measures; this packet would be an odd place to reintroduce it.

Worth stating why this does not contradict D2. D2 refuses a gate that BLOCKS AN ADVANCE for leaving the aggregation behind — divergence is a lawful state the pin file has carried, with reasons, through four cycles. What fails here is not the divergence but the false declaration about it. The advance stays lawful; only the sentence claiming something untrue is refused, and fixing that is one edit rather than an act in another repository.

The asymmetry is deliberate: the one outcome that fails is the only one whose subject is THIS repository's own file. Turning another repository's outage into a red build here is how a governance check stops being run. Recorded at design.md D8 and in the scenario.


Correct, and fixed in 6184d18d. The row was seeded at 8cf65aa9 and leaving 4.4 open contradicted the artifact in the same pull request:

assert-review-lane-lockstep-against-aggregation: {state: active, class: sole, declares: [mirror-floor-regeneration-automation], depth: 2, prose: false, moved_by: "#1145", moved_on: "2026-09-22"}

sole rather than co-modifier because this packet's only delta is a ## ADDED requirement no other change in the corpus writes — recorded with the tick rather than left implicit.

tasks.md now also states which boxes stay open on purpose and who owes them a tick: §§ 1 and 4.3, and every box in § 5, are acts that have not happened yet — the ratifying word, the green head, the realization — so they are ticked by the acts that perform them. Every box for work this packet will never do carries [~], in § 6. That is the same distinction scripts/proposal-support.py:4632 enforces at the archive gate.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Unresolved critical findings remain around repository access, PR-side measurement, error handling, and distinct neutral check publication.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 4 High severity · 1 Medium severity

Open (5)
Resolved since last review (2)

Comment thread openspec/changes/assert-review-lane-lockstep-against-aggregation/tasks.md Outdated
Comment thread openspec/changes/assert-review-lane-lockstep-against-aggregation/tasks.md Outdated
Comment thread openspec/changes/assert-review-lane-lockstep-against-aggregation/tasks.md Outdated
…ory it measures

Copilot round 3 on #1145. Five findings, all FACT, all taken. Two of them are
consequences of my own round-2 fix, which is the honest way to describe them.

r4076152698 -- THE OUTCOMES OVERLAPPED. The stale-converged scenario's WHEN was
true whenever at least one surface did not name the new commit, INCLUDING when
the surfaces disagree with each other, which the INCONSISTENT scenario then
requires to be NEUTRAL. The same inputs demanded both FAIL and NEUTRAL. That is
not a wording problem; it is a requirement no implementation could satisfy. The
body now states the order -- UNREADABLE, then DISAGREEING, then the comparison,
then the declared state, each reached only where every earlier one does not hold
-- and both comparison scenarios' WHEN is qualified with the surfaces agreeing
with each other, and read. The defect was introduced by D8: a requirement gains a
contradiction the moment its cases stop being disjoint, and adding conclusions is
exactly when that happens.

r4076152594 -- THE CHECK HAD NO WAY TO READ THE REPOSITORY IT MEASURES, and this
is the most consequential finding on the packet. Measured: opensoft/xFactory is
PRIVATE (gh api repos/opensoft/xFactory --jq .visibility -> private; an
unauthenticated raw read of a surface returns 404), and
contracts/review-lane-repin-binding.template.yaml declares source_repository
codeXfactory/codexFactory with grants [contents:read] and nothing for the
aggregation. So on today's credentials the check would answer UNDETERMINED on
every run -- and a check that never concludes looks exactly like a check that is
working. Three things follow and all three are written down: the realization
declares a READ-ONLY binding for opensoft/xFactory in the same shape as the
existing grant, carrying the same never_grants set so the family's rule that
neither repository's lane may reach into the other's is kept; the requirement
forbids UNDETERMINED as a STANDING state; and 5.4's evidence is now one
observation of the check CONCLUDING, not merely running, because an observation
of UNDETERMINED proves the access is missing rather than that the check works.

r4076152473 -- a step exits 0 or non-zero, which is a green pass or a red failure
and nothing else, so "NEUTRAL, visible, not reported as a pass" has no expression
by exit code. The realization publishes through the check-run API with conclusion
neutral, and the test asserts the PUBLISHED CONCLUSION for each outcome rather
than the process exit code -- because this contract degrades silently to a green
pass if nobody checks which of the two was published.

r4076152645 -- the pull-request host is a separate workflow run and cannot consume
the scheduled run's step outputs, so 5.1a takes its own reading. Two reads of one
fact by two runs is the cost of the D7 split, and cheaper than a shared artifact
whose staleness would need a rule of its own -- which would be this packet's own
subject, recursively.

r4076152544 -- a read failure must be an INPUT and never a fatal step. The advance
workflow treats a non-404 API failure as step-fatal, so an auth error or a rate
limit would abort before the comparison ran and the UNDETERMINED scenario would
be unreachable by the only route that reaches it.

Recorded at design.md D9, D10 and D11; tasks 5.1a-5.1e and 5.4; and the declared
code surface gains the binding template.

Lane: openxfactory-4
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings September 22, 2026 20:32

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Credential and required-check wiring, evidence omissions, and specification gaps remain unresolved.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 High severity · 1 Low severity

Open (2)
Resolved since last review (5)

Comment thread README.md Outdated
…ct one workflow over

Copilot round 4 on #1145. One finding taken, one answered by measurement.

r4076254027 -- TAKEN. Task 5.1c declared a read-only binding for opensoft/xFactory
against the repin identity alone, while 5.1a's pull-request gate is a SEPARATE
WORKFLOW RUN: it cannot reuse the advance lane's minted App token any more than
it can reuse its step outputs, and the ambient GITHUB_TOKEN cannot read a private
opensoft/xFactory at all. So the PR-side check would have been UNDETERMINED on
every run -- the exact defect 5.1c exists to close, reappearing one workflow over,
and the state the requirement now forbids from standing.

The gate mints its OWN token from the same App (actions/create-github-app-token,
as review-lane-repin.yml already does at :278 and :303) under the same read-only
scope, the binding NAMES BOTH CONSUMERS, and a test asserts THE PULL-REQUEST
SIDE's read succeeds rather than only the advance side's. The two runs share an
identity and a scope; they share no token and no outputs -- D7's split restated
where it costs something.

r4076254100 -- ANSWERED, not taken: true when computed, already false at the body
it was raised against. The README row reads "five scenarios" at the reviewed
commit 266f9f1 and so does the pull request description; the body edit and the
review submission carry the SAME timestamp, 20:39:17Z, so the review read the
pre-edit body by seconds.

A note on how this landed: the first attempt at these three edits was one script
whose SECOND assertion failed, and it wrote nothing -- the anchor was missing a
leading backtick. The tree was verified clean before retrying, and the three
edits were then applied and verified one at a time, because an aborted script
that has already written some of its edits is how a packet acquires a half-fix
nobody sees.

Recorded at design.md D11 and proposal.md's code_surface declaration.

Lane: openxfactory-4
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings September 22, 2026 20:43

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Unresolved specification and implementation details could produce incorrect or non-gating results.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 High severity · 1 Medium severity

Open (2)
Resolved since last review (2)
Previously missed (2)

In code that hasn't changed since last review

Medium severity Read all aggregation surfaces from one immutable commit

openspec/​changes/​assert-review-lane-lockstep-against-aggregation/​specs/​review-lane-floor-mirror/​spec.md:30

Reading the three aggregation files independently from main does not produce one set snapshot: a re-point commit can land between API calls, yielding a mixed old/new set and a false INCONSISTENT result. Resolve the aggregation branch to one commit first and read every surface at that SHA (or fetch one tree response); otherwise the measurement is of read timing rather than the aggregation state.

Medium severity Grant and test checks:write for neutral publication

openspec/​changes/​assert-review-lane-lockstep-against-aggregation/​tasks.md:149

The planned neutral publication has no authorized write path. §5.1e requires creating/updating a check run, but §5.1c gives the new aggregation binding only contents:read and the task never grants checks:write to either the gate's GITHUB_TOKEN or its App token. Without that permission the API call will 403, so INCONSISTENT/UNDETERMINED cannot be visible as neutral; add an explicit least-privilege checks permission and a test for the published check-run path while keeping the aggregation read token read-only.

Comment thread openspec/changes/assert-review-lane-lockstep-against-aggregation/tasks.md Outdated
Comment thread openspec/changes/assert-review-lane-lockstep-against-aggregation/tasks.md Outdated
…e neutral conclusion had no write path

Copilot round 5 on #1145. Four findings -- two open, two from the PREVIOUSLY
MISSED block. All FACT, all taken, and all four are consequences of my own
earlier fixes rather than of the original text.

THE SNAPSHOT ONE CHANGES THE REQUIREMENT, not the task list, because it is about
what is measured. Reading the three surfaces independently from main is not a
snapshot: a re-point landing between two API calls returns a MIXED set, which the
check would report as INCONSISTENT -- a state that never existed in the
repository it was reading. The check now resolves the aggregation's branch to ONE
COMMIT first, reads every surface at that commit, and names the commit with its
verdict. A measurement taken across a moving ref measures read timing, not the
aggregation; and the outcome it would manufacture is the one nobody can check
against anything, because it says the other repository disagrees with itself.

THE NEUTRAL CONCLUSION HAD NO WRITE PATH. 5.1e publishes through the check-run
API while 5.1c's binding grants contents:read only, and no workflow in this
repository grants checks: write today (merge-master-approval.yml:453 grants
checks: read). The publish would 403 and NEUTRAL could never be visible -- the
contract degrading to exactly the silent green pass D11 was written to prevent,
by a different route. The gate's own job grants least-privilege checks: write;
the aggregation read token stays read-only and gains nothing, because the thing
written is a check run HERE and the thing read is ANOTHER repository.

r4076368784 -- THE BINDING IS STRUCTURALLY SINGLE-CONSUMER and round 4's "names
both consumers" waved at how. Measured: consumer.holder_ref names one workflow
(:100), resolution.resolved_by is review_lane_repin_workflow_only (:167), and the
statement says the minted token is "never passed to a second workflow" (:190).
The representation is a SECOND CONSUMER ENTRY with its own holder_ref and its own
mint, resolved_by widened to name both workflows, and the never-passed statement
CARRIED UNCHANGED -- because nothing is passed: each workflow mints its own token
from the same App under the same scope. The existing sentence is not an obstacle
the design works around; it is the design.

r4076368722 -- A FORK PULL REQUEST IS OUT OF SCOPE, NOT UNDETERMINED. A fork
event receives no secrets, so the gate could not mint a token and would answer
UNDETERMINED on every one -- the state the requirement forbids from standing,
reached by a route with nothing to do with the aggregation. The gate runs only
where the head repository IS this repository, which is where every pin advance
comes from, and a fork event is reported OUT OF SCOPE: "not applicable" and
"could not measure" are different answers, and this packet's whole subject is a
corpus that had stopped distinguishing a claim from a measurement.
pull_request_target is REFUSED BY NAME -- it is not foreign here
(doc-health-reusable.yml and merge-master-approval.yml both use it) but it runs
with secrets against an untrusted head, and no cross-repository measurement is
worth that. Recorded as a refusal rather than an omission.

Recorded at design.md D12, D13, D14 and D15; tasks 5.1f, 5.1g, 5.1h; and the
code_surface declaration.

Lane: openxfactory-4
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings September 22, 2026 20:54
@brettheap

Copy link
Copy Markdown
Contributor Author

Lane: openxfactory-4 (openXfactory-4-openDox_extraction)

Answering the review's Previously missed (2) block, which carries no threads of its own. Both are FACT, both are fixed in 162b9937, and both are consequences of my own earlier fixes rather than of the original text.

1. Reading three files from a moving branch measures READ TIMING, not the aggregation

This one changed the REQUIREMENT rather than the task list, because it is about what is measured:

THE SURFACES ARE READ AT ONE RESOLVED COMMIT OF THE AGGREGATION AND NOT INDEPENDENTLY FROM A MOVING REF. The check SHALL resolve the aggregation's branch to a single commit first and read every surface AT THAT COMMIT, and SHALL name that commit with its verdict.

Your reasoning is exact: a re-point landing between two API calls returns a MIXED set, which the check would report as INCONSISTENT — a state that never existed in the repository it was reading. And for this packet that failure would be particularly cruel, because INCONSISTENT is the one outcome nobody can check against anything: it says the other repository disagrees with itself.

The INCONSISTENT scenario now also carries **AND** the disagreement is the aggregation's own at that commit, not an artefact of reading its surfaces one at a time while a re-point landed between the reads. design.md D12.

2. The neutral publication had no authorized write path

Measured: no workflow in this repository grants checks: write — merge-master-approval.yml:453 grants checks: read and that is the only checks: grant anywhere. So § 5.1e's publish would 403 and INCONSISTENT/UNDETERMINED could never be visible as neutral — the contract degrading to exactly the silent green pass D11 was written to prevent, by a different route.

§ 5.1h grants least-privilege checks: write in the gate's own job, and the aggregation read token stays read-only and gains nothing — your own constraint, and the right one: the thing being written is a check run in THIS repository and the thing being read is ANOTHER repository, and keeping those two privileges in different places is the same separation the binding's never_grants: set exists to state. A test exercises the published check-run path. design.md D15.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Unresolved workflow security, input-model, revision-pinning, and evidence requirements remain.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 High severity · 1 Medium severity

Open (2)
Resolved since last review (2)
Previously missed (1)

In code that hasn't changed since last review

Medium severity Define outcomes for unknown or invalid declared statuses

openspec/​changes/​assert-review-lane-lockstep-against-aggregation/​specs/​review-lane-floor-mirror/​spec.md:69

The claim that exactly one outcome holds for any input has no case for a missing or unknown declared status. With agreeing surfaces, a value such as pending is neither converged nor diverged, so the comparison reaches no defined outcome; the current pin test only checks the live literal and does not close this input domain. Either make the pure function's precondition explicitly restrict status to those two values and define rejection, or add an invalid-declaration outcome and fixture.

Comment thread openspec/changes/assert-review-lane-lockstep-against-aggregation/tasks.md Outdated
Comment thread openspec/changes/assert-review-lane-lockstep-against-aggregation/tasks.md Outdated
…eady has a test against

Copilot round 6 on #1145. Three findings, all FACT, all taken -- and the first
reverses this packet's own D14 on evidence that was in the tree all along.

r4076474882 -- THE DECISION REVERSES ITSELF ON A RUNNING TEST. Round 5's fix
refused pull_request_target by name and scoped a plain pull_request gate by
head.repo.full_name. tests/review_lane_pin/test_review_lane_caller.py's
test_the_head_executing_trigger_is_absent refuses exactly that, verbatim: "a
plain pull_request trigger would run the head's copy of this file with the App
credential that reads a private repository in scope -- the exfiltration shape the
base-branch rule prevents", with EXPECTED_TRIGGERS = {"pull_request_target",
"workflow_dispatch"} beside it. A same-repository condition cannot fix it,
because under pull_request the head's copy of the WORKFLOW runs and can delete
the condition; any same-repository pull request author could have read a private
repository through this gate.

The gate now takes the estate's worked shape rather than one reasoned out here:
pull_request_target so the BASE's copy runs; no checkout of
github.event.pull_request.head, mirroring test_no_checkout_takes_the_pull_request_head
("rules must come from the base branch") and costing nothing because the check
reads two repositories over the API and needs no candidate code at all -- which
is exactly the safe base-code/API design r4076368722 asked for; plus a head-ref
allowlist (bot/review-lane-repin) and the same-repository condition as defence in
depth, with a test that a same-repository NON-BOT pull request is skipped.

The lesson is the one this packet keeps relearning about itself: I refused
pull_request_target from its general hazard without checking whether this estate
had already solved it -- and it had, in the workflow this very capability
governs, with the reason written into a test's failure message. A
first-principles refusal that contradicts a running test is a finding about the
reasoner.

PREVIOUSLY MISSED -- THE ORDERING HAD AN INPUT IT DID NOT REACH. D8's table and
D9's ordering claim exactly one outcome holds for any input, and neither had a
case for a declared status absent or outside the two words: with agreeing
surfaces, a value such as "pending" is neither converged nor diverged and the
comparison reaches no defined outcome. The requirement now closes it -- an absent
or foreign value FAILS naming what was found, in the same class as a contradicted
declaration, because both are this repository's own file failing to say something
true and the remedy for both is one edit to the field. Its own scenario; the
ordering is now total.

r4076474933 -- THE RESOLVED SHA IS NOW WIRED AND THE WIRING IS TESTED. D12
requires reading every surface at one immutable commit, and task 5.1 only said to
obtain the values. A workflow-level test now captures the resolved sha, asserts
EVERY surface read names it, and asserts the verdict names it too. Value fixtures
alone cannot catch a reader that takes each file from main independently.

Recorded at design.md D14 (reversed) and D14a; tasks 5.1 and 5.1g.

Lane: openxfactory-4
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings September 22, 2026 21:07

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🔵 Needs a closer look

Unresolved critical trigger coverage and additional specification gaps remain in a complex cross-repository, credentialed workflow design.

Review effort: Lite
Findings: 1 High severity · 1 Medium severity

Open (2)
Resolved since last review (2)

Comment thread openspec/changes/assert-review-lane-lockstep-against-aggregation/proposal.md Outdated
…of the pair the check observes

Copilot r4078648354: target_release still said ALL SIX and listed six
scenarios after the check's own access failure became the seventh. That is the
same inventory D14b names, left behind again. It now enumerates all seven,
access failure included, and D14b records the second miss.

Copilot r4078648320: the gate observes this repository's pull requests, so a
MIGRATION_PIN re-point in the aggregation can falsify the declaration with no
pull request here. That was true of the design, and nothing in the text said
so. The requirement now states which side it observes: an aggregation-only
move is reported at the next pull request that changes the pin, and the
ceremony's own pull request here is such a pull request (#1136, author
brettheap, changed the pin file). Observing the aggregation as it moves would
take an act in the aggregation or a schedule, and the requirement takes
neither. design D2 records this, and tasks 6.7 registers the gap for the
ratifier.

D2 also drops a claim the design never guaranteed: that the report lands
before the armed auto-merge can. The verdict is not a required check, and 6.5
already says requiring it is a ruleset act this packet does not take.

Lane: openxfactory-4 (openXfactory-4-openDox_extraction)
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings September 23, 2026 03:34

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Critical and moderate findings remain unresolved across workflow safety, parsing and commit validation, and outcome semantics.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 3 High severity

Open (3)
Resolved since last review (2)

…nothing derived from it reach a script but through env

Copilot r4078835190: forty lowercase hex characters prove the shape, not a
commit. A legacy personal access token was forty hex characters too. No
surface's value is now published as it is. A surface outside the grammar is
named by its classification. One inside it is named by its relation to
core_commit and the first twelve hex characters of its SHA-256. Only the
commit the API resolves the default branch to is named as it is. Verifying a
real commit would need a codexFactory read, which the gate's one mint
deliberately does not reach (D13). The spec, D6, D8, D11, D17, D20 and tasks
5.1, 5.1e, 5.1g and 5.1i move with it, and the lane's body uses the same
form.

Copilot r4078835237: the candidate's bytes are the fork's to choose. They are
now parsed under a 1 MiB byte ceiling fixed in code, against the pin's
measured 70,039 bytes, as exactly one YAML document with no duplicate key,
alias, merge key or tag. Each refusal counts as ABSENT and FAILS, and each has
its own fixture. A base that cannot be parsed counts as differing, so its
candidate is judged.

Copilot r4078835258: the safety argument constrained only event fields.
Nothing derived from the event or the candidate now reaches a run: script
except through env: or a file: no step output, no parsed field and no parse
error. That is held by a static test in the shape of test_workflow_contract.py's
test_untrusted_workflow_inputs_are_not_interpolated_into_bash.

Lane: openxfactory-4 (openXfactory-4-openDox_extraction)
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings September 23, 2026 03:47

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Unresolved critical and moderate findings remain in the specification and task plan, along with a documentation mismatch.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 2 High severity · 1 Medium severity

Open (3)
Resolved since last review (3)
Previously missed (2)

In code that hasn't changed since last review

Medium severity Add structured read outcomes to pure-function input contract

openspec/​changes/​assert-review-lane-lockstep-against-aggregation/​tasks.md:479

The pure-function task lists only the declared state, converged_with, core_commit, and surface values, but the function is also required to return the access-failure and unreadable-aggregation outcomes with the specified precedence. Those cases need explicit inputs for access/read errors and the resolved aggregation commit (and any validation result); otherwise the implementation must perform I/O or cannot reproduce those outcomes. Add those structured read outcomes to the function's input contract.

Low severity Update PR scope to match seven scenarios and three failures

README.md:642

The packet now defines seven scenarios and three failing outcomes (including the check's own access failure), but the PR description's requirement and generated summary still say six scenarios and two failures. Please update the PR description so the acceptance scope and the README/spec record do not disagree.

Comment thread openspec/changes/assert-review-lane-lockstep-against-aggregation/tasks.md Outdated
Comment thread openspec/changes/assert-review-lane-lockstep-against-aggregation/tasks.md Outdated
Comment thread openspec/changes/assert-review-lane-lockstep-against-aggregation/tasks.md Outdated
…ire the live literal, widen the pure function's inputs

Copilot r4078883755: the requirement orders its outcomes and has exactly one
hold for any input, so a skipped conclusion beside them was a bypass it does
not authorize. The skip is withdrawn and every valid admitted candidate is
judged, a reason:-only edit included. The declaration is still validated
first, with no mint for an invalid one. The exposure r4078308120 named is now
closed where it lives: the read plan is fixed in code (D19), and no
aggregation value is published as it is (D20). Tasks 5.1g, 5.1e and 6.5, D11,
D17, D19 and the proposal move with it.

Copilot r4078883704: neither cancellation nor the last head re-read is atomic
with check-run creation. The guarantee is now the one that holds without
atomicity. A verdict attaches to the one commit it verified and binds nothing
else, and every consumer reads it only on the pull request's current head.

Copilot r4078883732: test_review_lane_caller.py:641 holds lockstep.status to a
literal, the tautology tasks 3.6 names. Once the measurement exists, that
assertion becomes closed-vocabulary validation, and the status stops needing
its literal moved beside it by hand.

Review 5286688301's previously-missed item: the pure function of tasks 5.2 now
takes a structured read result: the access outcome, the resolved branch and
commit, and each surface's value or its transient failure. Without those it
could not return the access and unreadable outcomes without doing I/O.

Lane: openxfactory-4 (openXfactory-4-openDox_extraction)
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@brettheap

Copy link
Copy Markdown
Contributor Author

Lane: openxfactory-4 (openXfactory-4-openDox_extraction)

Review 5286688301's two previously missed items.

  • The pure function needs its read outcomes as input (tasks.md:479). Correct, and fixed in d36cb68f. § 5.2's function now takes the candidate's parse result (its three declared values, or the parse refusal), the plan's workflow members, and a structured READ RESULT. The read result holds the access outcome (granted, or which access failure), the resolved aggregation branch and commit, and each surface's value or the transient failure that kept it unread. It reaches no network and returns the six outcomes in the requirement's order, each with the values its verdict publishes.
  • The PR description said six scenarios and two failures (README.md:642). Correct. The PR body is updated to seven scenarios and three failing outcomes, the check's own access failure among them, with the outcome table rebuilt to match the spec.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Unresolved critical and moderate findings remain in the trigger coverage, credential provisioning, selector handling, verdict inputs, and comparator integration.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 2 High severity

Open (2)
Resolved since last review (3)

…te; state the host's path-filter exception in the requirement

Copilot r4078926291: a binding template and a mint step declare the xFactory
read but do not provision it. Tasks 5.1c now names the App installation grant
on opensoft (xFactory, contents: read), an organization owner's act, as the
realization's first prerequisite. Its evidence is the first successful mint,
recorded by run id beside the 5.4 observation. Until then every run ends in
the ACCESS failure, which is how the missing grant shows itself. Five
statements still said a missing grant would leave the check UNDETERMINED on
every run; they predated the ACCESS outcome and now say it fails on its own
access (tasks 5.1c twice and 5.1f, D10, D11, the proposal).

Copilot r4078926263: the requirement's "at least every proposed advance" read
as closing a gap that tasks 6.6 registers, GitHub's 3,000-file path-filter
limit. The requirement now states that exception itself. Tasks 6.6 adds what
closing it would take: an unconditional trigger comparing the pin's bytes at
the head and the merge base before minting. That is a different design, and
it is the ratifier's to choose.

Lane: openxfactory-4 (openXfactory-4-openDox_extraction)
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Eight unresolved findings remain, including a critical fork-safe publication gap.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 High severity

Open (1)
Resolved since last review (2)

Comment thread openspec/changes/assert-review-lane-lockstep-against-aggregation/tasks.md Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🔵 Needs a closer look

Unresolved moderate issues remain around failure outcome coverage, fixed-plan rationale, and pure-function scenario inputs.

Review effort: Lite
Findings: None

Resolved since last review (1)

… reproduction scenario the function's whole input; bring D5 onto the fixed plan

Review 5286923179's summary names three themes and carries no thread. Each
held on measurement.

Failure-outcome coverage: "EACH OUTCOME SHALL CARRY A CONCLUSION" listed the
conclusions for four of the six outcomes. The vocabulary FAIL stood in its own
paragraph, and nothing normative said that a declaration the measurement
agrees with PASSES. The paragraph now closes by naming all six: three FAIL,
two NEUTRAL, and the one PASS.

Pure-function scenario inputs: the reproduction scenario gave the reviewer
only the pinned commit, the surfaces' commits and the declared state. From
those alone the access, unreadable, parse and converged_with outcomes cannot
be reproduced. Its WHEN now gives the function's whole input, as tasks 5.2
defines it.

Fixed-plan rationale: design D5 still called converged_with: "the check's own
subject list" and read each workflow at the step checking out "the pin's own
repository:". Both predate the plan fixed in code (D19). D5 now says the plan
has the two surfaces converged_with: declares as its workflow members, read
at the step checking out codeXfactory/codexFactory. Tasks 6.2 names the
judgment against them.

Lane: openxfactory-4 (openXfactory-4-openDox_extraction)
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@brettheap

Copy link
Copy Markdown
Contributor Author

Lane: openxfactory-4 (openXfactory-4-openDox_extraction)

On review 5286923179's summary line. It reads "Unresolved moderate issues remain around failure outcome coverage, fixed-plan rationale, and pure-function scenario inputs", with Findings: None and no thread. I measured each theme against the text, found a real defect behind all three, and took each in d6787530.

  1. Failure-outcome coverage. EACH OUTCOME SHALL CARRY A CONCLUSION named the conclusions of only four of the six outcomes: a contradiction and an access failure FAIL, INCONSISTENT and UNDETERMINED are NEUTRAL. The vocabulary FAIL stood in its own paragraph, and nothing normative said that a declaration the measurement agrees with PASSES. The paragraph now closes by naming all six: three FAIL, two NEUTRAL, and the one PASS.
  2. Fixed-plan rationale. design.md D5 still called converged_with: "the check's own subject list", and it read each workflow at the step that checks out "the pin's own repository:". Both predate the plan fixed in code (D19). D5 now says the plan's two WORKFLOW MEMBERS are the two surfaces converged_with: declares, read at the step that checks out codeXfactory/codexFactory. tasks.md § 6.2 names the judgment against those members.
  3. Pure-function scenario inputs. The verdict is reproduced without firing the lane gave the reviewer only the pinned commit, the surfaces' commits and the declared state. From those alone the access, unreadable, parse and converged_with: outcomes cannot be reproduced. Its WHEN now gives the function's whole input, as § 5.2 defines it: the candidate's declaration as parsed (or its parse refusal), the plan's workflow members, and the read result (access outcome, resolved aggregation commit, and each surface's value or the failure that kept it unread).

Strict validation, the three front-matter validators and both doc-health families pass at d6787530.

sourcery-ai[bot]
sourcery-ai Bot previously approved these changes Sep 23, 2026

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sourcery assessment

Approved.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Unresolved issues affect outcome ordering, candidate and surface read failures, and commit validation semantics.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 High severity

Open (1)

…oth repositories, and a moved head no verdict

Copilot r4084891099 (review 5293871865): the requirement fetched the
candidate at the proposed head and gave a failure of that fetch, or of the
head's own lookup, no outcome, so "exactly one holds for any input" was
false for inputs the check meets on its own repository.

- The order now begins at the CANDIDATE UNREAD.
- One partition covers every failure to read, of either repository: a
  transient failure is UNDETERMINED; a 404 for a branch, a commit or a file
  is ABSENCE, the pin's failing as an absent declaration; every other
  failure is the check's own ACCESS and fails.
- A VERIFIED head is defined, and a read that finds another head publishes
  no verdict: the one run that publishes none.
- "Is a commit" is defined once, as the grammar.
- An eighth scenario. The five scenario inventories, D8, D9, D11, D14b,
  D14c, D16, D17, D20 and tasks 5.1d, 5.1e, 5.1g, 5.1i, 5.2 and 5.3 move
  with it, and D21 records the round.
- D8's asymmetry paragraph still counted two failing outcomes; it counts
  three.

Lane: openxfactory-4 (openXfactory-4-openDox_extraction)
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@brettheap

Copy link
Copy Markdown
Contributor Author

Lane: openxfactory-4 (openXfactory-4-openDox_extraction)

On the summary line of review 5293871865. It reads "Unresolved issues affect outcome ordering, candidate and surface read failures, and commit validation semantics" and carries one finding, r4084891099. I measured each theme against the text at d6787530, and each one held. All three were taken in b0f02bd9, and design.md D21 records them.

  • Outcome ordering. The order began at the declaration, which presupposes that the candidate is in hand. It now begins at the candidate unread.

  • Candidate and surface read failures. The failure classes were stated for the aggregation alone, and they did not partition even there:

    • a 422 fell under neither the ACCESS list nor the UNDETERMINED list;
    • a 403 on a surface's read could fall under either, depending on how "the aggregation repository itself" was read.

    One partition now covers every failure to read, of either repository. ACCESS is the residual class, because a response nobody anticipated is the one most likely to recur on every run.

  • Commit validation semantics. "Verified" was never defined, and "refused rather than reported" had no conclusion. Both are now defined. "Is a commit" is also defined once, as the grammar: the check never asks whether a commit of that name exists. D20 already said so; the requirement did not.

The sweep also found one stale statement that no review named. D8's asymmetry paragraph still counted two failing outcomes, under a heading and a table that count three. It is corrected in the same commit.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Unresolved critical and moderate specification issues remain, along with documentation inconsistencies.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 2 High severity · 1 Low severity

Open (3)
Resolved since last review (1)

Comment thread README.md
…is looked up by reading the pull request

Copilot r4085195340 (review 5294266328): "exactly one holds for any input"
was left unscoped beside the moved-head rule, which judges nothing. It now
reads "for any input a run judges".

Copilot r4085195422: a 404 from the head's lookup was already the check's
own access ("a 404 for the repository or the pull request itself"), but
nothing said that the head is looked up by reading the pull request. The
requirement now says so and separates that 404 from the pin's absence and
from a moved head, and tasks 5.1d lists its fixture. The 5.1d heading that
called access "not a read failure" now says it is never UNDETERMINED,
matching the partition.

D21 records the round, with r4085195461 measured stale: the description was
corrected at 17:10:30Z, after that review's run began at 17:08:42Z.

Lane: openxfactory-4 (openXfactory-4-openDox_extraction)
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🔵 Needs a closer look

Two moderate findings remain regarding access classification and branch-resolution inputs.

Review effort: Lite
Findings: None

Resolved since last review (3)

@sonarqubecloud

Copy link
Copy Markdown

@brettheap

Copy link
Copy Markdown
Contributor Author

Lane: openxfactory-4 (openXfactory-4-openDox_extraction)

On the summary line of review 5294384086 (at d05ce453: Findings: None, no thread, no suppressed or Previously missed block). It reads "Two moderate findings remain regarding access classification and branch-resolution inputs." Both classes have already had their two head-moving rounds on this pull request. Under the lane's rule, each is answered here by measurement and registered for the next act on spec.md, and the head stays at d05ce453.

Access classification: measured, and no defect found.

  • The partition is total and disjoint by construction. It sits in the spec paragraph that begins UNDETERMINED SHALL NOT BE A STANDING STATE:
    • a TRANSIENT failure is UNDETERMINED;
    • a 404 for a branch, a commit or a file is ABSENCE;
    • every other failure is the check's own ACCESS failing.
  • The two 404s that are not absences are named: "a 404 for the repository or the pull request itself". The head's lookup is stated to be a read of the pull request.
  • The order puts this repository's access failures in its first rung and the aggregation's in its third.
  • Every statement of the class agrees with that:
    • the access scenario ("either repository refusing its request … or any other failure that is neither transient nor an absence");
    • the ACCESS row of design.md D8;
    • tasks.md § 5.1d.

Branch-resolution inputs: one gap, registered.

  • The requirement says the check "SHALL name the branch and the commit with its verdict". The reproduction scenario's WHEN lists "the resolved aggregation commit" but not the branch.
  • tasks.md § 5.2, which specifies the function, already takes "the resolved aggregation branch and commit". A failed resolution reaches the function as the failure that kept each surface unread. So the realization is specified correctly, and the scenario's input list is one word short.
  • Registered for the next act on specs/review-lane-floor-mirror/spec.md: that WHEN should read "the resolved aggregation branch and commit".

@brettheap

Copy link
Copy Markdown
Contributor Author

Lane: openxfactory-4 (openXfactory-4-openDox_extraction)

READY #1145 @d05ce453

@brettheap

Copy link
Copy Markdown
Contributor Author

Lane: openxfactory-4 (openXfactory-4-openDox_extraction)

LANDING — lane openxfactory-4, session faabad8b-8c31-4958-abd5-b35bdc9cd282@Eagle, 2026-09-23T17:43:04Z, PR #1145 into opensoft/openxFactory main

@brettheap

Copy link
Copy Markdown
Contributor Author

Lane: openxfactory-4 (openXfactory-4-openDox_extraction)

LANDED — lane openxfactory-4, 2026-09-23T17:43:20Z, PR #1145 → 45ccaad (opensoft/openxFactory main; squash)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants