diff --git a/cmd/crw/main.go b/cmd/crw/main.go index a26723624a..dd19098011 100644 --- a/cmd/crw/main.go +++ b/cmd/crw/main.go @@ -37,6 +37,7 @@ import ( "github.com/thisisjun786/codex-relay-workflow/internal/relay/childcleanup" _ "github.com/thisisjun786/codex-relay-workflow/internal/relay/dag" _ "github.com/thisisjun786/codex-relay-workflow/internal/relay/dagsched" + _ "github.com/thisisjun786/codex-relay-workflow/internal/relay/integrate" _ "github.com/thisisjun786/codex-relay-workflow/internal/relay/managed" _ "github.com/thisisjun786/codex-relay-workflow/internal/relay/mergeturn" _ "github.com/thisisjun786/codex-relay-workflow/internal/relay/routing" diff --git a/contract/schema/relay-exit-codes.json b/contract/schema/relay-exit-codes.json index 74a8aba2f0..23bedc27b8 100644 --- a/contract/schema/relay-exit-codes.json +++ b/contract/schema/relay-exit-codes.json @@ -60,6 +60,7 @@ "CRITERIA_SET_CHANGED": "criteria_set_changed", "UNKNOWN_CRITERION": "unknown_criterion", "FINDINGS_REQUIRED": "findings_required", + "HEAD_NOT_RECEIPT_HEAD": "head_not_receipt_head", "DUPLICATE_ASSIGNMENT": "duplicate_assignment", "READBACK_MISMATCH": "readback_mismatch", "SYNC_NOT_CLAIMABLE": "sync_not_claimable", diff --git a/docs/relay/dag-scheduler.md b/docs/relay/dag-scheduler.md index 65e60f0278..76eb01cef5 100644 --- a/docs/relay/dag-scheduler.md +++ b/docs/relay/dag-scheduler.md @@ -547,6 +547,57 @@ observation for (a negative one included, so observing a subset never completes the same revision of the same generation (the head, generation and revision the acceptance stands on: after a [base refresh](#a-base-refresh-of-an-accepted-node) those the newest valid record names, which are the acceptance's own generation and revision when the parent refreshed the branch in its merge turn and a later generation's when the child did). The observation alone, or the mark alone, integrates nothing. A squashed or rebased landing leaves a head that is not an ancestor: when a merge turn recorded that it landed this head on the branch, the reading says `blocked:integration_unprovable` rather than guessing; without such a turn the node simply keeps waiting. Observations are append-only: an identical reading (same head, same tip, same answer) is a replay, and any change is the next observation. A paused or cancelled relationship records none (contract 3.2); observations already written stay. The targets are read from the plan as it stands when the observation is written, and the node's newest slot tenure is returned, by name, when it integrates. +## Commit acceptance and local integration (CRW-965) + +An implementation node needs no pull request. It is accepted by its commit: + +crw relay dag-accept --plan P --node N --actor A --commit HEAD --base BASE --checkout DIR --verification RECORD --rule-version ... + +The relay reads the receipt head of the node's current generation (dag_verified_heads) and refuses unless the commit is that head (head_not_receipt_head), descends from the base in the checkout at DIR (merge_base_mismatch), and the verification record is reusable for this commit (below). The same output again is a replay, and the --pull-request path is unchanged. The proof stands on the event, revision and generation read before the write transaction; the transaction reads the current head again and refuses a head that moved (stale_generation), so the head it stores is the one proved. + +A verification record is the one CRW-964's writer (crw-dev ci local) leaves: camelCase members (schema, runner, repository, baseCommit, headCommit, treeHash, ciDigest, tools, pins, goFlags, goEnv, pinMismatch, dependencies, os, arch, result, jobs, digest). The reader pins those names and recomputes the digest from the record's own bytes (sha256 of the sorted-key JSON without the digest member, no HTML escaping). A record is reusable only when every reuse key holds. A record that does not pass (result other than pass, a pinMismatch, a member absent, a digest that does not match, a ci.yml, dependency, base, os or arch that differs from the commit's and the host's) is disposition_conflict with its own detail. A record of another tree (treeHash) is revision_mismatch. A file that is not a verification-record/1 is malformed_receipt. headCommit is not a reuse key: the same tree under another commit is the same verification. + +crw relay dag-integrate --plan P --actor A --checkout DIR --integration-ref BRANCH --base BASE --verify COMMAND [--node N ...] [--expect-epoch E] + +is the one integration command. It is parent-only: --actor must be the parent of every candidate's relationship, and the coordinator epoch goes through the fence dag-accept uses (a stale epoch is refused as stale_coordinator_epoch). It takes the plan's ready candidates (or the named ones). A candidate is ready when its active acceptance is current: not stale under the scheduler's own judgment, its relationship active and on the generation the acceptance stands on, and its node live and not held by the plan. Before git runs, each candidate is frozen (an intent row in dag_integration_stages), so the marks name the acceptance and event the merge stood on. + +The batch merges the candidates in node-id order with merge commits in a temporary detached worktree of the integration branch (or of the base when the branch does not exist yet). It runs the --verify command in that worktree, split into arguments and never through a shell, with CRW_VERIFY_RECORD (a fresh path that did not exist before the run), CRW_VERIFY_BASE and CRW_VERIFY_HEAD (the merged commit) in its environment. The run passes only when the command exits 0, the record exists and the record passes the check above for the merged tree. A non-zero exit is a failure whatever the record says. The temporary worktree is removed on every path. + +When the merged tree fails, the batch bisects the ordered candidates to the first failing prefix. That candidate and every candidate that depends on it through the plan's edges are left out with the reason verification_failed (the candidate itself) or depends_on_. A candidate that did not merge cleanly is not judged failing: it is deferred, and once the surviving set is verified each deferred candidate is retried once, in order, on top of it. A retry that merges cleanly joins and the new merged tree is verified again; one that still conflicts stays out with the reason conflict_with_, and one that fails verification with verification_failed. A candidate that conflicted only with a removed candidate therefore integrates. + +The integration branch moves only to a tree whose record passed, by a compare-and-swap on the head the batch read: refs/heads/BRANCH is updated from that head, or created from the zero object id when the branch does not exist (create-only). A branch that moved under the batch is refused (stale_mark_context), and nothing is overwritten. After the move the batch row is written and each merged candidate gets the parent's merged mark, written from its frozen acceptance and event. A mark that cannot be written (a paused node, a changed acceptance) is recorded as mark_pending with the refusal, and the batch stays visible as ref_moved. The next dag-integrate run completes pending marks from the frozen rows, for a batch whose merge commit is an ancestor of the integration branch; a mark already present for that revision counts as done. + +crw relay dag-integrate-push --checkout DIR --integration-ref BRANCH --remote origin --remote-ref dev + +moves the remote branch to the integration commit by a fast-forward only. An equal remote is up_to_date. A remote that holds a commit the integration branch contains is advanced. A remote that holds a commit the integration branch does not contain is refused with merge_base_mismatch, and nothing is pushed; there is no force. A remote that cannot be reached (a GitHub outage) defers the push with outcome deferred and exit 0. The batch and the merged marks already stand, so run the same command again once the remote answers. + +Completion on this path is counted by observation, not by a merged pull request: dag-integration-observe --plan P --node N --actor A --target DIR@BRANCH reads the integration branch as the target, and the node is integrated when the commit is contained there and the merged mark exists. dag-progress then counts it as integrated, with no pull request link. The new path uses no merge turn, no merge train and no merge evidence. + +Verification command example. The command is split into arguments and run without a shell, so it cannot expand the variables itself. Give it a script that reads them from its environment, for example a file verify-merged.sh whose first line is #!/bin/sh and whose second line is: exec go run -tags dev ./cmd/crw-dev ci local --commit "$CRW_VERIFY_HEAD" --base "$CRW_VERIFY_BASE" --record "$CRW_VERIFY_RECORD" --runner local. Then pass --verify /absolute/path/to/verify-merged.sh. The batch runs the script in the merged worktree, so crw-dev ci local verifies the merged tree, and the record it writes is the one the batch judges. The tests use a stub script that exits 1 when a bad.txt marker is present in the worktree and writes its record through the same judge; they never run the real crw-dev. + +The relay judges a verification record by its reuse keys: the tree, the base, the ci.yml and dependency digests, the platform, the result, the pin mismatch list and the digest, and the tool values against the record's own pins. A tool that differs from its pin makes the record internally inconsistent and is refused; a tool without a pin is not judged. The Go flags and the Go environment stay the writer's to judge: the relay cannot know the environment of the host that wrote the record, so it only requires their members to be present, empty values included. CRW-964's writer refuses to write a reusable record on a pin mismatch, and its reuse mode compares the tools, the Go flags and the Go environment with the current environment before it answers. A batch reuses only its own covering PASS: a stored verification of a head stands for a candidate only while the tree, the ci.yml and dependency digests, the platform and the candidate's criteria set digest still match the ones it was judged under. Otherwise the batch runs the verify command again on that head, and the command may reuse a record through crw-dev ci local. + +A criteria change of a node is the scheduler's revalidation: the node's acceptance is stale until it is revalidated, and a candidate whose criteria changed is not ready until then. A node that already integrated is not a candidate and is never verified again by a batch; its changed criteria make it stale (stale:criteria_changed) until its parent re-rules it, which dag-progress shows. The verification criteria are the keys of the record judge: the ci.yml digest, the dependency digests, the pins and the platform. A branch head that a batch marks from a verified head is covered only while the keys that verification was judged under still hold for the head's tree. When they no longer hold, the batch verifies the head again without a merge, and only that PASS covers the candidates, so a batch whose candidates are all already contained runs the verifier exactly when those keys changed. Every frozen candidate row of a batch names the criteria set the batch verified the candidate under. An explicit --node that names a node already integrated is refused disposition_conflict, with the reason that the node is already integrated. + +An accepted implementation node satisfies a code-pinned artifact_verified edge through either of its two acceptance shapes: a pull-request acceptance (its forge row and pull request number) or a commit acceptance (its verification record row). A commit-accepted node completes on the local integration branch its batches moved. Its outgoing integrated and code-pinned edges must name the same checkout, compared as written, but the edge's base ref is not part of what integrated means for it: the completion targets are the checkout and the integration ref each batch recorded, and dag-integrate names that ref in its targets output. A pull-request node keeps the edge's base ref. A node with neither acceptance row stays acceptance_incomplete. + +A replay of a commit acceptance is the same output: a record for the same head and tree replays the acceptance or revalidates it, whatever its bytes (job timings, for example). The relay judges the supplied record with the same judge and compares the verified head and tree with the stored ones; the first record stays the evidence. A record naming another head or tree is a different output and is refused. + +A batch moves the branch in one fenced transaction: the coordinator epoch is checked, the compare-and-swap runs and the moved record is written before the commit. The verification runs outside that transaction, and the planned intent (the verified head, its tree and its keys) is committed before the swap. If the process dies after the swap, the next run reconciles a planned intent whose head the branch holds and that has no moved record: it records the move once and makes no second move. A planned intent whose head the branch does not hold is reported as abandoned. A push that GitHub answers with an HTTP 500, 502, 503 or 504 status is an outage: the push is deferred, and the local integration stands. + +**The judgement boundary of a record (D1).** The judge compares the record with the reuse keys above and each tool value with the pin the record carries for that tool. It does not compare goFlags, goEnv or the tool values with the relay host, because the relay cannot read the environment the writer ran in: those are the writer's judgement (CRW-964 refuses to write a reusable record on a pin mismatch, and its `--reuse` compares them with the current environment). A batch reuses only its own covering PASS: a stored verification of a head covers a candidate only while the tree, the ci.yml and dependency digests, the platform and the candidate's criteria set digest still match the ones it was judged under. A candidate whose criteria set changed after a revalidation is verified again on its head, with no merge. + +**What a batch verifies (criterion c2).** A change of a node's criteria is revalidation, which the scheduler's staleness already handles: a node that integrated is not merged or verified again, and a candidate whose criteria changed is not ready until it is revalidated, after which the next batch verifies it afresh. The integration branch is not re-verified for a criteria change, because its tree did not change. The verification criteria are the ci.yml digest, the dependency digests and the platform. A head a batch recorded as verified stands only while those keys hold for its tree (the stored row carries the tree, the digests, `os` and `arch`); when a key no longer holds, the batch verifies that head again on a worktree of it, with no merge, and only that PASS covers the candidates it contains. + +**Currency at the swap (D3).** Each merged candidate is proved current again inside the fenced transaction that moves the branch, by the same predicate candidate selection uses: a live node, the active acceptance on the frozen head, an active relationship on the acceptance's generation, and no staleness. A candidate that is no longer current refuses the batch with `merge_candidate_moved`: the branch does not move and nothing is marked, and the next batch freezes again without that candidate and verifies again. + +**Bisection and its limitation (D5).** When the merged set fails, the batch finds the first failing prefix by bisection. This assumes that a prefix which fails keeps failing when more candidates follow. When a later candidate repairs an interaction, an innocent candidate can be left out of this batch; that is safe, because only a PASS moves the branch, and it costs one more batch. A candidate the bisection leaves out is reported as `not_proven_failing` unless it fails when verified alone (`verification_failed`), and it keeps its ready state with no mark. + +**Completion targets (D4).** A commit-accepted node completes on the integration ref that a batch with a moved record has moved on its checkout. One function gives those refs to the scheduler's edges and to the observation. A planned, abandoned, deferred or failed intent adds no target, and the edge's base ref is not part of it. + +**The push (D6).** `dag-integrate-push` reads the relay store and pushes only a tip the relay moved the branch onto: a batch with a moved record whose new head is the tip and whose verification passed. A commit added on top of such a move, or a branch the relay did not verify, is refused with `merge_base_mismatch` before the remote is read, and the remote keeps its commit. The push stays fast-forward only, with no force, and a GitHub outage defers it. + + ## A base refresh of an accepted node The parent accepted a node's result and the base moved afterwards, so the pull request cannot land as it stands. `dag-base-refresh` is the route out of both shapes of that: it records that the acceptance also stands on a head the base was merged into, after the relay has proved from git that the head is the accepted head plus merges of the base branch and nothing else. Which generation the record names depends on who refreshed the branch. diff --git a/internal/contract/exit_codes_generated.go b/internal/contract/exit_codes_generated.go index 7deb904fd8..419de552d0 100644 --- a/internal/contract/exit_codes_generated.go +++ b/internal/contract/exit_codes_generated.go @@ -51,6 +51,7 @@ const ( RefusalForeignScope RefusalReason = "foreign_scope" RefusalHandoverUnconfirmed RefusalReason = "handover_unconfirmed" RefusalHandoverWouldStrand RefusalReason = "handover_would_strand" + RefusalHeadNotReceiptHead RefusalReason = "head_not_receipt_head" RefusalInsufficientPathBinding RefusalReason = "insufficient_path_binding" RefusalLinkConflict RefusalReason = "link_conflict" RefusalLinkNotActive RefusalReason = "link_not_active" diff --git a/internal/contracttest/cli.go b/internal/contracttest/cli.go index c7cb88cc01..b24225a978 100644 --- a/internal/contracttest/cli.go +++ b/internal/contracttest/cli.go @@ -23,6 +23,7 @@ import ( _ "github.com/thisisjun786/codex-relay-workflow/internal/relay/cli" _ "github.com/thisisjun786/codex-relay-workflow/internal/relay/dag" _ "github.com/thisisjun786/codex-relay-workflow/internal/relay/dagsched" + _ "github.com/thisisjun786/codex-relay-workflow/internal/relay/integrate" _ "github.com/thisisjun786/codex-relay-workflow/internal/relay/managed" _ "github.com/thisisjun786/codex-relay-workflow/internal/relay/mergeturn" _ "github.com/thisisjun786/codex-relay-workflow/internal/relay/routing" diff --git a/internal/contracttest/dag_integrate_cli_test.go b/internal/contracttest/dag_integrate_cli_test.go new file mode 100644 index 0000000000..26f5aa44aa --- /dev/null +++ b/internal/contracttest/dag_integrate_cli_test.go @@ -0,0 +1,11 @@ +package contracttest + +import "testing" + +// The commit-acceptance and integration commands (CRW-965) are proved against the built crw, every case of +// testdata/fixtures/dag-integrate-cli-cases.json replayed through `crw relay`: each prints the stdout bytes and exit code its +// golden holds. The cases cover each new command's usage and absent-store refusal, the refusal family a host without a +// store answers before any git or verification runs. +func TestDagIntegrateCommands_the_built_crw_prints_its_frozen_answers(t *testing.T) { + replayCLICases(t, "dag-integrate-cli-cases.json", false, []string{"dag-integrate", "dag-integrate-push", "dag-integration-observe", "dag-accept"}) +} diff --git a/internal/contracttest/testdata/fixtures/dag-integrate-cli-cases.json b/internal/contracttest/testdata/fixtures/dag-integrate-cli-cases.json new file mode 100644 index 0000000000..8a82a3987b --- /dev/null +++ b/internal/contracttest/testdata/fixtures/dag-integrate-cli-cases.json @@ -0,0 +1,98 @@ +{ + "integrate-missing-plan": [ + { + "argv": [ + "dag-integrate", + "--actor", + "parent", + "--checkout", + "/nonexistent/checkout", + "--integration-ref", + "dev-int", + "--base", + "dev", + "--verify", + "true" + ] + } + ], + "integrate-absent-store": [ + { + "argv": [ + "dag-integrate", + "--plan", + "p", + "--actor", + "parent", + "--checkout", + "/nonexistent/checkout", + "--integration-ref", + "dev-int", + "--base", + "dev", + "--verify", + "true" + ] + } + ], + "push-missing-checkout": [ + { + "argv": [ + "dag-integrate-push", + "--checkout", + "/nonexistent/checkout", + "--integration-ref", + "dev-int", + "--remote", + "origin", + "--remote-ref", + "dev" + ] + } + ], + "push-missing-remote": [ + { + "argv": [ + "dag-integrate-push", + "--checkout", + "/nonexistent/checkout", + "--integration-ref", + "dev-int", + "--remote-ref", + "dev" + ] + } + ], + "observe-absent-store": [ + { + "argv": [ + "dag-integration-observe", + "--plan", + "p", + "--node", + "a", + "--actor", + "parent" + ] + } + ], + "accept-commit-absent-store": [ + { + "argv": [ + "dag-accept", + "--plan", + "p", + "--node", + "a", + "--actor", + "parent", + "--commit", + "0123456789abcdef0123456789abcdef01234567", + "--base", + "0123456789abcdef0123456789abcdef01234567", + "--verification", + "/nonexistent/record.json" + ] + } + ] +} diff --git a/internal/contracttest/testdata/golden/TestDagIntegrateCommands_the_built_crw_prints_its_frozen_answers__accept-commit-absent-store-aaf20ffe7790.json.gz b/internal/contracttest/testdata/golden/TestDagIntegrateCommands_the_built_crw_prints_its_frozen_answers__accept-commit-absent-store-aaf20ffe7790.json.gz new file mode 100644 index 0000000000..4f3ca274c5 Binary files /dev/null and b/internal/contracttest/testdata/golden/TestDagIntegrateCommands_the_built_crw_prints_its_frozen_answers__accept-commit-absent-store-aaf20ffe7790.json.gz differ diff --git a/internal/contracttest/testdata/golden/TestDagIntegrateCommands_the_built_crw_prints_its_frozen_answers__integrate-absent-store-77e0744b6275.json.gz b/internal/contracttest/testdata/golden/TestDagIntegrateCommands_the_built_crw_prints_its_frozen_answers__integrate-absent-store-77e0744b6275.json.gz new file mode 100644 index 0000000000..771326416a Binary files /dev/null and b/internal/contracttest/testdata/golden/TestDagIntegrateCommands_the_built_crw_prints_its_frozen_answers__integrate-absent-store-77e0744b6275.json.gz differ diff --git a/internal/contracttest/testdata/golden/TestDagIntegrateCommands_the_built_crw_prints_its_frozen_answers__integrate-missing-plan-85bf4eb2e73f.json.gz b/internal/contracttest/testdata/golden/TestDagIntegrateCommands_the_built_crw_prints_its_frozen_answers__integrate-missing-plan-85bf4eb2e73f.json.gz new file mode 100644 index 0000000000..4f3ca274c5 Binary files /dev/null and b/internal/contracttest/testdata/golden/TestDagIntegrateCommands_the_built_crw_prints_its_frozen_answers__integrate-missing-plan-85bf4eb2e73f.json.gz differ diff --git a/internal/contracttest/testdata/golden/TestDagIntegrateCommands_the_built_crw_prints_its_frozen_answers__observe-absent-store-be0abf6ca1fa.json.gz b/internal/contracttest/testdata/golden/TestDagIntegrateCommands_the_built_crw_prints_its_frozen_answers__observe-absent-store-be0abf6ca1fa.json.gz new file mode 100644 index 0000000000..5078cbd76b Binary files /dev/null and b/internal/contracttest/testdata/golden/TestDagIntegrateCommands_the_built_crw_prints_its_frozen_answers__observe-absent-store-be0abf6ca1fa.json.gz differ diff --git a/internal/contracttest/testdata/golden/TestDagIntegrateCommands_the_built_crw_prints_its_frozen_answers__push-missing-checkout-a10bc8848edc.json.gz b/internal/contracttest/testdata/golden/TestDagIntegrateCommands_the_built_crw_prints_its_frozen_answers__push-missing-checkout-a10bc8848edc.json.gz new file mode 100644 index 0000000000..7e8ace5dad Binary files /dev/null and b/internal/contracttest/testdata/golden/TestDagIntegrateCommands_the_built_crw_prints_its_frozen_answers__push-missing-checkout-a10bc8848edc.json.gz differ diff --git a/internal/contracttest/testdata/golden/TestDagIntegrateCommands_the_built_crw_prints_its_frozen_answers__push-missing-remote-ac1fa83d2a4c.json.gz b/internal/contracttest/testdata/golden/TestDagIntegrateCommands_the_built_crw_prints_its_frozen_answers__push-missing-remote-ac1fa83d2a4c.json.gz new file mode 100644 index 0000000000..4f3ca274c5 Binary files /dev/null and b/internal/contracttest/testdata/golden/TestDagIntegrateCommands_the_built_crw_prints_its_frozen_answers__push-missing-remote-ac1fa83d2a4c.json.gz differ diff --git a/internal/relay/argparse/specs.json b/internal/relay/argparse/specs.json index 906b8a9b7a..59be2aa7af 100644 --- a/internal/relay/argparse/specs.json +++ b/internal/relay/argparse/specs.json @@ -907,6 +907,10 @@ {"flag":"--repository","help":"an implementation node's pull request repository (owner/name)"}, {"flag":"--pull-request","type":"int","help":"an implementation node's pull request number"}, {"flag":"--supersedes","help":"the active acceptance of another output this one replaces"}, + {"flag":"--commit","help":"an implementation node accepted without a pull request: the commit that is the receipt head of its generation (full lower-case hex). Needs --base, --checkout and --verification; not with --repository or --pull-request"}, + {"flag":"--base","help":"with --commit: the commit the head must descend from"}, + {"flag":"--checkout","help":"with --commit: an absolute path to the local checkout that holds both commits"}, + {"flag":"--verification","help":"with --commit: the verification-record/1 document (JSON or @file) that names this commit tree with a PASS it may reuse"}, {"flag":"--rule-version","required":true,"help":"the rules the result was verified under, {skills_digest, model, effort} as JSON or @file"}, {"flag":"--expect-epoch","type":"int","help":"the coordinator epoch this session holds (dag-coordinator-claim answers it); a write from another epoch is refused stale_coordinator_epoch. 0 names none, which only a plan nobody claimed takes"}]}, {"name":"dag-integration-observe","summary":"records whether an accepted implementation node's head is contained in the branches it has to land on, read by the relay itself (git merge-base --is-ancestor for a local checkout, the compare API for a forge repository). Integration needs every required target to contain the head and the parent's merged mark on the same revision; the node's slot is returned when it does. A landing also runs the conflict sweep it owes (conflict_sweep in the answer), once per landing","options":[ @@ -915,6 +919,20 @@ {"flag":"--actor","required":true}, {"flag":"--target","kind":"append","help":"repository@ref to observe (repeatable); the targets of the node's outgoing edges by default. A terminal node needs it"}, {"flag":"--expect-epoch","type":"int","help":"the coordinator epoch this session holds (dag-coordinator-claim answers it); a write from another epoch is refused stale_coordinator_epoch. 0 names none, which only a plan nobody claimed takes"}]}, +{"name":"dag-integrate","summary":"merges the plan's ready accepted implementation candidates (or the named --node ones) one after another onto a local integration branch with merge commits, runs one verification (--verify, split into arguments, never a shell) on the merged tree, and moves the branch only to a tree whose verification record is a PASS. A candidate whose merge conflicts or whose verification breaks is left out and returned in split. The branch moves by a compare-and-swap on its old head, so a head that moved under the command is refused. On success the parent's merged mark is recorded for each merged candidate; integration is then observed by dag-integration-observe --target on the integration branch","options":[ + {"flag":"--plan","required":true}, + {"flag":"--actor","required":true,"help":"the parent running the batch"}, + {"flag":"--checkout","required":true,"help":"an absolute path to the local checkout that holds the candidates' commits and the integration branch"}, + {"flag":"--integration-ref","required":true,"help":"the local branch the batch merges onto"}, + {"flag":"--base","required":true,"help":"the branch the candidates descend from, read in the same checkout"}, + {"flag":"--verify","required":true,"help":"the one verification run on the merged tree, a command split into arguments (no shell); it runs in the merged worktree with CRW_VERIFY_RECORD (a fresh path to write a verification-record/1 to), CRW_VERIFY_BASE and CRW_VERIFY_HEAD in its environment; a non-zero exit is a failure"}, + {"flag":"--node","kind":"append","help":"a candidate to consider (repeatable); none means every ready accepted candidate of the plan"}, + {"flag":"--expect-epoch","type":"int","help":"the coordinator epoch this session holds (dag-coordinator-claim answers it); a write from another epoch is refused stale_coordinator_epoch. 0 names none, which only a plan nobody claimed takes"}]}, +{"name":"dag-integrate-push","summary":"moves the remote branch to the integration branch's commit by a fast-forward only: nothing is pushed when the remote already holds that commit (up_to_date), when the remote holds a commit the integration branch does not contain (refused merge_base_mismatch), or never by force. When the remote cannot be reached the push is deferred (outcome deferred, exit 0) and the integration recorded by dag-integrate stands; run this command again later to make the push","options":[ + {"flag":"--checkout","required":true,"help":"an absolute path to the local checkout that holds the integration branch"}, + {"flag":"--integration-ref","required":true,"help":"the local integration branch whose commit is pushed"}, + {"flag":"--remote","required":true,"help":"the remote name, for example origin"}, + {"flag":"--remote-ref","required":true,"help":"the remote branch to fast-forward, for example dev"}]}, {"name":"dag-base-refresh","summary":"records that an accepted implementation node's acceptance also stands on a later generation of its relationship that only refreshed the base: the relay reads the pull request head from the forge and the base tip from the target, and proves in a local checkout that the head is the accepted head plus merges of the base branch (each merge a two-parent commit whose tree is the tree git merges from its parents; a merge git cannot do cleanly is accepted only when the commit differs from git's tree in the files git could not merge, and files not proved by the node's declared mechanical rule must be named exactly with --resolved). The acceptance is not replaced; integration is then judged on the refreshed head and the merged mark of that generation. A stale node, a landed node and a head that holds anything but base merges are refused","options":[ {"flag":"--plan","required":true}, {"flag":"--node","required":true}, diff --git a/internal/relay/dagsched/accept.go b/internal/relay/dagsched/accept.go index 8161acd924..681b8e9ae5 100644 --- a/internal/relay/dagsched/accept.go +++ b/internal/relay/dagsched/accept.go @@ -29,8 +29,14 @@ type VerifierRule struct { // AcceptInput is what a parent supplies to accept a node's result. Nothing in it names a head: for an implementation node the head is the one the relay reads from the forge, so a child's // statement can never become the accepted head (P-AV-2). type AcceptInput struct { - Event string // the event the parent verified; the current head of the generation when empty - PullRequest *PRRef // required for an implementation node, forbidden for a non_pr node + Event string // the event the parent verified; the current head of the generation when empty + PullRequest *PRRef // required for an implementation node, forbidden for a non_pr node + // Commit is the pull-request-less half (CRW-965): the commit to accept, the base it descends from, + // the local checkout that holds both, and the verification-record/1 the parent supplied. It is + // required for an implementation node accepted without a pull request and forbidden together with + // PullRequest. Every check and write it needs lives in accept_commit.go; the guards below only pick + // the path, so the pull-request path keeps its own reading and its own writes. + Commit *CommitRef Supersedes string // the acceptance this one replaces, when the node already has an active acceptance of another output RuleVersion VerifierRule // every field required } @@ -218,8 +224,16 @@ func (s *Scheduler) accept(ctx context.Context, plan, node, actor string, in Acc return out, err } implementation := n.Kind == dag.NodeImplementation + commitPath := in.Commit != nil + if commitPath && !implementation { + return out, refuse(contract.RefusalMalformedReceipt, "node %s is a %s node: it has no commit to accept", node, n.Kind) + } + if commitPath && in.PullRequest != nil { + return out, refuse(contract.RefusalMalformedReceipt, "an acceptance names either a pull request or a commit, never both") + } var pr PullRequest - if implementation { + var prepared acceptCommitInput + if implementation && !commitPath { if in.PullRequest == nil || in.PullRequest.Repository == "" || in.PullRequest.Number < 1 { return out, refuse(contract.RefusalMalformedReceipt, "an implementation node is accepted on its pull request: name its repository (owner/name) and number") } @@ -233,6 +247,26 @@ func (s *Scheduler) accept(ctx context.Context, plan, node, actor string, in Acc return out, err } out.prRepository, out.baseRef = in.PullRequest.Repository, pr.BaseRef + } else if commitPath { + // the commit path reads what it needs before its write transaction, as the pull-request path reads + // its forge before it: the relationship and the head event, then the checks in the checkout + rel, found, err := currentRelationshipOf(ctx, q, plan, node) + if err != nil { + return out, err + } + if !found { + return out, refuse(contract.RefusalUnregisteredRelationship, "node %s has no execution to accept", node) + } + head, err := s.verifiedHead(ctx, q, rel, in.Event) + if err != nil { + return out, err + } + if head.SetDigest != n.CriteriaSetDigest { + return out, refuse(contract.RefusalCriteriaSetChanged, "the output was ruled against criteria %s and the plan fixed %s for %s", head.SetDigest, n.CriteriaSetDigest, node) + } + if prepared, err = s.prepareCommitAcceptance(ctx, *in.Commit, head, rel); err != nil { + return out, err + } } else if in.PullRequest != nil { return out, refuse(contract.RefusalMalformedReceipt, "node %s is a %s node: it has no pull request to name", node, n.Kind) } @@ -290,6 +324,11 @@ func (s *Scheduler) accept(ctx context.Context, plan, node, actor string, in Acc return refuse(contract.RefusalCriteriaSetChanged, "the output was ruled against criteria %s and the plan fixed %s for %s", head.SetDigest, n.CriteriaSetDigest, node) } out.RelationshipID, out.Generation = rel.ID, rel.Generation + if commitPath && (head.EventID != prepared.event || head.RevisionHash != prepared.revision || rel.Generation != prepared.generation) { + // the proof stood on the event, revision and generation read before the transaction; a head that moved since is + // refused, never stored under the old proof (finding d9) + return refuse(contract.RefusalStaleGeneration, "the current head of generation %d changed while the commit was checked (event %s, checked %s): read the assignment again", rel.Generation, head.EventID, prepared.event) + } // the same output accepted before: a replay, or the same acceptance re-ruled under re-registered criteria var existing, state, ownDigest string @@ -321,9 +360,22 @@ func (s *Scheduler) accept(ctx context.Context, plan, node, actor string, in Acc return err } if implementation { - // the output is accepted already, on the pull request and at the head recorded then: a call that reads another pull request, or the same one at another head, is not a replay of it - if err := s.sameForgeReading(txCtx, tx, a, *in.PullRequest, pr); err != nil { - return err + if commitPath { + // the same output accepted before on the commit path: the replay is the same head with + // the same verification record. A call naming another record is not a replay of it. + // the same output is the same head and tree: a fresh record of that tree is a replay, and its bytes are not compared (CRW-965, D3) + stored, found, err := storedVerificationIdentity(txCtx, tx, a.AcceptanceID) + if err != nil { + return err + } + if !found || stored.Head != prepared.ref.Head || stored.Tree != prepared.tree { + return refuse(contract.RefusalDispositionConflict, "the output of %s was accepted with another verification record; accept a new output instead", node) + } + } else { + // the output is accepted already, on the pull request and at the head recorded then: a call that reads another pull request, or the same one at another head, is not a replay of it + if err := s.sameForgeReading(txCtx, tx, a, *in.PullRequest, pr); err != nil { + return err + } } } effective, err := effectiveCriteria(txCtx, tx, a) @@ -358,7 +410,7 @@ func (s *Scheduler) accept(ctx context.Context, plan, node, actor string, in Acc return refuse(contract.RefusalStaleGeneration, "generation %d of %s is not recorded as an execution of %s (a correction is recorded with dag-correct before its result is accepted)%s", rel.Generation, rel.ID, node, s.unsentGenerationHint(txCtx, tx, rel)) } // a new output: it supersedes the node's active acceptance only explicitly - if implementation && (pr.State != "open" || pr.IsDraft) { + if implementation && !commitPath && (pr.State != "open" || pr.IsDraft) { return refuse(contract.RefusalDispositionConflict, "pull request %s#%d is %s%s: only an open, non-draft pull request is accepted", pr.Repository, pr.Number, pr.State, map[bool]string{true: " and a draft", false: ""}[pr.IsDraft]) } var activeID string @@ -379,11 +431,21 @@ func (s *Scheduler) accept(ctx context.Context, plan, node, actor string, in Acc CriteriaSetDigest: head.SetDigest, Verdict: "verified", OutputManifestRef: head.ManifestRef, AckTier: head.AckTier, VerdictTurnID: head.VerdictTurn, RuleVersionJSON: ruleJSON, AcceptedByTask: actor, CoordinatorEpoch: s.ExpectedEpoch, AcceptedAt: s.now(), SupersedesAcceptanceID: activeID, State: "active"} if implementation { - repository, err := s.acceptTarget(txCtx, tx, current, node, in.PullRequest.Repository) - if err != nil { - return err + if commitPath { + // the commit path's target is the local checkout the caller named; its outgoing edges have + // to name the same path, compared as written + repository, err := s.acceptCommitTarget(txCtx, tx, current, node, in.Commit.Checkout) + if err != nil { + return err + } + a.HeadSHA, a.Repository, a.EvidenceDigest = prepared.ref.Head, repository, shaOf(prepared.recordRaw) + } else { + repository, err := s.acceptTarget(txCtx, tx, current, node, in.PullRequest.Repository) + if err != nil { + return err + } + a.HeadSHA, a.Repository, a.PRNumber, a.EvidenceDigest = pr.HeadSHA, repository, pr.Number, EvidenceDigest(EvidenceBodyOf(pr)) } - a.HeadSHA, a.Repository, a.PRNumber, a.EvidenceDigest = pr.HeadSHA, repository, pr.Number, EvidenceDigest(EvidenceBodyOf(pr)) } a.AcceptanceID = AcceptanceDigest(a) var supersedes, ref, head2, repo2, evidence2, pr2 any @@ -403,9 +465,15 @@ func (s *Scheduler) accept(ctx context.Context, plan, node, actor string, in Acc a.AckTier, a.VerdictTurnID, a.RuleVersionJSON, a.AcceptedByTask, a.CoordinatorEpoch, a.AcceptedAt, supersedes, a.State); err != nil { return err } - // the acceptance row first: dag_acceptance_forge references it with an immediate foreign key + // the acceptance row first: dag_acceptance_forge and dag_acceptance_verifications both reference it with an immediate foreign key if implementation { - if _, err := tx.ExecContext(txCtx, "INSERT INTO dag_acceptance_forge (acceptance_id, forge_repository, pr_number) VALUES (?,?,?)", a.AcceptanceID, in.PullRequest.Repository, pr.Number); err != nil { + if commitPath { + // the verification record travels with the acceptance it justifies, so the replay identity + // and the batch's reuse rule have a readable source + if err := s.storeCommitVerification(txCtx, a.AcceptanceID, actor, prepared, s.ExpectedEpoch); err != nil { + return err + } + } else if _, err := tx.ExecContext(txCtx, "INSERT INTO dag_acceptance_forge (acceptance_id, forge_repository, pr_number) VALUES (?,?,?)", a.AcceptanceID, in.PullRequest.Repository, pr.Number); err != nil { return err } } else if out.SlotReleased, err = s.releaseSlot(txCtx, tx, plan, node, actor, "dag_accepted"); err != nil { diff --git a/internal/relay/dagsched/accept_commit.go b/internal/relay/dagsched/accept_commit.go new file mode 100644 index 0000000000..48ae701532 --- /dev/null +++ b/internal/relay/dagsched/accept_commit.go @@ -0,0 +1,185 @@ +package dagsched + +import ( + "context" + "os" + "path/filepath" + "runtime" + "strings" + + "github.com/thisisjun786/codex-relay-workflow/internal/contract" + "github.com/thisisjun786/codex-relay-workflow/internal/relay/acceptance" + "github.com/thisisjun786/codex-relay-workflow/internal/relay/dag" + "github.com/thisisjun786/codex-relay-workflow/internal/relay/store" +) + +// The pull-request-less acceptance path (CRW-965). An implementation node whose result is a commit is +// accepted with --commit --base --verification --checkout : the relay +// proves in the local checkout that the commit is the head the generation's ruling fixed +// (dag_verified_heads.head_sha, read through acceptance.RulingHead), that it descends from the base, +// and that the verification record names this commit's tree with a PASS it may reuse. Every failure is +// refused with its own reason. The pull-request path is untouched: this path never reads a forge, never +// calls acceptTarget and writes no dag_acceptance_forge row, and that absence is what tells the two +// paths apart. + +// CommitRef is the pull-request-less half of an acceptance input: the commit to accept, the base it +// must descend from, the local checkout that holds both, and the verification-record/1 the parent +// supplied. +type CommitRef struct { + Head string + Base string + Checkout string + Record string +} + +// VerificationRecordSchema is the schema tag a verification record must carry. +const VerificationRecordSchema = "verification-record/1" + +func readVerificationRecord(input string) ([]byte, error) { + if !strings.HasPrefix(input, "@") { + if err := store.EncodeUTF8(input); err != nil { + return nil, usage("--verification is not valid UTF-8: " + err.Error()) + } + return []byte(input), nil + } + raw, err := os.ReadFile(input[1:]) + if err != nil { + return nil, usage("--verification names a file that cannot be read: " + err.Error()) + } + return raw, nil +} + +// gitTreeOf reads the tree object of a commit in a checkout, through the clean-environment git helper + +// commitIsAncestor answers whether ancestor is an ancestor of descendant in the checkout. +func commitIsAncestor(ctx context.Context, checkout, ancestor, descendant string) (bool, error) { + // merge-base --is-ancestor answers with its exit status: 0 contained, 1 not contained, anything + // else a read failure that must not become an answer. + code, _, err := runGitExit(ctx, checkout, nil, "merge-base", "--is-ancestor", ancestor, descendant) + switch { + case code == 0: + return true, nil + case code == 1: + return false, nil + } + return false, err +} + +// acceptCommitInput is the read side of the commit path: everything the checks produce, read before the write +// transaction opens. The event, revision and generation are what the proof stood on; the transaction compares them +// with the current head again (finding d9), and the head it stores is the one proved here. +type acceptCommitInput struct { + ref CommitRef + recordRaw []byte + record VerificationRecord + tree string + event string + revision string + generation int64 +} + +// prepareCommitAcceptance runs every check the commit path makes outside the write transaction: the record is +// readable and judged reusable for this commit's tree, base, ci.yml, dependencies and host (JudgeVerificationRecord), +// the commit is the head the generation's ruling fixed, and it descends from the base. A failure here writes nothing. +func (s *Scheduler) prepareCommitAcceptance(ctx context.Context, in CommitRef, head verifiedHead, rel relRow) (acceptCommitInput, error) { + out := acceptCommitInput{ref: in, event: head.EventID, revision: head.RevisionHash, generation: rel.Generation} + if in.Head == "" || in.Base == "" || in.Checkout == "" || in.Record == "" { + return out, refuse(contract.RefusalMalformedReceipt, "a pull-request-less acceptance names --commit, --base, --verification and --checkout") + } + if !refreshCommitPattern.MatchString(in.Head) || !refreshCommitPattern.MatchString(in.Base) { + return out, refuse(contract.RefusalMalformedReceipt, "--commit and --base are full commit ids, not %q and %q", in.Head, in.Base) + } + if !filepath.IsAbs(in.Checkout) { + return out, refuse(contract.RefusalMalformedReceipt, "--checkout is an absolute path to a local checkout, not %q", in.Checkout) + } + raw, err := readVerificationRecord(in.Record) + if err != nil { + return out, err + } + out.recordRaw = raw + + // the commit has to be the head the generation's ruling fixed. The event's own revision hash is the artifact + // manifest digest, not a commit, so the commit comes from dag_verified_heads. + q := s.Store.Q(ctx) + ruled, source, err := acceptance.RulingHead(ctx, q, head.EventID, "") + if err != nil { + return out, err + } + if source != "verified_heads" || ruled == "" { + return out, refuse(contract.RefusalHeadNotReceiptHead, "the ruling on event %s fixed no head commit, so no commit can be the head of that generation", head.EventID) + } + if !strings.EqualFold(ruled, in.Head) { + return out, refuse(contract.RefusalHeadNotReceiptHead, "the ruling on event %s fixed head %s and the call names %s", head.EventID, ruled, in.Head) + } + // the commit descends from the base + if ancestor, err := commitIsAncestor(ctx, in.Checkout, in.Base, in.Head); err != nil { + return out, refuse(contract.RefusalMergeTargetUnreadable, "git could not read %s: %v", in.Checkout, err) + } else if !ancestor { + return out, refuse(contract.RefusalMergeBaseMismatch, "%s is not an ancestor of %s in %s: the commit does not descend from the base", in.Base, in.Head, in.Checkout) + } + base, err := runGit(ctx, in.Checkout, nil, "rev-parse", "--verify", in.Base+"^{commit}") + if err != nil { + return out, refuse(contract.RefusalMergeTargetUnreadable, "git could not resolve the base %s in %s: %v", in.Base, in.Checkout, err) + } + keys, err := CommitVerificationKeys(ctx, in.Checkout, in.Head) + if err != nil { + return out, err + } + keys.Base = strings.TrimSpace(base) + keys.OS, keys.Arch = runtime.GOOS, runtime.GOARCH + record, err := JudgeVerificationRecord(raw, keys) + if err != nil { + return out, err + } + out.tree, out.record = keys.Tree, record + return out, nil +} + +// storeCommitVerification appends the verification record with the acceptance, inside the acceptance's +// own transaction (txCtx is the composing transaction's context), so the record and the acceptance it +// justifies are one durable effect. +func (s *Scheduler) storeCommitVerification(txCtx context.Context, acceptanceID, actor string, prepared acceptCommitInput, epoch int64) error { + return store.RecordAcceptanceVerification(txCtx, s.Store, store.AcceptanceVerificationRow{ + AcceptanceID: acceptanceID, RecordDigest: shaOf(prepared.recordRaw), RecordJSON: string(prepared.recordRaw), + HeadCommit: prepared.ref.Head, TreeSHA: prepared.tree, BaseCommit: prepared.ref.Base, + RecordedBy: actor, CoordinatorEpoch: epoch, RecordedAt: s.now(), + }) +} + +// storedVerification is the head and tree a commit acceptance's verification row stores (CRW-965, D3). +type storedVerification struct{ Head, Tree string } + +// storedVerificationIdentity reads the head and tree recorded with an acceptance. found is false when the acceptance +// has no verification row, which is what tells a pull-request acceptance from a commit one. +func storedVerificationIdentity(ctx context.Context, q store.Querier, acceptanceID string) (storedVerification, bool, error) { + var v storedVerification + found, err := queryOne(ctx, q, "SELECT head_commit, tree_sha FROM dag_acceptance_verifications WHERE acceptance_id = ?", []any{acceptanceID}, &v.Head, &v.Tree) + return v, found, err +} + +// acceptCommitTarget is the repository a pull-request-less acceptance is judged against: the local +// checkout the caller named. The node's outgoing integrated and code-pinned edges have to name that +// same checkout, compared as written; a node with no such edge takes the checkout. This is the one +// deliberate departure from "an implementation node is accepted on its pull request's forge +// repository" (CRW-965): without a pull request there is no forge repository to name, and the commit +// itself is only readable in the checkout. +func (s *Scheduler) acceptCommitTarget(ctx context.Context, q store.Querier, snap dag.Snapshot, node, checkout string) (string, error) { + targets := map[string]bool{} + for _, e := range snap.Edges { + if e.FromNodeID == node && e.TargetRepository != "" && (e.Kind == dag.EdgeIntegrated || (e.Kind == dag.EdgeArtifactVerified && e.PinsCodeHead)) { + targets[e.TargetRepository] = true + } + } + switch len(targets) { + case 0: + return checkout, nil + case 1: + for t := range targets { + if t != checkout { + return "", refuse(contract.RefusalDispositionConflict, "the outgoing edges of %s name %s as their target and the commit is accepted on the checkout %s", node, t, checkout) + } + return t, nil + } + } + return "", refuse(contract.RefusalMalformedReceipt, "the outgoing edges of %s name more than one repository; one acceptance is judged against one", node) +} diff --git a/internal/relay/dagsched/accept_commit_edge_test.go b/internal/relay/dagsched/accept_commit_edge_test.go new file mode 100644 index 0000000000..92fa7eb9c8 --- /dev/null +++ b/internal/relay/dagsched/accept_commit_edge_test.go @@ -0,0 +1,47 @@ +package dagsched + +import ( + "context" + "testing" + + "github.com/thisisjun786/codex-relay-workflow/internal/relay/dag" +) + +// commitEdgeStatusOf reads one edge of plan "g" the way the scheduler's readers do. +func (k *batchKit) commitEdgeStatusOf(t *testing.T, edgeID string) EdgeStatus { + t.Helper() + ctx := context.Background() + snap, _, err := dag.SnapshotAt(ctx, k.sched.Store.Q(ctx), "g", 0) + if err != nil { + t.Fatalf("snapshot: %v", err) + } + for _, e := range snap.Edges { + if e.EdgeID == edgeID { + st, err := k.sched.edgeStatus(ctx, k.sched.Store.Q(ctx), "g", snap, e) + if err != nil { + t.Fatalf("edge %s: %v", edgeID, err) + } + return st + } + } + t.Fatalf("no edge %s", edgeID) + return EdgeStatus{} +} + +// CRW-965 (parent decision d5): a commit-accepted node satisfies a code-pinned artifact_verified edge without a +// pull request, with and without its integration. The acceptance is made by the production commit path. +func TestCommitAcceptedNodeSatisfiesAPinnedArtifactEdge(t *testing.T) { + k := newBatchKit(t, batchNode{name: "a", files: map[string]string{"a.txt": "a\n"}}) + k.putPlan("g", 1, "g-r2", addRelNode("b", dag.NodeNonPR), + addEdge("ab", "a", "b", dag.EdgeArtifactVerified, doc{"pins_code_head": true, "target_repository": k.repo.path, "target_base_ref": "dev"})) + k.acceptByCommit("a") + if st := k.commitEdgeStatusOf(t, "ab"); !st.Satisfied { + t.Fatalf("a commit-accepted node should satisfy the pinned edge before integration, got %+v", st) + } + if _, err := k.sched.IntegrateBatch(context.Background(), k.batchIn(), IntegrationBatchDeps{Verify: stubVerifier(writeStubVerifier(t)), Update: updateIntegrationRef}); err != nil { + t.Fatalf("batch: %v", err) + } + if st := k.commitEdgeStatusOf(t, "ab"); !st.Satisfied { + t.Fatalf("a commit-accepted and integrated node should satisfy the pinned edge, got %+v", st) + } +} diff --git a/internal/relay/dagsched/accept_commit_judge_test.go b/internal/relay/dagsched/accept_commit_judge_test.go new file mode 100644 index 0000000000..d7e795a153 --- /dev/null +++ b/internal/relay/dagsched/accept_commit_judge_test.go @@ -0,0 +1,45 @@ +package dagsched + +import ( + "context" + "encoding/json" + "runtime" + "strings" + "testing" +) + +// CRW-965 (parent decision d1): the relay judges a record by its reuse keys only. The tool versions and the Go +// flags and environment are the writer's to judge (CRW-964 refuses a pin mismatch, and its --reuse compares the +// environment), so a record that names other values is still reusable here, while a record without the member is +// refused. +func TestVerificationRecordIsJudgedByItsKeysNotByEnvironmentValues(t *testing.T) { + k := newCommitAcceptKit(t) + ctx := context.Background() + keys, err := CommitVerificationKeys(ctx, k.repo.path, k.head) + if err != nil { + t.Fatal(err) + } + want := VerificationKeys{Tree: k.treeOf(k.head), Base: k.base, CiDigest: keys.CiDigest, Dependencies: keys.Dependencies, OS: runtime.GOOS, Arch: runtime.GOARCH} + record := VerificationRecord{Runner: "local", Repository: "owner/repo", BaseCommit: k.base, HeadCommit: k.head, TreeHash: want.Tree, + CiDigest: keys.CiDigest, Tools: map[string]string{"go": "go0.0-other"}, Pins: map[string]string{}, GoFlags: "-mod=vendor", GoEnv: "GOOS=plan9", + PinMismatch: []string{}, Dependencies: keys.Dependencies, OS: runtime.GOOS, Arch: runtime.GOARCH, Result: "pass", Jobs: []json.RawMessage{}} + raw, err := SealVerificationRecord(record) + if err != nil { + t.Fatal(err) + } + if _, err := JudgeVerificationRecord(raw, want); err != nil { + t.Fatalf("a record that differs only in tools, goFlags and goEnv should be judged reusable by its keys: %v", err) + } + var members map[string]json.RawMessage + if err := json.Unmarshal(raw, &members); err != nil { + t.Fatal(err) + } + delete(members, "goFlags") + without, err := json.Marshal(members) + if err != nil { + t.Fatal(err) + } + if _, err := JudgeVerificationRecord(without, want); refusalReasonOf(err) != "disposition_conflict" || !strings.Contains(err.Error(), "goFlags") { + t.Fatalf("a record without its goFlags member should be refused under its name, got %v", err) + } +} diff --git a/internal/relay/dagsched/accept_commit_test.go b/internal/relay/dagsched/accept_commit_test.go new file mode 100644 index 0000000000..74236168a7 --- /dev/null +++ b/internal/relay/dagsched/accept_commit_test.go @@ -0,0 +1,280 @@ +package dagsched + +import ( + "context" + "encoding/json" + "errors" + "os" + "os/exec" + "path/filepath" + "runtime" + "strings" + "testing" + + "github.com/thisisjun786/codex-relay-workflow/internal/relay/dag" + "github.com/thisisjun786/codex-relay-workflow/internal/relay/store" +) + +// The pull-request-less acceptance path (CRW-965, criterion c1). A node whose result is a commit is +// accepted with --commit/--base/--verification/--checkout: the relay proves in the local checkout that +// the commit is the head the generation's ruling fixed, that it descends from the base, and that the +// verification record names this commit's tree with a PASS it may reuse. Every failure is refused with +// its own reason, and the pull-request path is untouched. + +// commitAcceptKit is a release kit over a real repository, with the plan's implementation node I and a +// terminal node D. +type commitAcceptKit struct { + *releaseKit + repo *gitRepo + head string // the commit the generation's ruling fixed + base string // the commit the head descends from +} + +func newCommitAcceptKit(t *testing.T) *commitAcceptKit { + t.Helper() + k := &commitAcceptKit{releaseKit: newReleaseKit(t), repo: newGitRepo(t)} + k.repo.git("checkout", "-q", "-b", "feature") + k.head = k.repo.commit("feature.txt", "feature") + k.repo.git("checkout", "-q", "dev") + k.base = k.repo.git("rev-parse", "dev") + k.putPlan("g", 0, "g-r1", addRelNode("I", dag.NodeImplementation), addRelNode("D", dag.NodeImplementation)) + return k +} + +// report fixes the head the generation's ruling verified (dag_verified_heads), which is what the +// commit path compares the --commit value against. +func (k *commitAcceptKit) report(plan, node string) accepted { + k.t.Helper() + r := k.reportNode(plan, node, acceptOpts{}) + k.exec("INSERT INTO dag_verified_heads (event_id, relationship_id, execution_generation, verdict_turn_id, head_sha, recorded_by_task_id, recorded_at) VALUES (?,?,?,?,?,?,?)", + r.Event, r.Acceptance.RelationshipID, 1, "verdict-turn", k.head, "parent", k.clock()) + return r +} + +// record writes a sealed verification-record/1 in the CRW-964 writer's format (camelCase members, the digest the +// writer computes), naming a tree and a result. The extra keys are the camelCase members a test changes: result, +// pinMismatch. +func (k *commitAcceptKit) record(tree, result string, extra map[string]any) string { + k.t.Helper() + record := VerificationRecord{Runner: "local", Repository: "owner/repo", BaseCommit: k.base, HeadCommit: k.head, TreeHash: tree, + CiDigest: "", Tools: map[string]string{"go": "go1.27.1"}, Pins: map[string]string{}, GoFlags: "", GoEnv: "", + PinMismatch: []string{}, Dependencies: map[string]string{"go.sum": "", "web/package-lock.json": ""}, OS: runtime.GOOS, Arch: runtime.GOARCH, + Result: result, Jobs: []json.RawMessage{}} + if pins, ok := extra["pinMismatch"].([]string); ok { + record.PinMismatch = pins + } + if value, ok := extra["result"].(string); ok { + record.Result = value + } + keys, err := CommitVerificationKeys(context.Background(), k.repo.path, k.head) + if err != nil { + k.t.Fatal(err) + } + record.CiDigest, record.Dependencies = keys.CiDigest, keys.Dependencies + raw, err := SealVerificationRecord(record) + if err != nil { + k.t.Fatal(err) + } + path := filepath.Join(k.t.TempDir(), "verification-record.json") + if err := os.WriteFile(path, raw, 0o600); err != nil { + k.t.Fatal(err) + } + return "@" + path +} + +// treeOf reads the tree of a commit the way the relay does. +func (k *commitAcceptKit) treeOf(commit string) string { + k.t.Helper() + out, err := exec.Command("git", "-C", k.repo.path, "rev-parse", commit+"^{tree}").Output() + if err != nil { + k.t.Fatal(err) + } + return strings.TrimSpace(string(out)) +} + +func (k *commitAcceptKit) acceptCommit(record string) (AcceptResult, error) { + k.t.Helper() + return k.sched.Accept(context.Background(), "g", "I", "parent", AcceptInput{ + RuleVersion: VerifierRule{SkillsDigest: dig("skills"), Model: "m", Effort: "none"}, + Commit: &CommitRef{Head: k.head, Base: k.base, Checkout: k.repo.path, Record: record}, + }) +} + +func refusalReasonOf(err error) string { + var refused *store.RefusedError + if errors.As(err, &refused) { + return refused.Reason + } + return "" +} + +// c1: the happy path. The acceptance records the commit as its head and the checkout as its +// repository, and writes the verification record with it. +func TestCommitAcceptanceRecordsTheCommit(t *testing.T) { + t.Parallel() + k := newCommitAcceptKit(t) + k.report("g", "I") + res, err := k.acceptCommit(k.record(k.treeOf(k.head), "pass", nil)) + if err != nil { + t.Fatalf("accept by commit: %v", err) + } + if res.HeadSHA != k.head || res.AcceptanceID == "" || res.Generation != 1 || res.Replayed || res.Revalidated { + t.Fatalf("the acceptance = %+v", res) + } + var repo, head string + if _, err := queryOne(context.Background(), k.s.Q(context.Background()), "SELECT repository, head_sha FROM dag_acceptances WHERE acceptance_id = ?", []any{res.AcceptanceID}, &repo, &head); err != nil { + t.Fatal(err) + } + if repo != k.repo.path || head != k.head { + t.Fatalf("the acceptance row = repository %q head %q", repo, head) + } + // no forge row: the absence is what distinguishes the commit path + if k.count("SELECT COUNT(*) FROM dag_acceptance_forge WHERE acceptance_id = ?", res.AcceptanceID) != 0 { + t.Fatal("the commit path wrote a forge identity") + } + // the verification record is stored with the acceptance + row, found, err := store.AcceptanceVerification(context.Background(), k.s, res.AcceptanceID) + if err != nil || !found || row.HeadCommit != k.head || row.TreeSHA != k.treeOf(k.head) || row.RecordDigest == "" || row.RecordJSON == "" { + t.Fatalf("the stored verification = %+v found=%v err=%v", row, found, err) + } +} + +// c1: a repeat of the same call is a replay, not a second acceptance. +func TestCommitAcceptanceReplays(t *testing.T) { + t.Parallel() + k := newCommitAcceptKit(t) + k.report("g", "I") + record := k.record(k.treeOf(k.head), "pass", nil) + first, err := k.acceptCommit(record) + if err != nil { + t.Fatal(err) + } + again, err := k.acceptCommit(record) + if err != nil || !again.Replayed || again.AcceptanceID != first.AcceptanceID { + t.Fatalf("the replay = %+v err=%v", again, err) + } + if n := k.count("SELECT COUNT(*) FROM dag_acceptances WHERE plan_id = 'g' AND node_id = 'I'"); n != 1 { + t.Fatalf("%d acceptances were written", n) + } +} + +// c1: a commit that is not the head the generation's ruling fixed is refused head_not_receipt_head. +func TestCommitAcceptanceRefusesACommitThatIsNotTheRuledHead(t *testing.T) { + t.Parallel() + k := newCommitAcceptKit(t) + k.report("g", "I") + // another commit on the feature branch, which the ruling never fixed + k.repo.git("checkout", "-q", "feature") + other := k.repo.commit("other.txt", "other") + k.repo.git("checkout", "-q", "dev") + _, err := k.sched.Accept(context.Background(), "g", "I", "parent", AcceptInput{RuleVersion: VerifierRule{SkillsDigest: "s", Model: "m", Effort: "none"}, + Commit: &CommitRef{Head: other, Base: k.base, Checkout: k.repo.path, Record: k.record(k.treeOf(other), "pass", nil)}}) + if got := refusalReasonOf(err); got != "head_not_receipt_head" { + t.Fatalf("reason %q (err %v)", got, err) + } + if n := k.count("SELECT COUNT(*) FROM dag_acceptances"); n != 0 { + t.Fatalf("%d acceptance rows were written", n) + } +} + +// c1: a commit that does not descend from the base is refused merge_base_mismatch. +func TestCommitAcceptanceRefusesACommitThatDoesNotDescendFromTheBase(t *testing.T) { + t.Parallel() + k := newCommitAcceptKit(t) + k.report("g", "I") + // an unrelated base: the base of the acceptance is a commit the head does not descend from + k.repo.git("checkout", "-q", "-b", "unrelated", "dev") + k.repo.commit("unrelated.txt", "unrelated") + unrelated := k.repo.git("rev-parse", "HEAD") + k.repo.git("checkout", "-q", "dev") + _, err := k.sched.Accept(context.Background(), "g", "I", "parent", AcceptInput{RuleVersion: VerifierRule{SkillsDigest: "s", Model: "m", Effort: "none"}, + Commit: &CommitRef{Head: k.head, Base: unrelated, Checkout: k.repo.path, Record: k.record(k.treeOf(k.head), "pass", nil)}}) + if got := refusalReasonOf(err); got != "merge_base_mismatch" { + t.Fatalf("reason %q (err %v)", got, err) + } + if n := k.count("SELECT COUNT(*) FROM dag_acceptances"); n != 0 { + t.Fatalf("%d acceptance rows were written", n) + } +} + +// c1: a record naming another tree is refused revision_mismatch. +func TestCommitAcceptanceRefusesARecordOfAnotherTree(t *testing.T) { + t.Parallel() + k := newCommitAcceptKit(t) + k.report("g", "I") + _, err := k.acceptCommit(k.record(k.treeOf(k.base), "pass", nil)) + if got := refusalReasonOf(err); got != "revision_mismatch" { + t.Fatalf("reason %q (err %v)", got, err) + } + if n := k.count("SELECT COUNT(*) FROM dag_acceptances"); n != 0 { + t.Fatalf("%d acceptance rows were written", n) + } +} + +// c1: a record whose result is not PASS, or which is not reusable, is refused disposition_conflict. +func TestCommitAcceptanceRefusesAFailedOrNonReusableRecord(t *testing.T) { + t.Parallel() + for _, c := range []struct { + name string + extra map[string]any + }{ + {"fail", map[string]any{"result": "fail"}}, + {"pin mismatch", map[string]any{"pinMismatch": []string{"go"}}}, + } { + c := c + t.Run(c.name, func(t *testing.T) { + t.Parallel() + k := newCommitAcceptKit(t) + k.report("g", "I") + _, err := k.acceptCommit(k.record(k.treeOf(k.head), "pass", c.extra)) + if got := refusalReasonOf(err); got != "disposition_conflict" { + t.Fatalf("reason %q (err %v)", got, err) + } + if n := k.count("SELECT COUNT(*) FROM dag_acceptances"); n != 0 { + t.Fatalf("%d acceptance rows were written", n) + } + }) + } +} + +// c1: a record that is not a readable verification-record/1 is refused malformed_receipt. +func TestCommitAcceptanceRefusesAMalformedRecord(t *testing.T) { + t.Parallel() + for _, c := range []struct { + name string + raw string + }{ + {"not json", "{"}, + {"wrong schema", `{"schema":"other/1","head_commit":"` + strings.Repeat("a", 40) + `","tree":"` + strings.Repeat("b", 40) + `","result":"PASS"}`}, + } { + c := c + t.Run(c.name, func(t *testing.T) { + t.Parallel() + k := newCommitAcceptKit(t) + k.report("g", "I") + path := filepath.Join(t.TempDir(), "record.json") + if err := os.WriteFile(path, []byte(c.raw), 0o600); err != nil { + t.Fatal(err) + } + _, err := k.acceptCommit("@" + path) + if got := refusalReasonOf(err); got != "malformed_receipt" { + t.Fatalf("reason %q (err %v)", got, err) + } + if n := k.count("SELECT COUNT(*) FROM dag_acceptances"); n != 0 { + t.Fatalf("%d acceptance rows were written", n) + } + }) + } +} + +// c1: the existing pull-request path is unchanged: a commit-path call is refused when the generation's +// ruling fixed no head at all. +func TestCommitAcceptanceRefusesWhenTheRulingFixedNoHead(t *testing.T) { + t.Parallel() + k := newCommitAcceptKit(t) + k.reportNode("g", "I", acceptOpts{}) // no dag_verified_heads row + _, err := k.acceptCommit(k.record(k.treeOf(k.head), "pass", nil)) + if got := refusalReasonOf(err); got != "head_not_receipt_head" { + t.Fatalf("reason %q (err %v)", got, err) + } +} diff --git a/internal/relay/dagsched/accept_criteria_batch_test.go b/internal/relay/dagsched/accept_criteria_batch_test.go new file mode 100644 index 0000000000..8aac8e5da3 --- /dev/null +++ b/internal/relay/dagsched/accept_criteria_batch_test.go @@ -0,0 +1,106 @@ +package dagsched + +import ( + "context" + "reflect" + "testing" + + "github.com/thisisjun786/codex-relay-workflow/internal/relay/store" +) + +// acceptByCommitForCriteria accepts one node through the production commit path and returns its acceptance. +func (k *batchKit) acceptByCommitForCriteria(node string) AcceptResult { + k.t.Helper() + head := k.heads[node] + record := writeSealedRecord(k.t, k.repo.path, head, k.base, k.treeOf(head), "pass") + out, err := k.sched.Accept(context.Background(), "g", node, "parent", AcceptInput{ + RuleVersion: VerifierRule{SkillsDigest: dig("skills"), Model: "m", Effort: "none"}, + Commit: &CommitRef{Head: head, Base: k.base, Checkout: k.repo.path, Record: record}, + }) + if err != nil { + k.t.Fatalf("accept %s: %v", node, err) + } + return out +} + +// CRW-965 (parent decision d2): a criteria change is verified again through the scheduler's revalidation, and the next +// batch verifies the node afresh. An integrated node is never merged or verified again, an unrevalidated candidate is +// not ready, and the frozen rows name the criteria set each candidate is verified under. +func TestIntegrationBatchReadsACriteriaChangeAsDecided(t *testing.T) { + k := newBatchKit(t, batchNode{name: "a", files: map[string]string{"a.txt": "a\n"}}, batchNode{name: "b", files: map[string]string{"b.txt": "b\n"}}) + runs := 0 + base := stubVerifier(writeStubVerifier(t)) + deps := IntegrationBatchDeps{Verify: func(ctx context.Context, dir string, env []string) error { + runs++ + return base(ctx, dir, env) + }, Update: updateIntegrationRef} + ctx := context.Background() + + // A is accepted and integrated. + k.acceptByCommit("a") + in := k.batchIn() + in.Nodes = []string{"a"} + if _, err := k.sched.IntegrateBatch(ctx, in, deps); err != nil { + t.Fatalf("batch a: %v", err) + } + if runs != 1 { + t.Fatalf("verifier ran %d times for A; want 1", runs) + } + + // A's criteria change: A is stale and an explicit request for it names the integration instead of a generic refusal. + k.invRevise("g", "a", "g-r2", func(n doc) { n["criteria_set_digest"] = dig("changed a") }) + if got := invStaleIDs(k.read("g")); !reflect.DeepEqual(got, []string{"a"}) { + t.Fatalf("after A's criteria change stale = %v, want [a]", got) + } + if _, err := k.sched.IntegrateBatch(ctx, in, deps); refusalReasonOf(err) != "disposition_conflict" { + t.Fatalf("an integrated node named again: reason %q (err %v)", refusalReasonOf(err), err) + } + if runs != 1 { + t.Fatalf("verifier ran %d times after A's change; an integrated node must not be verified again", runs) + } + + // B is accepted, its criteria change before any batch: B is stale, is not merged and is not verified. + bAccept := k.acceptByCommitForCriteria("b") + k.invRevise("g", "b", "g-r3", func(n doc) { n["criteria_set_digest"] = dig("changed b") }) + if got := invStaleIDs(k.read("g")); !reflect.DeepEqual(got, []string{"a", "b"}) { + t.Fatalf("after B's criteria change stale = %v, want [a b]", got) + } + if res, err := k.sched.IntegrateBatch(ctx, k.batchIn(), deps); err == nil && len(res.Merged) != 0 { + t.Fatalf("a stale candidate was merged: %+v", res) + } + if runs != 1 { + t.Fatalf("verifier ran %d times for a stale candidate; want 1", runs) + } + + // The re-verification under the new criteria (the revalidation row, as the scheduler leaves it) makes B a candidate + // again; the next batch verifies it afresh and freezes the new criteria digest on its row. + var event string + if err := k.s.DB.QueryRow("SELECT event_id FROM events WHERE relationship_id = ?", bAccept.RelationshipID).Scan(&event); err != nil { + t.Fatal(err) + } + k.exec("UPDATE canonical_criteria SET set_digest = ? WHERE relationship_id = ?", dig("changed b"), bAccept.RelationshipID) + k.exec("INSERT INTO dag_acceptance_revalidations (revalidation_id, acceptance_id, criteria_set_digest, event_id, verdict_turn_id, reval_seq, revalidated_by, revalidated_at) VALUES ('rv-b', ?, ?, ?, 'vt2', 1, 'parent', 't')", + bAccept.AcceptanceID, dig("changed b"), event) + if got := invStaleIDs(k.read("g")); !reflect.DeepEqual(got, []string{"a"}) { + t.Fatalf("after B's revalidation stale = %v, want [a]", got) + } + if _, err := k.sched.IntegrateBatch(ctx, k.batchIn(), deps); err != nil { + t.Fatalf("batch b: %v", err) + } + if runs != 2 { + t.Fatalf("verifier ran %d times after B's revalidation; want 2", runs) + } + rows, err := store.IntegrationStagesOfPlan(ctx, k.sched.Store, "g") + if err != nil { + t.Fatal(err) + } + var frozen []string + for _, r := range rows { + if r.Stage == "intent" && r.NodeID == "b" { + frozen = append(frozen, r.Detail) + } + } + if !reflect.DeepEqual(frozen, []string{dig("changed b")}) { + t.Fatalf("B's frozen intent rows name criteria %v; want the new set", frozen) + } +} diff --git a/internal/relay/dagsched/accept_fresh_record_test.go b/internal/relay/dagsched/accept_fresh_record_test.go new file mode 100644 index 0000000000..4ddd46745e --- /dev/null +++ b/internal/relay/dagsched/accept_fresh_record_test.go @@ -0,0 +1,49 @@ +package dagsched + +import ( + "context" + "encoding/json" + "os" + "path/filepath" + "runtime" + "testing" +) + +// freshRecord writes a PASS record for tree with one job whose timings differ, so its bytes differ from record's. +func (k *commitAcceptKit) freshRecord(tree, job string) string { + k.t.Helper() + keys, err := CommitVerificationKeys(context.Background(), k.repo.path, k.head) + if err != nil { + k.t.Fatal(err) + } + record := VerificationRecord{Runner: "local", Repository: "owner/repo", BaseCommit: k.base, HeadCommit: k.head, TreeHash: tree, + CiDigest: keys.CiDigest, Tools: map[string]string{"go": "go1.27.1"}, Pins: map[string]string{}, GoFlags: "", GoEnv: "", + PinMismatch: []string{}, Dependencies: keys.Dependencies, OS: runtime.GOOS, Arch: runtime.GOARCH, Result: "pass", + Jobs: []json.RawMessage{json.RawMessage(job)}} + raw, err := SealVerificationRecord(record) + if err != nil { + k.t.Fatal(err) + } + path := filepath.Join(k.t.TempDir(), "fresh-record.json") + if err := os.WriteFile(path, raw, 0o600); err != nil { + k.t.Fatal(err) + } + return "@" + path +} + +// CRW-965 (parent decision, D3): a fresh record for the same head and tree is the same output. The replay proceeds on the +// head and tree the verification row stores; the file bytes are not compared. +func TestCommitAcceptanceReplaysOnAFreshRecordOfTheSameTree(t *testing.T) { + k := newCommitAcceptKit(t) + k.report("g", "I") + if _, err := k.acceptCommit(k.record(k.treeOf(k.head), "pass", nil)); err != nil { + t.Fatalf("first acceptance: %v", err) + } + out, err := k.acceptCommit(k.freshRecord(k.treeOf(k.head), `{"name":"go test","seconds":"312"}`)) + if err != nil { + t.Fatalf("a fresh record of the same tree should replay the acceptance: %v", err) + } + if !out.Replayed { + t.Fatalf("want a replay, got %+v", out) + } +} diff --git a/internal/relay/dagsched/accept_pins_test.go b/internal/relay/dagsched/accept_pins_test.go new file mode 100644 index 0000000000..17092fa85e --- /dev/null +++ b/internal/relay/dagsched/accept_pins_test.go @@ -0,0 +1,41 @@ +package dagsched + +import ( + "context" + "runtime" + "strings" + "testing" +) + +// CRW-965 (parent decision, D1 refined): a record whose tool values contradict its own pins is internally inconsistent and +// is refused. A tool without a pin is not judged, and goFlags and goEnv stay the writer's. +func TestVerificationRecordPinsMustMatchTheirTools(t *testing.T) { + k := newCommitAcceptKit(t) + ctx := context.Background() + keys, err := CommitVerificationKeys(ctx, k.repo.path, k.head) + if err != nil { + t.Fatal(err) + } + want := VerificationKeys{Tree: k.treeOf(k.head), Base: k.base, CiDigest: keys.CiDigest, Dependencies: keys.Dependencies, OS: runtime.GOOS, Arch: runtime.GOARCH} + seal := func(tools, pins map[string]string) []byte { + t.Helper() + record := VerificationRecord{Runner: "local", Repository: "owner/repo", BaseCommit: k.base, HeadCommit: k.head, TreeHash: want.Tree, + CiDigest: keys.CiDigest, Tools: tools, Pins: pins, GoFlags: "", GoEnv: "", PinMismatch: []string{}, + Dependencies: keys.Dependencies, OS: runtime.GOOS, Arch: runtime.GOARCH, Result: "pass"} + raw, err := SealVerificationRecord(record) + if err != nil { + t.Fatal(err) + } + return raw + } + if _, err := JudgeVerificationRecord(seal(map[string]string{"go": "go1.27.1"}, map[string]string{"go": "go1.27.1"}), want); err != nil { + t.Fatalf("a record whose tools match its pins is reusable: %v", err) + } + contradicting := seal(map[string]string{"go": "go0.0-other"}, map[string]string{"go": "go1.27.1"}) + if _, err := JudgeVerificationRecord(contradicting, want); refusalReasonOf(err) != "disposition_conflict" || !strings.Contains(err.Error(), "go") { + t.Fatalf("a tool that contradicts its pin is refused under its name, got %v", err) + } + if _, err := JudgeVerificationRecord(seal(map[string]string{"go": "go0.0-other"}, map[string]string{}), want); err != nil { + t.Fatalf("a tool without a pin is not judged: %v", err) + } +} diff --git a/internal/relay/dagsched/candidates.go b/internal/relay/dagsched/candidates.go new file mode 100644 index 0000000000..0249f85e48 --- /dev/null +++ b/internal/relay/dagsched/candidates.go @@ -0,0 +1,260 @@ +package dagsched + +import ( + "context" + "database/sql" + "sort" + + "github.com/thisisjun786/codex-relay-workflow/internal/contract" + "github.com/thisisjun786/codex-relay-workflow/internal/relay/dag" + "github.com/thisisjun786/codex-relay-workflow/internal/relay/delivery" + "github.com/thisisjun786/codex-relay-workflow/internal/relay/store" +) + +// The read and write facade of the integration batch (CRW-965). The batch lives in this package so its tests can +// use the same plan, acceptance and mark fixtures as the scheduler's own tests. A candidate is an accepted +// implementation node whose active acceptance is current: the node is live, its relationship is active and on the +// generation the acceptance stands on, and the scheduler's staleness judgment (spec, consumed inputs and criteria) +// finds nothing wrong. Every judgment is the scheduler's own; the batch reads them and never restates them. + +// Candidate is one accepted implementation node a batch may merge, and the identity its merged mark is keyed on: +// the acceptance, its event, its revision hash, its generation and the head it stands on. A batch freezes these +// before it touches git, and the mark names the frozen acceptance and event, never whatever is current later. +type Candidate struct { + PlanID, NodeID, AcceptanceID string + RelationshipID string + EventID, RevisionHash string + Generation int64 + HeadSHA, Repository string + // CriteriaSetDigest is the criteria set the acceptance stands on: a frozen batch row names it (CRW-965, parent decision d2). + CriteriaSetDigest string + // AcceptedCriteriaDigest is the criteria set the acceptance was made under, before any revalidation (CRW-965, D1). + AcceptedCriteriaDigest string +} + +// alreadyIntegratedNode is whether a node's active acceptance already landed on an integration target (CRW-965, +// parent decision d2): such a node is never a candidate again, so an explicit request for it is refused by name. +func (s *Scheduler) alreadyIntegratedNode(ctx context.Context, plan, node string) (bool, error) { + q := s.Store.Q(ctx) + acc, found, err := loadActiveAcceptance(ctx, q, plan, node) + if err != nil || !found || acc.HeadSHA == "" { + return false, err + } + snap, _, err := dag.SnapshotAt(ctx, q, plan, 0) + if err != nil { + return false, err + } + landed, _, err := s.nodeIntegrated(ctx, q, plan, snap, acc) + return landed, err +} + +// AcceptedCandidates is the plan's ready accepted implementation candidates, in node-id order so two calls agree. +// A node that already integrated is left out, and so is one that is stale, whose relationship is not active or is +// not on the generation its acceptance stands on, or that the plan holds (paused, cancelled or archived). +func (s *Scheduler) AcceptedCandidates(ctx context.Context, plan string) ([]Candidate, error) { + q := s.Store.Q(ctx) + snap, _, err := dag.SnapshotAt(ctx, q, plan, 0) + if err != nil { + return nil, err + } + nodes := append([]dag.SnapNode(nil), snap.Nodes...) + sort.Slice(nodes, func(i, j int) bool { return nodes[i].NodeID < nodes[j].NodeID }) + var out []Candidate + for _, n := range nodes { + if n.Kind != dag.NodeImplementation { + continue + } + c, ok, err := s.currentCandidate(ctx, q, plan, snap, n) + if err != nil { + return nil, err + } + if ok { + out = append(out, c) + } + } + return out, nil +} + +// currentCandidate is the one currency predicate of an implementation node's candidate (CRW-965, parent decisions D-D and +// D3): the node is live, its active acceptance is not integrated, not stale, its relationship is active on the generation +// the acceptance stands on, and the accepted event is still the head of that generation. Candidate selection and the +// batch's check inside its fenced transaction both call it, so they never disagree. +func (s *Scheduler) currentCandidate(ctx context.Context, q store.Querier, plan string, snap dag.Snapshot, n dag.SnapNode) (Candidate, bool, error) { + if err := lifecycleRefusal(snap, n, "integrating its result", true); err != nil { + return Candidate{}, false, nil + } + acc, found, err := loadActiveAcceptance(ctx, q, plan, n.NodeID) + if err != nil { + return Candidate{}, false, err + } + if !found || acc.HeadSHA == "" { + return Candidate{}, false, nil + } + landed, _, err := s.nodeIntegrated(ctx, q, plan, snap, acc) + if err != nil { + return Candidate{}, false, err + } + if landed { + return Candidate{}, false, nil + } + stale, err := s.staleOf(ctx, q, plan, snap, n) + if err != nil { + return Candidate{}, false, err + } + if stale != nil { + return Candidate{}, false, nil + } + rel, found, err := currentRelationshipOf(ctx, q, plan, n.NodeID) + if err != nil { + return Candidate{}, false, err + } + if !found || rel.Status != "active" || rel.Superseded { + return Candidate{}, false, nil + } + stand, err := s.standOf(ctx, q, acc) + if err != nil { + return Candidate{}, false, err + } + criteria, err := currentCriteriaDigest(ctx, q, acc) + if err != nil { + return Candidate{}, false, err + } + if stand.Generation != rel.Generation { + return Candidate{}, false, nil + } + // the accepted event must still be the head of its generation: a newer receipt makes this acceptance stale (CRW-965 review) + head, err := delivery.HeadRevisionFrom(ctx, q, stand.RelationshipID, stand.Generation) + if err != nil { + return Candidate{}, false, err + } + if id, _ := objString(head, "eventId"); id != stand.EventID { + return Candidate{}, false, nil + } + return Candidate{PlanID: plan, NodeID: n.NodeID, AcceptanceID: acc.AcceptanceID, RelationshipID: stand.RelationshipID, + EventID: stand.EventID, RevisionHash: stand.RevisionHash, Generation: stand.Generation, HeadSHA: stand.Head, Repository: acc.Repository, + CriteriaSetDigest: criteria, AcceptedCriteriaDigest: acc.CriteriaSetDigest}, true, nil +} + +// frozenCandidateStillCurrent is whether a frozen candidate is still the current candidate of its node under the same +// predicate candidate selection uses, with the same acceptance, event, revision, generation and head (CRW-965, D3). The +// batch asks it inside the transaction that moves the branch. +func (s *Scheduler) frozenCandidateStillCurrent(ctx context.Context, f Candidate) (bool, error) { + q := s.Store.Q(ctx) + snap, _, err := dag.SnapshotAt(ctx, q, f.PlanID, 0) + if err != nil { + return false, err + } + n, ok := nodeOf(snap, f.NodeID) + if !ok { + return false, nil + } + c, ok, err := s.currentCandidate(ctx, q, f.PlanID, snap, n) + if err != nil || !ok { + return false, err + } + return c.AcceptanceID == f.AcceptanceID && c.EventID == f.EventID && c.RevisionHash == f.RevisionHash && + c.Generation == f.Generation && c.HeadSHA == f.HeadSHA && c.CriteriaSetDigest == f.CriteriaSetDigest, nil +} + +// currentCriteriaDigest is the criteria set a candidate is verified under: the one its latest revalidation names, else +// the one its acceptance stands on (CRW-965, parent decision d2). +func currentCriteriaDigest(ctx context.Context, q store.Querier, acc Acceptance) (string, error) { + var digest string + found, err := queryOne(ctx, q, "SELECT criteria_set_digest FROM dag_acceptance_revalidations WHERE acceptance_id = ? ORDER BY reval_seq DESC LIMIT 1", []any{acc.AcceptanceID}, &digest) + if err != nil { + return "", err + } + if found { + return digest, nil + } + return acc.CriteriaSetDigest, nil +} + +// MarkFrozen writes the parent's merged mark for one frozen candidate. The actor must be the parent of the node's +// relationship (notParentHeldBy, the check dag-accept makes), the acceptance must still be the frozen one and not +// stale, and the mark names the frozen event: a mark that cannot be written is refused, and the batch records it as +// pending. +func (s *Scheduler) MarkFrozen(ctx context.Context, f Candidate, actor, evidence string) (string, error) { + q := s.Store.Q(ctx) + snap, _, err := dag.SnapshotAt(ctx, q, f.PlanID, 0) + if err != nil { + return "", err + } + n, ok := nodeOf(snap, f.NodeID) + if !ok { + return "", refuse(contract.RefusalStaleMarkContext, "node %s is no longer in plan %s", f.NodeID, f.PlanID) + } + rel, found, err := currentRelationshipOf(ctx, q, f.PlanID, f.NodeID) + if err != nil { + return "", err + } + if !found { + return "", refuse(contract.RefusalUnregisteredRelationship, "node %s has no execution to mark", f.NodeID) + } + if rel.ParentTaskID != actor { + return "", notParentHeldBy(actor, rel) + } + acc, found, err := loadActiveAcceptance(ctx, q, f.PlanID, f.NodeID) + if err != nil { + return "", err + } + if !found || acc.AcceptanceID != f.AcceptanceID { + return "", refuse(contract.RefusalStaleMarkContext, "the active acceptance of %s is no longer the one the batch merged (%s)", f.NodeID, f.AcceptanceID) + } + stale, err := s.staleOf(ctx, q, f.PlanID, snap, n) + if err != nil { + return "", err + } + if stale != nil { + return "", refuse(contract.RefusalStaleMarkContext, "node %s is stale: its merged mark waits for the node to be current", f.NodeID) + } + view := s.assignmentView(ctx) + if _, err := view.Mark(ctx, rel.ID, "merged", evidence, actor, f.EventID); err != nil { + return "", err + } + return f.EventID, nil +} + +// IntegrationWrite runs the write of an integration stage inside one transaction after the coordinator-epoch fence +// dag-accept uses (fence, the same check): a stale epoch is refused before anything is written. The write sees the +// transaction's context. +func (s *Scheduler) IntegrationWrite(ctx context.Context, plan, actor string, write func(txCtx context.Context) error) error { + return s.Store.Compose(ctx, func(txCtx context.Context, _ *sql.Conn) error { + if err := s.fence(txCtx, s.Store.Q(txCtx), plan, actor); err != nil { + return err + } + return write(txCtx) + }) +} + +// Successors maps each node of a plan to the nodes that depend on it through the plan's edges, sorted. The batch +// uses it to leave a failing candidate's dependants out with it. +func (s *Scheduler) Successors(ctx context.Context, plan string) (map[string][]string, error) { + snap, _, err := dag.SnapshotAt(ctx, s.Store.Q(ctx), plan, 0) + if err != nil { + return nil, err + } + out := map[string][]string{} + for _, e := range snap.Edges { + out[e.FromNodeID] = append(out[e.FromNodeID], e.ToNodeID) + } + for k := range out { + sort.Strings(out[k]) + } + return out, nil +} + +// CandidateTargets are the targets a node's active acceptance has to land on, the same set the scheduler judges for +// integration. A node with no active acceptance has none. +func (s *Scheduler) CandidateTargets(ctx context.Context, plan, node string) ([]Target, error) { + q := s.Store.Q(ctx) + snap, _, err := dag.SnapshotAt(ctx, q, plan, 0) + if err != nil { + return nil, err + } + acc, found, err := loadActiveAcceptance(ctx, q, plan, node) + if err != nil || !found { + return nil, err + } + return s.nodeTargets(ctx, q, snap, acc) +} diff --git a/internal/relay/dagsched/cli.go b/internal/relay/dagsched/cli.go index 3d2e97631f..117d30b5cd 100644 --- a/internal/relay/dagsched/cli.go +++ b/internal/relay/dagsched/cli.go @@ -218,6 +218,16 @@ func runAccept(ctx context.Context, services dispatch.Services, args dispatch.Ar } input.PullRequest = &PRRef{Repository: args.Text("repository"), Number: args.Integer("pull-request")} } + if args.Given("commit") { + // the pull-request-less acceptance (CRW-965): the commit, its base, its checkout and its verification record + if args.Given("repository") || args.Given("pull-request") { + return nil, usage("--commit accepts a node by its commit, without a pull request: drop --repository and --pull-request") + } + if !args.Given("base") || !args.Given("checkout") || !args.Given("verification") { + return nil, usage("--commit names --base, --checkout and --verification too") + } + input.Commit = &CommitRef{Head: args.Text("commit"), Base: args.Text("base"), Checkout: args.Text("checkout"), Record: args.Text("verification")} + } sched, closeStore, err := openScheduler(ctx, services, args) if err != nil { return nil, err diff --git a/internal/relay/dagsched/commit_target_test.go b/internal/relay/dagsched/commit_target_test.go new file mode 100644 index 0000000000..5d3a794335 --- /dev/null +++ b/internal/relay/dagsched/commit_target_test.go @@ -0,0 +1,31 @@ +package dagsched + +import ( + "context" + "testing" + + "github.com/thisisjun786/codex-relay-workflow/internal/relay/dag" +) + +// CRW-965 (parent decision, D4): a commit-accepted node completes on the local integration branch its batch moved. Its +// outgoing integrated edge names the checkout; the edge's base ref is not part of what integrated means for such a node, +// so the successor becomes satisfied once the node is observed on the integration branch. +func TestCommitAcceptedNodeCompletesOnTheIntegrationBranchItMoved(t *testing.T) { + k := newBatchKit(t, batchNode{name: "a", files: map[string]string{"a.txt": "a\n"}}) + k.putPlan("g", 1, "g-r2", addRelNode("b", dag.NodeNonPR), + addEdge("ab", "a", "b", dag.EdgeIntegrated, doc{"target_repository": k.repo.path, "target_base_ref": "dev"})) + ctx := context.Background() + k.acceptByCommit("a") + if _, err := k.sched.IntegrateBatch(ctx, k.batchIn(), IntegrationBatchDeps{Verify: stubVerifier(writeStubVerifier(t)), Update: updateIntegrationRef}); err != nil { + t.Fatalf("batch: %v", err) + } + k.sched.Tips = newLegacyLocalTipReader(t) + k.sched.Ancestry = GitAncestry{}.Ancestry + obs, err := k.sched.ObserveIntegration(ctx, "g", "a", "parent", []Target{{Repository: k.repo.path, BaseRef: "dev-int"}}) + if err != nil || !obs.Integrated { + t.Fatalf("observe a on the integration branch: %+v, %v; want integrated", obs, err) + } + if st := k.commitEdgeStatusOf(t, "ab"); !st.Satisfied { + t.Fatalf("the successor should be satisfied once a landed on the integration branch, got %+v", st) + } +} diff --git a/internal/relay/dagsched/commit_targets.go b/internal/relay/dagsched/commit_targets.go new file mode 100644 index 0000000000..b32e33da93 --- /dev/null +++ b/internal/relay/dagsched/commit_targets.go @@ -0,0 +1,93 @@ +package dagsched + +import ( + "context" + "encoding/json" + "sort" + + "github.com/thisisjun786/codex-relay-workflow/internal/relay/dag" + "github.com/thisisjun786/codex-relay-workflow/internal/relay/store" +) + +// commitAccepted is whether an acceptance was made by the commit path: a verification row and no forge row (CRW-965). +// A store that predates the verification table has no commit acceptances, so it is answered without a query that +// would fail on it. +func commitAccepted(ctx context.Context, q store.Querier, acceptanceID string) (bool, error) { + var name string + exists, err := queryOne(ctx, q, "SELECT name FROM sqlite_master WHERE type = 'table' AND name = 'dag_acceptance_verifications'", nil, &name) + if err != nil || !exists { + return false, err + } + var one int + hasVerification, err := queryOne(ctx, q, "SELECT 1 FROM dag_acceptance_verifications WHERE acceptance_id = ?", []any{acceptanceID}, &one) + if err != nil || !hasVerification { + return false, err + } + hasForge, err := queryOne(ctx, q, "SELECT 1 FROM dag_acceptance_forge WHERE acceptance_id = ?", []any{acceptanceID}, &one) + if err != nil { + return false, err + } + return !hasForge, nil +} + +// integrationRefsOf are the integration refs an acceptance completes on (CRW-965, parent decision D4 and its revision): the +// refs of the acceptance's own batches that moved the ref (a ref_moved row), and the ref of any of its batches that marked the +// acceptance as already contained on it (a marked row, which moves no ref). A batch that failed before its move, and a planned +// or abandoned intent, name no ref. The result is sorted and deduplicated, and nodeTargets and the observation both read it. +func integrationRefsOf(ctx context.Context, q store.Querier, acceptanceID string) ([]string, error) { + rows, err := q.QueryContext(ctx, "SELECT DISTINCT b.detail FROM dag_integration_stages c JOIN dag_integration_stages b ON b.batch_id = c.batch_id AND b.stage = 'intent' AND b.node_id = '' WHERE c.acceptance_id = ? AND c.stage = 'intent' AND c.node_id <> '' AND (EXISTS (SELECT 1 FROM dag_integration_stages m WHERE m.batch_id = c.batch_id AND m.stage = 'ref_moved') OR EXISTS (SELECT 1 FROM dag_integration_stages k WHERE k.batch_id = c.batch_id AND k.acceptance_id = c.acceptance_id AND k.stage = 'marked'))", acceptanceID) + if err != nil { + return nil, err + } + defer rows.Close() + seen := map[string]bool{} + for rows.Next() { + var detail string + if err := rows.Scan(&detail); err != nil { + return nil, err + } + var d map[string]string + if json.Unmarshal([]byte(detail), &d) == nil && d["integration_ref"] != "" { + seen[d["integration_ref"]] = true + } + } + if err := rows.Err(); err != nil { + return nil, err + } + out := make([]string, 0, len(seen)) + for ref := range seen { + out = append(out, ref) + } + sort.Strings(out) + return out, nil +} + +// commitIntegratedEdge judges an integrated edge out of a commit-accepted node: the node must be integrated on every +// integration branch its batches moved, and the edge's base ref is not consulted (CRW-965, parent decision D4). +func (s *Scheduler) commitIntegratedEdge(ctx context.Context, q store.Querier, plan string, snap dag.Snapshot, e dag.SnapEdge, from dag.SnapNode, a Acceptance) (EdgeStatus, error) { + refs, err := integrationRefsOf(ctx, q, a.AcceptanceID) + if err != nil { + return EdgeStatus{}, err + } + if len(refs) == 0 { + return wait(e, "the accepted head is not yet moved onto an integration branch"), nil + } + var last IntegratedAt + for _, ref := range refs { + at, err := s.integratedAt(ctx, q, plan, a, e.TargetRepository, ref) + if err != nil { + return EdgeStatus{}, err + } + if at.Unprovable { + return blocked(BlockedIntegrationUnprovable, "a merge landed this head but the integration branch does not contain it"), nil + } + if !at.Satisfied { + return wait(e, "the accepted head is not yet observed in "+e.TargetRepository+" "+ref+" with the merged mark"), nil + } + last = at + } + if st, err := s.stalePredecessor(ctx, q, plan, snap, e, from); err != nil || st != nil { + return valueOf(st), err + } + return EdgeStatus{Satisfied: true, Since: last.Since, AcceptanceID: a.AcceptanceID, ObservationID: last.Observation}, nil +} diff --git a/internal/relay/dagsched/contained_keys_test.go b/internal/relay/dagsched/contained_keys_test.go new file mode 100644 index 0000000000..1bcb71da28 --- /dev/null +++ b/internal/relay/dagsched/contained_keys_test.go @@ -0,0 +1,68 @@ +package dagsched + +import ( + "context" + "encoding/json" + "testing" +) + +// CRW-965 (parent decision, D2): a batch with only contained candidates runs the verifier exactly when the keys of the +// covering record no longer hold for the branch head's tree. +func TestAllContainedBatchVerifiesOnlyWhenTheCoveringKeysChanged(t *testing.T) { + k := newBatchKit(t, batchNode{name: "a", files: map[string]string{"a.txt": "a\n"}}) + k.acceptByCommit("a") + ctx := context.Background() + runs := 0 + base := stubVerifier(writeStubVerifier(t)) + counting := IntegrationBatchDeps{Verify: func(ctx context.Context, dir string, env []string) error { + runs++ + return base(ctx, dir, env) + }, Update: updateIntegrationRef} + first := k.batchIn() + first.Nodes = []string{"a"} + if _, err := k.sched.IntegrateBatch(ctx, first, counting); err != nil { + t.Fatalf("first batch: %v", err) + } + if _, err := k.sched.IntegrateBatch(ctx, k.batchIn(), counting); err != nil { + t.Fatalf("all-contained batch with unchanged keys: %v", err) + } + if runs != 1 { + t.Fatalf("verifier ran %d times; an all-contained batch with unchanged keys must not verify (want 1)", runs) + } +} + +// CRW-965 (parent decision, D2): when the covering record's keys do not hold for the branch head's tree, the batch verifies +// the head without a merge, and the mark is written only after that PASS. +func TestAllContainedBatchReverifiesWhenTheCoveringKeysNoLongerHold(t *testing.T) { + k := newBatchKit(t, batchNode{name: "a", files: map[string]string{"a.txt": "a\n"}}) + k.acceptByCommit("a") + ctx := context.Background() + k.repo.git("update-ref", "refs/heads/dev-int", k.heads["a"]) + detail, err := json.Marshal(map[string]string{"verified_head": k.heads["a"], "integration_ref": "dev-int", "verification_digest": "sha256:old", + "tree": "stale-tree", "ci_digest": "sha256:stale", "dependency_go_sum": "", "dependency_web_lock": ""}) + if err != nil { + t.Fatal(err) + } + if err := k.sched.stageRow(ctx, k.batchIn(), "earlier-batch", "intent", Candidate{}, string(detail)); err != nil { + t.Fatal(err) + } + runs := 0 + base := stubVerifier(writeStubVerifier(t)) + counting := IntegrationBatchDeps{Verify: func(ctx context.Context, dir string, env []string) error { + runs++ + return base(ctx, dir, env) + }, Update: updateIntegrationRef} + res, err := k.sched.IntegrateBatch(ctx, k.batchIn(), counting) + if err != nil { + t.Fatalf("batch: %v", err) + } + if runs != 1 { + t.Fatalf("verifier ran %d times; stale keys must be verified again (want 1)", runs) + } + if len(res.AlreadyContained) != 1 || res.AlreadyContained[0].MarkedEvent == "" { + t.Fatalf("want a marked contained candidate after the PASS: %+v", res.AlreadyContained) + } + if tip, _ := k.branchTip("dev-int"); tip != k.heads["a"] { + t.Fatalf("the branch moved to %s; a contained-only batch moves no ref", tip) + } +} diff --git a/internal/relay/dagsched/edges.go b/internal/relay/dagsched/edges.go index e1c3fe29b1..4c2ab44290 100644 --- a/internal/relay/dagsched/edges.go +++ b/internal/relay/dagsched/edges.go @@ -326,12 +326,22 @@ func (s *Scheduler) artifactVerified(ctx context.Context, q store.Querier, plan } // 7. a code pin needs the accepted head, the target and the pull request to read it from. if e.PinsCodeHead { - var forge int + // A pull-request acceptance is read through its forge row and pull request; a commit acceptance (CRW-965) has + // its verification row and no pull request. Either one needs the accepted head and the edge's target. + var forge, verified int hasForge, err := queryOne(ctx, q, "SELECT 1 FROM dag_acceptance_forge WHERE acceptance_id = ?", []any{a.AcceptanceID}, &forge) if err != nil { return EdgeStatus{}, err } - if a.HeadSHA == "" || a.PRNumber < 1 || !hasForge || a.Repository != e.TargetRepository { + // the verification table is read only for an acceptance without a forge row: a store that predates it still reads its pull requests (CRW-965 review) + hasVerification := false + if !hasForge { + if hasVerification, err = queryOne(ctx, q, "SELECT 1 FROM dag_acceptance_verifications WHERE acceptance_id = ?", []any{a.AcceptanceID}, &verified); err != nil { + return EdgeStatus{}, err + } + } + readable := (hasForge && a.PRNumber >= 1) || (hasVerification && !hasForge) + if a.HeadSHA == "" || !readable || a.Repository != e.TargetRepository { return blocked(BlockedAcceptanceIncomplete, "a pinned acceptance needs its head, its pull request and the edge's target"), nil } } @@ -452,6 +462,12 @@ func (s *Scheduler) integratedEdge(ctx context.Context, q store.Querier, plan st if _, _, st, err := s.recordedEvidence(ctx, q, a); err != nil || st != nil { return valueOf(st), err } + // a commit-accepted node is judged on the integration branches its batches moved, not on the base ref its edge names (CRW-965, D4) + if commit, err := commitAccepted(ctx, q, a.AcceptanceID); err != nil { + return EdgeStatus{}, err + } else if commit { + return s.commitIntegratedEdge(ctx, q, plan, snap, e, from, a) + } at, err := s.integratedAt(ctx, q, plan, a, e.TargetRepository, e.TargetBaseRef) if err != nil { return EdgeStatus{}, err diff --git a/internal/relay/dagsched/integrate_local_test.go b/internal/relay/dagsched/integrate_local_test.go new file mode 100644 index 0000000000..877c30134b --- /dev/null +++ b/internal/relay/dagsched/integrate_local_test.go @@ -0,0 +1,45 @@ +package dagsched + +import ( + "context" + "testing" +) + +// c4 (CRW-965): an implementation node that landed on a local integration branch with no pull request is observed +// there and counted as integrated by the progress reading. The completion is the observation and the parent's merged +// mark, not a merged pull request. +func TestLocalIntegrationCountsAsIntegratedWithoutAPullRequest(t *testing.T) { + k := newLegacyLocalIntegrationKit(t) + repo := k.repo + repo.git("checkout", "-q", "-b", "feature") + feature := repo.commit("feature.txt", "feature") + repo.git("checkout", "-q", "dev") + k.declare("g", "I", "feature.txt") + a := k.acceptNode("g", "I", acceptOpts{HeadSHA: feature}) + k.holdSlotsFor("g", "I") + repo.git("merge", "-q", "--no-ff", "-m", "merge feature", "feature") + k.mark(a) + res, err := k.observe() + if err != nil || !res.Integrated { + t.Fatalf("observe on the local branch = %v %+v; want integrated", err, res) + } + p, err := k.sched.ReadProgress(context.Background(), "g") + if err != nil { + t.Fatal(err) + } + if p.Cumulative.Integrated.Nodes != 1 { + t.Fatalf("integrated count %d; want 1", p.Cumulative.Integrated.Nodes) + } + var node *NodeProgress + for i := range p.Nodes { + if p.Nodes[i].NodeID == "I" { + node = &p.Nodes[i] + } + } + if node == nil || node.Stage != StageIntegrated { + t.Fatalf("node I = %+v; want stage integrated", node) + } + if node.Links.PullRequest != nil { + t.Fatalf("node I names a pull request %+v; the path has none", node.Links.PullRequest) + } +} diff --git a/internal/relay/dagsched/integration_5xx_test.go b/internal/relay/dagsched/integration_5xx_test.go new file mode 100644 index 0000000000..bdf9dbe1c7 --- /dev/null +++ b/internal/relay/dagsched/integration_5xx_test.go @@ -0,0 +1,57 @@ +package dagsched + +import ( + "context" + "os" + "path/filepath" + "testing" +) + +// CRW-965 (parent decision, D5): a remote that answers ls-remote and rejects the push with an HTTP 503 status text is an +// outage: the push is deferred, the acceptance, integration, observation and integrated count are unaffected, and the same +// push completes once the remote accepts it. +func TestServerErrorOnPushDefersOnlyThePush(t *testing.T) { + k := newBatchKit(t, batchNode{name: "a", files: map[string]string{"a.txt": "a\n"}}, batchNode{name: "b", files: map[string]string{"b.txt": "b\n"}}) + remote := bareRemote(t) + k.pushRepo(t, remote) + hook := filepath.Join(remote, "hooks", "pre-receive") + if err := os.WriteFile(hook, []byte("#!/bin/sh\necho 'error: The requested URL returned error: 503' >&2\nexit 1\n"), 0o700); err != nil { + t.Fatal(err) + } + k.acceptByCommit("a") + k.acceptByCommit("b") + ctx := context.Background() + res, err := k.sched.IntegrateBatch(ctx, k.batchIn(), IntegrationBatchDeps{Verify: stubVerifier(writeStubVerifier(t)), Update: updateIntegrationRef}) + if err != nil || len(res.Merged) != 2 { + t.Fatalf("batch while the remote answers 503: %+v, %v", res, err) + } + k.sched.Tips = newLegacyLocalTipReader(t) + k.sched.Ancestry = GitAncestry{}.Ancestry + for _, node := range []string{"a", "b"} { + obs, err := k.sched.ObserveIntegration(ctx, "g", node, "parent", []Target{{Repository: k.repo.path, BaseRef: "dev-int"}}) + if err != nil || !obs.Integrated { + t.Fatalf("observe %s during the outage: %+v, %v; want integrated", node, obs, err) + } + } + progress, err := k.sched.ReadProgress(ctx, "g") + if err != nil || progress.Cumulative.Integrated.Nodes != 2 { + t.Fatalf("integrated count during the outage: %v, %v; want 2", progress.Cumulative.Integrated.Nodes, err) + } + deferred, err := PushIntegration(ctx, k.repo.path, "origin", "dev", "dev-int", k.sched.VerifiedMoveOnto) + if err != nil || deferred.Outcome != PushDeferred { + t.Fatalf("push answered 503: %+v, %v; want deferred", deferred, err) + } + if got := remoteBranch(t, remote, "dev"); got != "" { + t.Fatalf("the remote dev was written while the remote answered 503: %s", got) + } + if err := os.Remove(hook); err != nil { + t.Fatal(err) + } + done, err := PushIntegration(ctx, k.repo.path, "origin", "dev", "dev-int", k.sched.VerifiedMoveOnto) + if err != nil || done.Outcome != PushPushed { + t.Fatalf("push after the remote accepts again: %+v, %v; want pushed", done, err) + } + if got := remoteBranch(t, remote, "dev"); got != res.NewHead { + t.Fatalf("remote dev is %s; want %s", got, res.NewHead) + } +} diff --git a/internal/relay/dagsched/integration_batch.go b/internal/relay/dagsched/integration_batch.go new file mode 100644 index 0000000000..843ad83d87 --- /dev/null +++ b/internal/relay/dagsched/integration_batch.go @@ -0,0 +1,1103 @@ +package dagsched + +import ( + "context" + "crypto/sha256" + "encoding/hex" + "encoding/json" + "errors" + "fmt" + "os" + "path/filepath" + "runtime" + "sort" + "strings" + + "github.com/thisisjun786/codex-relay-workflow/internal/contract" + "github.com/thisisjun786/codex-relay-workflow/internal/relay/registry" + "github.com/thisisjun786/codex-relay-workflow/internal/relay/store" +) + +// The integration batch (CRW-965). One batch takes the plan's ready accepted candidates, merges them in node-id order +// with merge commits in a temporary detached worktree, verifies the merged tree (the verify command runs in that +// worktree with a fresh record path, and the record is judged against that exact tree), and moves the local +// integration branch to the verified commit by a compare-and-swap on the head it read. When the merged tree fails, the +// failing candidate is found by bisecting the ordered prefixes; it and every candidate that depends on it through the +// plan's edges are left out with a reason. A candidate that conflicts is deferred and, once the surviving set is +// verified, retried once in order on top of it. +// +// Each candidate is frozen before git runs (an intent row), so the merged marks name the acceptance and event the merge +// stood on. A batch whose marks did not all land records mark_pending, and the next run completes them from the frozen +// rows, so a partial integration is always visible and never repeated. + +// zeroObjectID is git's create-only expected value for update-ref. +const zeroObjectID = "0000000000000000000000000000000000000000" + +// IntegrationVerifierHostError is a verify command that could not be started at all (a missing executable, a bad +// directory): not a failed verification, so the batch stops instead of splitting every candidate out. +type IntegrationVerifierHostError struct{ Detail string } + +func (e *IntegrationVerifierHostError) Error() string { return e.Detail } + +// IntegrationBatchVerifier runs the one verification command in dir with the extra environment. A non-nil error is a +// failing run (a non-zero exit); the command decides the result only together with the record it writes. +type IntegrationBatchVerifier func(ctx context.Context, dir string, env []string) error + +// IntegrationBatchDeps are the seams the batch's verification and ref update go through, so a test can interleave a ref +// move between the read and the update. +type IntegrationBatchDeps struct { + Verify IntegrationBatchVerifier + Update func(ctx context.Context, checkout, ref, newCommit, oldCommit string) error + // AfterMove runs once the branch has moved and the moved record is committed, before the marks are written. It is a test seam + // for a change that lands between the move and the marks; it runs outside the store transaction (CRW-965, D6). + AfterMove func(ctx context.Context) error +} + +// IntegrationBatchInput is what one batch is asked to do. +type IntegrationBatchInput struct { + Plan, Actor, Checkout, IntegrationRef, BaseRef string + Nodes []string +} + +// IntegrationBatchMerged is one candidate the batch merged onto the branch. +type IntegrationBatchMerged struct { + NodeID, AcceptanceID, HeadSHA, MergeCommit string +} + +// IntegrationBatchSplit is one candidate the batch left out, with the reason it is returned to its parent. +type IntegrationBatchSplit struct { + NodeID, AcceptanceID, HeadSHA, Reason string +} + +// IntegrationBatchResult is the answer of one batch. +type IntegrationBatchResult struct { + BatchID, Plan, Checkout, Ref, BaseRef string + OldHead, NewHead string + Merged []IntegrationBatchMerged + Split []IntegrationBatchSplit + Verification VerificationRecord + VerificationDigest string + MarkedEvents []string + Pending []string + Targets []string + AlreadyContained []IntegrationBatchContained + ContainedUnverified []IntegrationBatchContained + Reconciled []string + Abandoned []string +} + +// IntegrateBatch runs one batch: it completes the marks earlier batches left pending when their commit is on the +// branch, then merges, verifies and moves the branch for the plan's ready candidates. +func (s *Scheduler) IntegrateBatch(ctx context.Context, in IntegrationBatchInput, deps IntegrationBatchDeps) (IntegrationBatchResult, error) { + out := IntegrationBatchResult{Plan: in.Plan, Checkout: in.Checkout, Ref: in.IntegrationRef, BaseRef: in.BaseRef} + if in.Plan == "" || in.Actor == "" { + return out, refuse(contract.RefusalMalformedReceipt, "an integration batch names --plan and --actor") + } + if in.Checkout == "" || in.IntegrationRef == "" || in.BaseRef == "" { + return out, refuse(contract.RefusalMalformedReceipt, "an integration batch names --checkout, --integration-ref and --base") + } + if deps.Verify == nil || deps.Update == nil { + return out, errors.New("an integration batch needs a verifier and a ref updater") + } + if _, err := runGit(ctx, in.Checkout, nil, "rev-parse", "--git-dir"); err != nil { + return out, refuse(contract.RefusalMergeTargetUnreadable, "%s is not a repository git can read", in.Checkout) + } + // the coordinator-epoch fence is checked before anything is read or written + if err := s.IntegrationWrite(ctx, in.Plan, in.Actor, func(context.Context) error { return nil }); err != nil { + return out, err + } + baseTip, err := resolveIntegrationCommit(ctx, in.Checkout, in.BaseRef) + if err != nil { + return out, refuse(contract.RefusalMergeTargetUnreadable, "%s names no commit in %s: %v", in.BaseRef, in.Checkout, err) + } + start, old := baseTip, zeroObjectID + if head, found, err := integrationBranchTip(ctx, in.Checkout, in.IntegrationRef); err != nil { + return out, refuse(contract.RefusalMergeTargetUnreadable, "git could not read %s in %s: %v", in.IntegrationRef, in.Checkout, err) + } else if found { + start, old = head, head + } + out.OldHead = old + if err := s.completePendingMarks(ctx, in); err != nil { + return out, err + } + if old != zeroObjectID { + if err := s.reconcilePlannedMoves(ctx, in, old, &out); err != nil { + return out, err + } + } + candidates, err := s.readyIntegrationCandidates(ctx, in) + if err != nil { + return out, err + } + if len(candidates) == 0 { + return out, refuse(contract.RefusalDispositionConflict, "plan %s has no ready accepted candidate to integrate", in.Plan) + } + batch := integrationBatchID(in, old, candidates) + out.BatchID = batch + if err := s.recordIntent(ctx, in, batch, old, baseTip, candidates); err != nil { + return out, err + } + // A candidate whose head the branch already contains is never merged again (CRW-965). When the branch head is a head + // this relay verified, the candidate is marked from its frozen row; otherwise it is reported and stays ready. + var fresh, contained []Candidate + for _, c := range candidates { + // containment is judged against the branch head the batch read once (start), the same head the coverage check reads (CRW-965 review) + inside := false + if old != zeroObjectID { + var err error + if inside, err = commitIsAncestor(ctx, in.Checkout, c.HeadSHA, start); err != nil { + return out, err + } + } + if inside { + contained = append(contained, c) + } else { + fresh = append(fresh, c) + } + } + var covered []Candidate + if len(contained) > 0 { + rowFound, coveredNodes, err := s.verifiedHeadState(ctx, in, start, contained) + if err != nil { + return out, err + } + // a candidate that no stored verification covers (its keys changed, or its criteria set is not the one verified) makes the + // head verified again, with no merge; only that PASS covers the candidates (CRW-965, parent decisions D2 and D1) + if rowFound && !allNodesCovered(contained, coveredNodes) { + _, dig, pass, err := s.verifyHead(ctx, in, deps, start, baseTip) + if err != nil { + return out, err + } + if pass { + if err := s.recordVerifiedHead(ctx, in, batch, start, dig, contained); err != nil { + return out, err + } + coveredNodes = nodesOf(contained) + } + } + for _, c := range contained { + if coveredNodes[c.NodeID] { + covered = append(covered, c) + continue + } + out.ContainedUnverified = append(out.ContainedUnverified, IntegrationBatchContained{NodeID: c.NodeID, AcceptanceID: c.AcceptanceID, HeadSHA: c.HeadSHA, ContainedIn: start}) + } + } + var settled settledIntegration + if len(fresh) > 0 { + if settled, err = s.settleCandidates(ctx, in, deps, start, baseTip, fresh); err != nil { + return out, err + } + } + out.Split = settled.split + for _, m := range settled.merged { + out.Merged = append(out.Merged, IntegrationBatchMerged{NodeID: m.NodeID, AcceptanceID: m.AcceptanceID, HeadSHA: m.HeadSHA, MergeCommit: m.MergeCommit}) + } + if len(settled.merged) == 0 { + out.NewHead = old + } else { + out.NewHead, out.Verification, out.VerificationDigest = settled.head, settled.record, settled.digest + // the coordinator-epoch fence is checked again right before the branch moves: verification can take long, and a + // session that lost its epoch in the meantime must not move the branch (finding d3) + if err := s.IntegrationWrite(ctx, in.Plan, in.Actor, func(context.Context) error { return nil }); err != nil { + return out, err + } + // the verified head is recorded before the branch moves, so a batch that dies after the move leaves its head known + if err := s.recordVerifiedHead(ctx, in, batch, settled.head, settled.digest, mergedCandidates(settled.merged)); err != nil { + return out, err + } + // the compare-and-swap and the moved record share one fenced transaction: the epoch is checked inside it (CRW-965, D6) + if err := s.IntegrationWrite(ctx, in.Plan, in.Actor, func(txCtx context.Context) error { + // every frozen candidate is proved current again inside the fenced transaction, before the branch moves: a candidate + // that changed after the verification is never published on the old verification (CRW-965, D3) + for _, m := range settled.merged { + current, err := s.frozenCandidateStillCurrent(txCtx, m.Candidate) + if err != nil { + return err + } + if !current { + return refuse(contract.RefusalMergeCandidateMoved, "candidate %s (acceptance %s) is no longer current after the verification of %s: the branch did not move, and the next batch freezes again without it", m.NodeID, m.AcceptanceID, in.IntegrationRef) + } + } + if err := deps.Update(txCtx, in.Checkout, in.IntegrationRef, settled.head, old); err != nil { + return refuse(contract.RefusalStaleMarkContext, "%s moved while the merged tree was being verified (the batch read %s): read it again and run the batch again", in.IntegrationRef, old) + } + return s.recordMovedIn(txCtx, in, batch, out) + }); err != nil { + return out, err + } + if deps.AfterMove != nil { + if err := deps.AfterMove(ctx); err != nil { + return out, err + } + } + for _, m := range settled.merged { + event, err := s.MarkFrozen(ctx, m.Candidate, in.Actor, settled.digest) + if err != nil { + out.Pending = append(out.Pending, m.NodeID) + if rerr := s.recordMark(ctx, in, batch, m.Candidate, "mark_pending", err.Error()); rerr != nil { + return out, rerr + } + continue + } + out.MarkedEvents = append(out.MarkedEvents, event) + if rerr := s.recordMark(ctx, in, batch, m.Candidate, "marked", event); rerr != nil { + return out, rerr + } + } + } + if err := s.markContained(ctx, in, batch, covered, start, &out); err != nil { + return out, err + } + if len(settled.merged) > 0 { + out.Targets = []string{in.Checkout + "@" + in.IntegrationRef} + } + return out, err +} + +// integrationMerge is one candidate the verified set holds, with the merge commit it produced. +type integrationMerge struct { + Candidate + MergeCommit string +} + +// settledIntegration is the verified outcome: what merged onto which head, what was left out, and the record of that +// exact merged tree. +type settledIntegration struct { + merged []integrationMerge + split []IntegrationBatchSplit + head string + record VerificationRecord + digest string +} + +// readyIntegrationCandidates is the candidates a batch considers: the named subset or every ready accepted candidate. +// Every candidate's relationship must be the batch's actor's, so a parent integrates only its own nodes. +func (s *Scheduler) readyIntegrationCandidates(ctx context.Context, in IntegrationBatchInput) ([]Candidate, error) { + all, err := s.AcceptedCandidates(ctx, in.Plan) + if err != nil { + return nil, err + } + all = candidatesInCheckout(all, in.Checkout) + pick := all + if len(in.Nodes) > 0 { + byNode := map[string]Candidate{} + for _, c := range all { + byNode[c.NodeID] = c + } + pick = nil + picked := map[string]bool{} + for _, id := range in.Nodes { + if picked[id] { + continue + } + picked[id] = true + c, ok := byNode[id] + if !ok { + landed, err := s.alreadyIntegratedNode(ctx, in.Plan, id) + if err != nil { + return nil, err + } + if landed { + return nil, refuse(contract.RefusalDispositionConflict, "node %s is already integrated into an integration target, so it has no candidate to merge again", id) + } + return nil, refuse(contract.RefusalDispositionConflict, "node %s is not a ready accepted candidate of plan %s in this checkout (a candidate accepted for another repository is judged there)", id, in.Plan) + } + pick = append(pick, c) + } + } + for _, c := range pick { + rel, found, err := currentRelationshipOf(ctx, s.Store.Q(ctx), in.Plan, c.NodeID) + if err != nil { + return nil, err + } + if !found { + return nil, refuse(contract.RefusalUnregisteredRelationship, "node %s has no execution to integrate", c.NodeID) + } + if rel.ParentTaskID != in.Actor { + return nil, notParentHeldBy(in.Actor, rel) + } + } + sort.Slice(pick, func(i, j int) bool { return pick[i].NodeID < pick[j].NodeID }) + return pick, nil +} + +// integrationBatchID is the identity of one batch: the plan, the checkout, the branch, its old head and the frozen +// candidates, so the same batch recorded twice is the same identity. +func integrationBatchID(in IntegrationBatchInput, old string, candidates []Candidate) string { + var parts []string + for _, c := range candidates { + parts = append(parts, c.NodeID+"="+c.AcceptanceID+"@"+c.EventID+"#"+c.HeadSHA) + } + return registry.CoordinationID("dib", in.Plan, in.Checkout, in.IntegrationRef, old, in.BaseRef, strings.Join(parts, ",")) +} + +// recordIntent writes the intent before any git call: one batch-level row and one row per frozen candidate. +func (s *Scheduler) recordIntent(ctx context.Context, in IntegrationBatchInput, batch, old, baseTip string, candidates []Candidate) error { + rows, err := store.IntegrationStagesOfPlan(ctx, s.Store, in.Plan) + if err != nil { + return err + } + for _, r := range rows { + if r.BatchID == batch && r.Stage == "intent" { + // the same batch recorded its intent before: the identity is the same, so the rows stand + return nil + } + } + detail, err := json.Marshal(map[string]string{"integration_ref": in.IntegrationRef, "base": baseTip, "old_head": old}) + if err != nil { + return err + } + return s.IntegrationWrite(ctx, in.Plan, in.Actor, func(txCtx context.Context) error { + if err := s.stageRow(txCtx, in, batch, "intent", Candidate{}, string(detail)); err != nil { + return err + } + for _, c := range candidates { + // each frozen row names the criteria set the batch verifies the candidate under (CRW-965, parent decision d2) + if err := s.stageRow(txCtx, in, batch, "intent", c, c.CriteriaSetDigest); err != nil { + return err + } + } + return nil + }) +} + +// recordMoved writes the batch row once the branch moved: the merged and split lists, the verified record's digest and the + +// recordMark writes one node's mark outcome: marked with the event it named, or mark_pending with the refusal. +func (s *Scheduler) recordMark(ctx context.Context, in IntegrationBatchInput, batch string, c Candidate, stage, detail string) error { + return s.IntegrationWrite(ctx, in.Plan, in.Actor, func(txCtx context.Context) error { + return s.stageRow(txCtx, in, batch, stage, c, detail) + }) +} + +// stageRow appends one stage row. A batch-level row carries no node; a node row carries the frozen identity. +func (s *Scheduler) stageRow(ctx context.Context, in IntegrationBatchInput, batch, stage string, c Candidate, detail string) error { + stageID := registry.CoordinationID("dis", batch, stage, c.NodeID, detail) + return store.RecordIntegrationStage(ctx, s.Store, store.IntegrationStageRow{StageID: stageID, BatchID: batch, PlanID: in.Plan, Stage: stage, + NodeID: c.NodeID, AcceptanceID: c.AcceptanceID, EventID: c.EventID, RevisionHash: c.RevisionHash, Generation: c.Generation, + HeadSHA: c.HeadSHA, Detail: detail, RecordedBy: in.Actor, RecordedAt: registry.SystemISO()}) +} + +// completePendingMarks completes the marks earlier batches left pending. A pending mark is completed only when its +// batch's merge commit is on the integration branch now, from the frozen candidate; a mark already present counts as done. +func (s *Scheduler) completePendingMarks(ctx context.Context, in IntegrationBatchInput) error { + rows, err := store.IntegrationStagesOfPlan(ctx, s.Store, in.Plan) + if err != nil { + return err + } + done := map[string]bool{} + for _, r := range rows { + if r.Stage == "marked" { + done[r.BatchID+"/"+r.NodeID] = true + } + } + for _, r := range rows { + if r.Stage != "mark_pending" || done[r.BatchID+"/"+r.NodeID] { + continue + } + moved := false + for _, x := range rows { + if x.BatchID == r.BatchID && x.Stage == "ref_moved" { + ok, err := isAncestorOf(ctx, in.Checkout, x.Detail, in.IntegrationRef) + if err != nil { + return err + } + moved = ok + } + } + if !moved { + continue + } + c := Candidate{PlanID: in.Plan, NodeID: r.NodeID, AcceptanceID: r.AcceptanceID, EventID: r.EventID, RevisionHash: r.RevisionHash, + Generation: r.Generation, HeadSHA: r.HeadSHA} + event, err := s.MarkFrozen(ctx, c, in.Actor, r.BatchID) + if err != nil { + continue + } + if err := s.recordMark(ctx, in, r.BatchID, c, "marked", event); err != nil { + return err + } + } + return nil +} + +// settleCandidates merges, verifies and settles the candidates in a temporary worktree, removed on every path. +func (s *Scheduler) settleCandidates(ctx context.Context, in IntegrationBatchInput, deps IntegrationBatchDeps, start, baseTip string, candidates []Candidate) (settledIntegration, error) { + tmp, err := os.MkdirTemp("", "crw-965-integrate-") + if err != nil { + return settledIntegration{}, err + } + wt := filepath.Join(tmp, "worktree") + defer func() { + _, _ = runGit(context.WithoutCancel(ctx), in.Checkout, nil, "worktree", "remove", "--force", wt) + _, _ = runGit(context.WithoutCancel(ctx), in.Checkout, nil, "worktree", "prune") + _ = os.RemoveAll(tmp) + }() + if _, err := runGit(ctx, in.Checkout, nil, "worktree", "add", "--detach", wt, start); err != nil { + return settledIntegration{}, refuse(contract.RefusalMergeTargetUnreadable, "git could not create a worktree at %s: %v", start, err) + } + w := &integrationWorktree{s: s, in: in, deps: deps, dir: wt, tmp: tmp, start: start, baseTip: baseTip} + return w.settle(ctx, candidates) +} + +// integrationWorktree is the temporary worktree a batch merges into and verifies in. +type integrationWorktree struct { + s *Scheduler + in IntegrationBatchInput + deps IntegrationBatchDeps + dir string + tmp string + start string + baseTip string + records int +} + +// integrationIdentity is the author and committer of the merge commits a batch makes. +var integrationIdentity = []string{"GIT_AUTHOR_NAME=crw-relay", "GIT_AUTHOR_EMAIL=crw-relay@invalid", "GIT_COMMITTER_NAME=crw-relay", "GIT_COMMITTER_EMAIL=crw-relay@invalid"} + +// reset moves the worktree to a commit. +func (w *integrationWorktree) reset(ctx context.Context, commit string) error { + _, err := runGit(ctx, w.dir, nil, "reset", "--hard", "-q", commit) + return err +} + +// head is the worktree's commit. +func (w *integrationWorktree) head(ctx context.Context) (string, error) { + out, err := runGit(ctx, w.dir, nil, "rev-parse", "HEAD") + return strings.TrimSpace(out), err +} + +// merge merges one candidate into the worktree with a merge commit. A conflict is an answer (false) and leaves the +// worktree as it was; a host failure is an error. +func (w *integrationWorktree) merge(ctx context.Context, c Candidate) (bool, string, error) { + code, _, err := runGitExit(ctx, w.dir, integrationIdentity, "merge", "--no-ff", "--no-edit", "-q", "-m", "CRW-965: integrate "+c.NodeID, c.HeadSHA) + if code < 0 { + return false, "", fmt.Errorf("git merge of %s: %v", c.HeadSHA, err) + } + if code != 0 { + if code != 1 { + // git answers 1 for a conflict; anything else (for example 128, a head this checkout does not hold) is a host failure, not a conflict + return false, "", fmt.Errorf("git merge of %s exited %d: %v", c.HeadSHA, code, err) + } + _, _, _ = runGitExit(ctx, w.dir, integrationIdentity, "merge", "--abort") + return false, "", nil + } + head, err := w.head(ctx) + return true, head, err +} + +// build resets the worktree to the start and merges the given candidates in order; a prefix that merged cleanly once +// merges cleanly again. It answers the merge commits. +func (w *integrationWorktree) build(ctx context.Context, set []Candidate) ([]string, *Candidate, error) { + if err := w.reset(ctx, w.start); err != nil { + return nil, nil, err + } + var commits []string + for i := range set { + c := set[i] + ok, commit, err := w.merge(ctx, c) + if err != nil { + return nil, nil, err + } + if !ok { + // a survivor that no longer merges cleanly is reported, not an error: the settle loop defers it (CRW-965, decision D-A) + return nil, &c, nil + } + commits = append(commits, commit) + } + return commits, nil, nil +} + +// verify runs the verification in the worktree at its current head, with a fresh record path, and judges the record +// against that merged tree. A non-zero exit is a failure whatever the record says; a record that does not pass is a +// failure with the judge's reason. Only a host failure is an error. +func (w *integrationWorktree) verify(ctx context.Context) (VerificationRecord, string, bool, error) { + commit, err := w.head(ctx) + if err != nil { + return VerificationRecord{}, "", false, err + } + w.records++ + dir := filepath.Join(w.tmp, fmt.Sprintf("record-%d", w.records)) + if err := os.Mkdir(dir, 0o700); err != nil { + return VerificationRecord{}, "", false, err + } + recordPath := filepath.Join(dir, "verification-record.json") + env := []string{"CRW_VERIFY_RECORD=" + recordPath, "CRW_VERIFY_BASE=" + w.baseTip, "CRW_VERIFY_HEAD=" + commit} + if err := w.deps.Verify(ctx, w.dir, env); err != nil { + var host *IntegrationVerifierHostError + if errors.As(err, &host) { + return VerificationRecord{}, "", false, err + } + return VerificationRecord{}, "", false, nil + } + // the verify command may move HEAD; what it verified must still be the commit that would land (CRW-965 review) + if moved, err := w.head(ctx); err != nil { + return VerificationRecord{}, "", false, err + } else if moved != commit { + return VerificationRecord{}, "", false, nil + } + raw, err := os.ReadFile(recordPath) + if err != nil { + return VerificationRecord{}, "", false, nil + } + keys, err := CommitVerificationKeys(ctx, w.in.Checkout, commit) + if err != nil { + return VerificationRecord{}, "", false, err + } + base, err := resolveIntegrationCommit(ctx, w.in.Checkout, w.baseTip) + if err != nil { + return VerificationRecord{}, "", false, err + } + keys.Base = base + keys.OS, keys.Arch = runtime.GOOS, runtime.GOARCH + record, err := JudgeVerificationRecord(raw, keys) + if err != nil { + var refused *store.RefusedError + if errors.As(err, &refused) { + return record, "", false, nil + } + return record, "", false, err + } + return record, sha256Digest(raw), true, nil +} + +// settle is the merge-verify-split loop (see settleCandidates). The worktree is at the verified head when it returns. +func (w *integrationWorktree) settle(ctx context.Context, candidates []Candidate) (settledIntegration, error) { + var out settledIntegration + if err := w.reset(ctx, w.start); err != nil { + return out, err + } + var kept, deferred []Candidate + for _, c := range candidates { + ok, _, err := w.merge(ctx, c) + if err != nil { + return out, err + } + if ok { + kept = append(kept, c) + } else { + deferred = append(deferred, c) + } + } + if err := w.reset(ctx, w.start); err != nil { + return out, err + } + // the merged set is verified once; when it fails, the first failing prefix is found by bisection and its candidate + // and every dependant of it are left out, until the rest verifies + var commits []string + var record VerificationRecord + var digest string + excluded := map[string]string{} + for len(kept) > 0 { + var err error + var conflicted *Candidate + if commits, conflicted, err = w.build(ctx, kept); err != nil { + return out, err + } + if conflicted != nil { + // it no longer merges with the set left after a removal: it is deferred and retried on the verified set + deferred = append(deferred, *conflicted) + kept = withoutCandidateNode(kept, conflicted.NodeID) + continue + } + rec, dig, pass, err := w.verify(ctx) + if err != nil { + return out, err + } + if pass { + record, digest = rec, dig + break + } + k, err := w.firstFailingPrefix(ctx, kept) + if err != nil { + return out, err + } + failing := kept[k-1] + // the bisection shows only that this candidate breaks the prefix before it; it is reported as failing only when it fails + // verified alone, and otherwise it is left for the next batch (CRW-965, parent decision D5) + reason := "verification_failed" + alone, err := w.failsAlone(ctx, failing) + if err != nil { + return out, err + } + if !alone { + reason = "not_proven_failing" + } + left := map[string]string{failing.NodeID: reason} + edges, err := w.s.Successors(ctx, w.in.Plan) + if err != nil { + return out, err + } + for _, d := range transitiveSuccessors(edges, failing.NodeID) { + left[d] = "depends_on_" + failing.NodeID + } + for node, reason := range left { + excluded[node] = reason + } + var next []Candidate + for _, c := range kept { + if reason, gone := left[c.NodeID]; gone { + out.split = append(out.split, IntegrationBatchSplit{NodeID: c.NodeID, AcceptanceID: c.AcceptanceID, HeadSHA: c.HeadSHA, Reason: reason}) + continue + } + next = append(next, c) + } + kept = next + } + for i, c := range kept { + out.merged = append(out.merged, integrationMerge{Candidate: c, MergeCommit: commits[i]}) + } + if len(kept) == 0 { + // nothing verified: the deferred candidates retry on top of the start, not on a failing merge + if err := w.reset(ctx, w.start); err != nil { + return out, err + } + } + // the deferred candidates: each is retried once, in order, on top of the verified set + for _, d := range deferred { + if reason, gone := excluded[d.NodeID]; gone { + out.split = append(out.split, IntegrationBatchSplit{NodeID: d.NodeID, AcceptanceID: d.AcceptanceID, HeadSHA: d.HeadSHA, Reason: reason}) + continue + } + before, err := w.head(ctx) + if err != nil { + return out, err + } + ok, commit, err := w.merge(ctx, d) + if err != nil { + return out, err + } + if !ok { + reason := "conflict" + if len(kept) > 0 { + reason = "conflict_with_" + w.conflictNode(ctx, d, kept) + } + out.split = append(out.split, IntegrationBatchSplit{NodeID: d.NodeID, AcceptanceID: d.AcceptanceID, HeadSHA: d.HeadSHA, Reason: reason}) + continue + } + rec, dig, pass, err := w.verify(ctx) + if err != nil { + return out, err + } + if !pass { + if err := w.reset(ctx, before); err != nil { + return out, err + } + out.split = append(out.split, IntegrationBatchSplit{NodeID: d.NodeID, AcceptanceID: d.AcceptanceID, HeadSHA: d.HeadSHA, Reason: "verification_failed"}) + continue + } + kept = append(kept, d) + out.merged = append(out.merged, integrationMerge{Candidate: d, MergeCommit: commit}) + record, digest = rec, dig + } + head, err := w.head(ctx) + if err != nil { + return out, err + } + out.head, out.record, out.digest = head, record, digest + return out, nil +} + +// firstFailingPrefix is the smallest k such that the first k candidates fail verification, found by bisection over the +// prefixes. The full set is known to fail when this is called. +func (w *integrationWorktree) firstFailingPrefix(ctx context.Context, kept []Candidate) (int, error) { + lo, hi := 1, len(kept) + for lo < hi { + mid := (lo + hi) / 2 + _, conflicted, err := w.build(ctx, kept[:mid]) + if err != nil { + return 0, err + } + if conflicted != nil { + return 0, fmt.Errorf("candidate %s conflicted while a prefix of a clean set was rebuilt", conflicted.NodeID) + } + _, _, pass, err := w.verify(ctx) + if err != nil { + return 0, err + } + if pass { + lo = mid + 1 + } else { + hi = mid + } + } + return lo, nil +} + +// failsAlone is whether a candidate fails verification when it is merged alone onto the start. A candidate that does not +// merge alone, or that verifies, does not fail alone (CRW-965, parent decision D5). +func (w *integrationWorktree) failsAlone(ctx context.Context, c Candidate) (bool, error) { + if err := w.reset(ctx, w.start); err != nil { + return false, err + } + ok, _, err := w.merge(ctx, c) + if err != nil { + return false, err + } + if !ok { + return false, nil + } + _, _, pass, err := w.verify(ctx) + if err != nil { + return false, err + } + return !pass, nil +} + +// conflictNode names the first kept candidate a deferred candidate conflicts with when merged on its own, by git's own +// merge-tree answer (exit 1 is a conflict). The first kept candidate is named when none is found. +func (w *integrationWorktree) conflictNode(ctx context.Context, d Candidate, kept []Candidate) string { + for _, k := range kept { + if code, _, _ := runGitExit(ctx, w.in.Checkout, nil, "merge-tree", "--write-tree", "--name-only", k.HeadSHA, d.HeadSHA); code == 1 { + return k.NodeID + } + } + return "" +} + +// transitiveSuccessors is every node that depends on a node through the plan's edges, sorted. +func transitiveSuccessors(edges map[string][]string, node string) []string { + seen := map[string]bool{} + queue := []string{node} + var out []string + for len(queue) > 0 { + next := queue[0] + queue = queue[1:] + for _, to := range edges[next] { + if !seen[to] { + seen[to] = true + out = append(out, to) + queue = append(queue, to) + } + } + } + sort.Strings(out) + return out +} + +// integrationBranchTip is the commit a local branch names, and whether the branch exists. +func integrationBranchTip(ctx context.Context, checkout, branch string) (string, bool, error) { + code, out, err := runGitExit(ctx, checkout, nil, "rev-parse", "--verify", "-q", "refs/heads/"+branch) + if code == 1 { + return "", false, nil + } + if err != nil { + return "", false, err + } + return strings.TrimSpace(out), true, nil +} + +// resolveIntegrationCommit is the full commit id a revision names in a checkout. +func resolveIntegrationCommit(ctx context.Context, checkout, rev string) (string, error) { + out, err := runGit(ctx, checkout, nil, "rev-parse", "--verify", rev+"^{commit}") + return strings.TrimSpace(out), err +} + +// isAncestorOf reports whether a commit is an ancestor of (or equal to) a branch's commit in a checkout. +func isAncestorOf(ctx context.Context, checkout, commit, branch string) (bool, error) { + tip, found, err := integrationBranchTip(ctx, checkout, branch) + if err != nil || !found { + return false, err + } + code, _, err := runGitExit(ctx, checkout, nil, "merge-base", "--is-ancestor", commit, tip) + if code == 0 { + return true, nil + } + if code == 1 { + return false, nil + } + return false, err +} + +// allNodesCovered is whether every candidate of the set is among the covered nodes. +func allNodesCovered(set []Candidate, covered map[string]bool) bool { + for _, c := range set { + if !covered[c.NodeID] { + return false + } + } + return true +} + +// nodesOf is the set of the node ids of the candidates. +func nodesOf(set []Candidate) map[string]bool { + out := map[string]bool{} + for _, c := range set { + out[c.NodeID] = true + } + return out +} + +// mergedCandidates are the candidates a settled merge holds, in merge order. +func mergedCandidates(merged []integrationMerge) []Candidate { + out := make([]Candidate, 0, len(merged)) + for _, m := range merged { + out = append(out, m.Candidate) + } + return out +} + +// sha256Digest is sha256: of bytes. +func sha256Digest(raw []byte) string { + sum := sha256.Sum256(raw) + return "sha256:" + hex.EncodeToString(sum[:]) +} + +// IntegrationBatchContained is one candidate the branch already contained when the batch ran. A covered one is marked +// (MarkedEvent) or refused (Reason); an uncovered one is only reported and stays ready (ContainedUnverified). +type IntegrationBatchContained struct { + NodeID, AcceptanceID, HeadSHA, ContainedIn string + MarkedEvent, Reason string +} + +// markContained writes the merged marks of the candidates the branch already contained, from their frozen rows (CRW-965). +// A mark this batch already recorded (a retried run) is reported from its stage row and is not inserted again. +func (s *Scheduler) markContained(ctx context.Context, in IntegrationBatchInput, batch string, candidates []Candidate, tip string, out *IntegrationBatchResult) error { + rows, err := store.IntegrationStagesOfPlan(ctx, s.Store, in.Plan) + if err != nil { + return err + } + prior := map[string]string{} + for _, r := range rows { + if r.BatchID == batch && r.Stage == "marked" { + prior[r.NodeID] = r.Detail + } + } + for _, c := range candidates { + row := IntegrationBatchContained{NodeID: c.NodeID, AcceptanceID: c.AcceptanceID, HeadSHA: c.HeadSHA, ContainedIn: tip} + if event, done := prior[c.NodeID]; done { + row.MarkedEvent = event + out.AlreadyContained = append(out.AlreadyContained, row) + continue + } + event, err := s.MarkFrozen(ctx, c, in.Actor, tip) + if err != nil { + row.Reason = err.Error() + out.AlreadyContained = append(out.AlreadyContained, row) + continue + } + row.MarkedEvent = event + out.AlreadyContained = append(out.AlreadyContained, row) + out.MarkedEvents = append(out.MarkedEvents, event) + if rerr := s.recordMark(ctx, in, batch, c, "marked", event); rerr != nil { + return rerr + } + } + return nil +} + +// withoutCandidateNode is the candidates with one node removed, in their order. +func withoutCandidateNode(set []Candidate, node string) []Candidate { + var out []Candidate + for _, c := range set { + if c.NodeID != node { + out = append(out, c) + } + } + return out +} + +// candidatesInCheckout is the candidates accepted for the checkout a batch integrates into: a candidate accepted in +// another repository is judged there, never merged here (CRW-965 review). +func candidatesInCheckout(set []Candidate, checkout string) []Candidate { + var out []Candidate + for _, c := range set { + if c.Repository == checkout { + out = append(out, c) + } + } + return out +} + +// verifiedHeadState reads the verified-head rows this relay wrote for a head on the integration ref. rowFound is whether any +// such row exists. covered names the candidates a stored verification stands for: a row covers a candidate when the keys it +// was judged under still hold for the head's tree, ci.yml, dependencies and platform, and the row names the candidate's +// criteria set as the one it verified (CRW-965, parent decisions D2 and D1). A commit's content does not change, so the keys +// differ only when the verification no longer stands for the tree. +func (s *Scheduler) verifiedHeadState(ctx context.Context, in IntegrationBatchInput, head string, candidates []Candidate) (bool, map[string]bool, error) { + rows, err := store.IntegrationStagesOfPlan(ctx, s.Store, in.Plan) + if err != nil { + return false, nil, err + } + var stored []map[string]string + for _, r := range rows { + if r.Stage != "intent" || r.NodeID != "" { + continue + } + var d map[string]string + if json.Unmarshal([]byte(r.Detail), &d) == nil && d["verified_head"] == head && d["integration_ref"] == in.IntegrationRef { + stored = append(stored, d) + } + } + if len(stored) == 0 { + return false, map[string]bool{}, nil + } + now, err := verifiedKeysOf(ctx, in.Checkout, head) + if err != nil { + return true, map[string]bool{}, err + } + covered := map[string]bool{} + for _, d := range stored { + if !verifiedKeysHold(d, now) { + continue + } + criteria := map[string]string{} + if json.Unmarshal([]byte(d["criteria"]), &criteria) != nil { + continue + } + for _, c := range candidates { + if c.CriteriaSetDigest == "" { + continue + } + // a node the row names is covered under the criteria set it names; a node the row does not name is covered only + // while its criteria set is still the accepted one (a revalidated set was never verified on this head) + named, ok := criteria[c.NodeID] + if (ok && named == c.CriteriaSetDigest) || (!ok && c.CriteriaSetDigest == c.AcceptedCriteriaDigest) { + covered[c.NodeID] = true + } + } + } + return true, covered, nil +} + +// verifiedKeysHold is whether a stored verification stands for the keys the verifier would judge now: the same tree, ci.yml +// digest, dependency digests and platform (CRW-965, parent decisions D2 and D2 remainder). A row with a key missing is not +// reused, so an older row is verified again. +func verifiedKeysHold(stored, now map[string]string) bool { + for _, key := range verifiedKeyNames { + value, ok := stored[key] + if !ok || value != now[key] { + return false + } + } + return true +} + +// verifiedKeyNames are the keys a verified head's row carries, in the order they are judged. +var verifiedKeyNames = []string{"tree", "ci_digest", "dependency_go_sum", "dependency_web_lock", "os", "arch"} + +// verifiedKeysOf is what a verified head's record is judged under: its tree, its ci.yml digest and its dependency digests. +func verifiedKeysOf(ctx context.Context, checkout, head string) (map[string]string, error) { + tree, err := runGit(ctx, checkout, nil, "rev-parse", head+"^{tree}") + if err != nil { + return nil, err + } + keys, err := CommitVerificationKeys(ctx, checkout, head) + if err != nil { + return nil, err + } + return map[string]string{"tree": strings.TrimSpace(tree), "ci_digest": keys.CiDigest, + "dependency_go_sum": keys.Dependencies["go.sum"], "dependency_web_lock": keys.Dependencies["web/package-lock.json"], + "os": runtime.GOOS, "arch": runtime.GOARCH}, nil +} + +// recordVerifiedHead writes the batch's intent to move the branch to a verified head, before the branch moves. The row +// names the tree and the keys the verification stands on, and the criteria set of each candidate it covers (CRW-965, +// parent decisions D2 and D1), so a later batch compares them without reading old records. +func (s *Scheduler) recordVerifiedHead(ctx context.Context, in IntegrationBatchInput, batch, head, digest string, covered []Candidate) error { + keys, err := verifiedKeysOf(ctx, in.Checkout, head) + if err != nil { + return err + } + criteria := map[string]string{} + for _, c := range covered { + criteria[c.NodeID] = c.CriteriaSetDigest + } + criteriaJSON, err := json.Marshal(criteria) + if err != nil { + return err + } + keys["criteria"] = string(criteriaJSON) + keys["verified_head"], keys["verification_digest"], keys["integration_ref"] = head, digest, in.IntegrationRef + detail, err := json.Marshal(keys) + if err != nil { + return err + } + return s.IntegrationWrite(ctx, in.Plan, in.Actor, func(txCtx context.Context) error { + return s.stageRow(txCtx, in, batch, "intent", Candidate{}, string(detail)) + }) +} + +// verifyHead verifies the tree of a branch head in a temporary worktree, with no merge: the record is judged by the same +// judge a merged tree is (CRW-965, parent decision D2). The worktree is removed on every path. +func (s *Scheduler) verifyHead(ctx context.Context, in IntegrationBatchInput, deps IntegrationBatchDeps, head, baseTip string) (VerificationRecord, string, bool, error) { + tmp, err := os.MkdirTemp("", "crw-965-verify-") + if err != nil { + return VerificationRecord{}, "", false, err + } + wt := filepath.Join(tmp, "worktree") + defer func() { + _, _ = runGit(context.WithoutCancel(ctx), in.Checkout, nil, "worktree", "remove", "--force", wt) + _, _ = runGit(context.WithoutCancel(ctx), in.Checkout, nil, "worktree", "prune") + _ = os.RemoveAll(tmp) + }() + if _, err := runGit(ctx, in.Checkout, nil, "worktree", "add", "--detach", wt, head); err != nil { + return VerificationRecord{}, "", false, refuse(contract.RefusalMergeTargetUnreadable, "git could not create a worktree at %s: %v", head, err) + } + w := &integrationWorktree{s: s, in: in, deps: deps, dir: wt, tmp: tmp, start: head, baseTip: baseTip} + return w.verify(ctx) +} + +// recordMovedIn writes the batch row once the branch moved and the ref_moved stage, inside the transaction that moved +// the branch (CRW-965, parent decision D6). +func (s *Scheduler) recordMovedIn(txCtx context.Context, in IntegrationBatchInput, batch string, out IntegrationBatchResult) error { + mergedJSON, err := json.Marshal(out.Merged) + if err != nil { + return err + } + splitJSON, err := json.Marshal(out.Split) + if err != nil { + return err + } + verification, err := json.Marshal(map[string]string{"result": out.Verification.Result, "treeHash": out.Verification.TreeHash, "digest": out.VerificationDigest}) + if err != nil { + return err + } + row := store.IntegrationBatchRow{BatchID: batch, PlanID: in.Plan, Repository: in.Checkout, IntegrationRef: in.IntegrationRef, BaseRef: in.BaseRef, + OldHead: out.OldHead, NewHead: out.NewHead, MergedJSON: string(mergedJSON), SplitJSON: string(splitJSON), VerificationJSON: string(verification), + RecordedBy: in.Actor, CoordinatorEpoch: s.ExpectedEpoch, RecordedAt: registry.SystemISO()} + if err := store.RecordIntegrationBatch(txCtx, s.Store, row); err != nil { + return err + } + return s.stageRow(txCtx, in, batch, "ref_moved", Candidate{}, out.NewHead) +} + +// reconcilePlannedMoves settles the batches that planned a move (CRW-965, parent decision D6). A planned head the branch +// holds and no ref_moved row records is recorded as moved, once: the batch died after the swap. A planned head the branch +// does not hold is abandoned and reported. +func (s *Scheduler) reconcilePlannedMoves(ctx context.Context, in IntegrationBatchInput, tip string, out *IntegrationBatchResult) error { + rows, err := store.IntegrationStagesOfPlan(ctx, s.Store, in.Plan) + if err != nil { + return err + } + moved := map[string]bool{} + for _, r := range rows { + if r.Stage == "ref_moved" { + moved[r.BatchID] = true + } + } + seen := map[string]bool{} + for _, r := range rows { + if r.Stage != "intent" || r.NodeID != "" || moved[r.BatchID] || seen[r.BatchID] { + continue + } + var d map[string]string + if json.Unmarshal([]byte(r.Detail), &d) != nil || d["verified_head"] == "" || d["integration_ref"] != in.IntegrationRef { + continue + } + seen[r.BatchID] = true + if d["verified_head"] != tip { + out.Abandoned = append(out.Abandoned, r.BatchID) + continue + } + batch, head := r.BatchID, tip + if err := s.IntegrationWrite(ctx, in.Plan, in.Actor, func(txCtx context.Context) error { + return s.stageRow(txCtx, in, batch, "ref_moved", Candidate{}, head) + }); err != nil { + return err + } + out.Reconciled = append(out.Reconciled, r.BatchID) + } + return nil +} diff --git a/internal/relay/dagsched/integration_batch_test.go b/internal/relay/dagsched/integration_batch_test.go new file mode 100644 index 0000000000..f1ff1e031a --- /dev/null +++ b/internal/relay/dagsched/integration_batch_test.go @@ -0,0 +1,433 @@ +package dagsched + +import ( + "context" + "encoding/json" + "fmt" + "os" + "os/exec" + "path/filepath" + "runtime" + "strings" + "testing" + + "github.com/thisisjun786/codex-relay-workflow/internal/relay/dag" + "github.com/thisisjun786/codex-relay-workflow/internal/relay/store" +) + +// The integration batch through the production path (CRW-965, criteria c2, c3, c6 and the evaluation's d11). Every +// candidate is a real commit in a real repository, accepted with dag-accept's commit path, marked by the batch, and +// the verification is a real stub script that runs in the merged worktree. Nothing is seeded in SQL. + +// batchNode is one implementation node: the files its feature branch commits on top of dev. +type batchNode struct { + name string + files map[string]string +} + +// batchKit is a plan of implementation nodes over one repository whose dev branch is the base. +type batchKit struct { + *releaseKit + repo *gitRepo + base string + heads map[string]string +} + +func newBatchKit(t *testing.T, nodes ...batchNode) *batchKit { + t.Helper() + k := &batchKit{releaseKit: newReleaseKit(t), repo: newGitRepo(t), heads: map[string]string{}} + k.base = k.repo.git("rev-parse", "dev") + var changes []doc + for _, n := range nodes { + k.repo.git("checkout", "-q", "-B", "feature-"+n.name, k.base) + for path, body := range n.files { + k.repo.write(path, body) + k.repo.git("add", path) + } + k.repo.git("commit", "-q", "-m", "node "+n.name) + k.heads[n.name] = k.repo.git("rev-parse", "HEAD") + changes = append(changes, addRelNode(n.name, dag.NodeImplementation)) + } + k.repo.git("checkout", "-q", "dev") + k.putPlan("g", 0, "g-r1", changes...) + for _, n := range nodes { + r := k.reportNode("g", n.name, acceptOpts{}) + k.exec("INSERT INTO dag_verified_heads (event_id, relationship_id, execution_generation, verdict_turn_id, head_sha, recorded_by_task_id, recorded_at) VALUES (?,?,?,?,?,?,?)", + r.Event, r.Acceptance.RelationshipID, 1, "verdict-turn", k.heads[n.name], "parent", k.clock()) + } + return k +} + +// acceptByCommit accepts one node through dag-accept's commit path with a record the writer would have sealed. +func (k *batchKit) acceptByCommit(node string) { + k.t.Helper() + head := k.heads[node] + record := writeSealedRecord(k.t, k.repo.path, head, k.base, k.treeOf(head), "pass") + if _, err := k.sched.Accept(context.Background(), "g", node, "parent", AcceptInput{ + RuleVersion: VerifierRule{SkillsDigest: dig("skills"), Model: "m", Effort: "none"}, + Commit: &CommitRef{Head: head, Base: k.base, Checkout: k.repo.path, Record: record}, + }); err != nil { + k.t.Fatalf("accept %s: %v", node, err) + } +} + +// treeOf is the tree of a commit in the kit's repository. +func (k *batchKit) treeOf(commit string) string { + k.t.Helper() + return k.repo.git("rev-parse", commit+"^{tree}") +} + +// writeSealedRecord writes a verification-record/1 in CRW-964's format, sealed by the same function the judge reads, +// and answers it as the "@file" spelling the accept path takes. +func writeSealedRecord(t *testing.T, checkout, head, base, tree, result string) string { + t.Helper() + keys, err := CommitVerificationKeys(context.Background(), checkout, head) + if err != nil { + t.Fatal(err) + } + raw, err := SealVerificationRecord(VerificationRecord{Runner: "local", Repository: "owner/repo", BaseCommit: base, HeadCommit: head, + TreeHash: tree, CiDigest: keys.CiDigest, Tools: map[string]string{"go": "go1.27.1"}, Pins: map[string]string{}, GoFlags: "", GoEnv: "", + PinMismatch: []string{}, Dependencies: keys.Dependencies, OS: runtime.GOOS, Arch: runtime.GOARCH, Result: result, Jobs: []json.RawMessage{}}) + if err != nil { + t.Fatal(err) + } + path := filepath.Join(t.TempDir(), "verification-record.json") + if err := os.WriteFile(path, raw, 0o600); err != nil { + t.Fatal(err) + } + return "@" + path +} + +// writeStubVerifier is a real verify command: it exits 1 when a bad.txt marker is present in the worktree it runs in, +// and otherwise writes its record through this test binary (TestVerifyRecordHelper). +func writeStubVerifier(t *testing.T) string { + t.Helper() + bin, err := os.Executable() + if err != nil { + t.Fatal(err) + } + t.Setenv("CRW_TEST_RECORD_HELPER", "1") + t.Setenv("CRW_TEST_HELPER_BIN", bin) + script := filepath.Join(t.TempDir(), "verify.sh") + body := "#!/bin/sh\nif [ -e bad.txt ]; then\n echo 'bad.txt is present: the merged tree fails' >&2\n exit 1\nfi\nexec \"$CRW_TEST_HELPER_BIN\" -test.run='^TestVerifyRecordHelper$' -test.count=1\n" + if err := os.WriteFile(script, []byte(body), 0o700); err != nil { + t.Fatal(err) + } + return script +} + +// stubVerifier runs the verify script in the directory the batch names, with the batch's environment added. +func stubVerifier(script string) IntegrationBatchVerifier { + return func(ctx context.Context, dir string, env []string) error { + cmd := exec.CommandContext(ctx, script) + cmd.Dir = dir + cmd.Env = append(os.Environ(), env...) + out, err := cmd.CombinedOutput() + if err != nil { + return fmt.Errorf("%v: %s", err, strings.TrimSpace(string(out))) + } + return nil + } +} + +// updateIntegrationRef is the compare-and-swap the production command runs. +func updateIntegrationRef(ctx context.Context, checkout, ref, newCommit, oldCommit string) error { + _, err := runGit(ctx, checkout, nil, "update-ref", "refs/heads/"+ref, newCommit, oldCommit) + return err +} + +// batchIn is the batch input the tests share. +func (k *batchKit) batchIn() IntegrationBatchInput { + return IntegrationBatchInput{Plan: "g", Actor: "parent", Checkout: k.repo.path, IntegrationRef: "dev-int", BaseRef: "dev"} +} + +// branchTip reads a local branch, and whether it exists. +func (k *batchKit) branchTip(name string) (string, bool) { + tip, found, err := integrationBranchTip(context.Background(), k.repo.path, name) + if err != nil { + k.t.Fatal(err) + } + return tip, found +} + +// stageRows reads the batch's stage rows of the plan. +func (k *batchKit) stageRows() []store.IntegrationStageRow { + rows, err := store.IntegrationStagesOfPlan(context.Background(), k.sched.Store, "g") + if err != nil { + k.t.Fatal(err) + } + return rows +} + +// countStage counts the stage rows of one kind. +func countStage(rows []store.IntegrationStageRow, stage string) int { + n := 0 + for _, r := range rows { + if r.Stage == stage { + n++ + } + } + return n +} + +// TestVerifyRecordHelper is the verification stub's record writer. It runs only when the stub sets its environment, so +// the ordinary test run skips it. It seals the record of the worktree's merged commit, naming the base's tree instead when +// CRW_TEST_WRONG_TREE is set, the case where the verifier was given a tree other than the merged one. +func TestVerifyRecordHelper(t *testing.T) { + if os.Getenv("CRW_TEST_RECORD_HELPER") != "1" { + t.Skip("the verification stub's record writer") + } + ctx := context.Background() + cwd, err := os.Getwd() + if err != nil { + t.Fatal(err) + } + head, err := runGit(ctx, cwd, nil, "rev-parse", "HEAD") + if err != nil { + t.Fatal(err) + } + head = strings.TrimSpace(head) + base := os.Getenv("CRW_VERIFY_BASE") + tree := "" + if os.Getenv("CRW_TEST_WRONG_TREE") == "1" { + tree, err = runGit(ctx, cwd, nil, "rev-parse", base+"^{tree}") + } else { + tree, err = runGit(ctx, cwd, nil, "rev-parse", head+"^{tree}") + } + if err != nil { + t.Fatal(err) + } + keys, err := CommitVerificationKeys(ctx, cwd, head) + if err != nil { + t.Fatal(err) + } + raw, err := SealVerificationRecord(VerificationRecord{Runner: "local", Repository: "owner/repo", BaseCommit: base, HeadCommit: head, + TreeHash: strings.TrimSpace(tree), CiDigest: keys.CiDigest, Tools: map[string]string{"go": "go1.27.1"}, Pins: map[string]string{}, + PinMismatch: []string{}, Dependencies: keys.Dependencies, OS: runtime.GOOS, Arch: runtime.GOARCH, Result: "pass", Jobs: nil}) + if err != nil { + t.Fatal(err) + } + if err := os.WriteFile(os.Getenv("CRW_VERIFY_RECORD"), raw, 0o600); err != nil { + t.Fatal(err) + } +} + +// c2 (the evaluation's d11): the merged tree is verified where it is merged. Two clean candidates integrate, both are +// marked, and the branch is created by the batch (create-only, from nothing). +func TestIntegrationBatchIntegratesAndMarksEveryCandidate(t *testing.T) { + k := newBatchKit(t, batchNode{name: "a", files: map[string]string{"a.txt": "a\n"}}, batchNode{name: "b", files: map[string]string{"b.txt": "b\n"}}) + k.acceptByCommit("a") + k.acceptByCommit("b") + res, err := k.sched.IntegrateBatch(context.Background(), k.batchIn(), IntegrationBatchDeps{Verify: stubVerifier(writeStubVerifier(t)), Update: updateIntegrationRef}) + if err != nil { + t.Fatalf("batch: %v", err) + } + if len(res.Merged) != 2 || len(res.Split) != 0 { + t.Fatalf("merged %d split %d; want 2 and 0: %+v", len(res.Merged), len(res.Split), res) + } + tip, found := k.branchTip("dev-int") + if !found || tip != res.NewHead { + t.Fatalf("the integration branch is %s (found %v); want %s", tip, found, res.NewHead) + } + files := k.repo.git("ls-tree", "-r", "--name-only", tip) + if !strings.Contains(files, "a.txt") || !strings.Contains(files, "b.txt") { + t.Fatalf("the integration branch holds %q; want a.txt and b.txt", files) + } + if n := countStage(k.stageRows(), "marked"); n != 2 { + t.Fatalf("%d merged marks written; want 2", n) + } +} + +// c2: one failing candidate among several is isolated and the others integrate. The stub fails the merged tree that +// holds bad.txt, so the batch bisects to bad and leaves only it out. +func TestIntegrationBatchIsolatesTheFailingCandidate(t *testing.T) { + k := newBatchKit(t, batchNode{name: "a", files: map[string]string{"a.txt": "a\n"}}, + batchNode{name: "bad", files: map[string]string{"bad.txt": "bad\n"}}, + batchNode{name: "c", files: map[string]string{"c.txt": "c\n"}}) + for _, n := range []string{"a", "bad", "c"} { + k.acceptByCommit(n) + } + res, err := k.sched.IntegrateBatch(context.Background(), k.batchIn(), IntegrationBatchDeps{Verify: stubVerifier(writeStubVerifier(t)), Update: updateIntegrationRef}) + if err != nil { + t.Fatalf("batch: %v", err) + } + if len(res.Merged) != 2 || res.Merged[0].NodeID != "a" || res.Merged[1].NodeID != "c" { + t.Fatalf("merged %+v; want a and c", res.Merged) + } + if len(res.Split) != 1 || res.Split[0].NodeID != "bad" || res.Split[0].Reason != "verification_failed" { + t.Fatalf("split %+v; want bad with verification_failed", res.Split) + } + files := k.repo.git("ls-tree", "-r", "--name-only", res.NewHead) + if strings.Contains(files, "bad.txt") || !strings.Contains(files, "a.txt") || !strings.Contains(files, "c.txt") { + t.Fatalf("the branch holds %q; want a.txt and c.txt without bad.txt", files) + } +} + +// c2: a candidate that conflicted only with the candidate that was removed integrates on the retry. +func TestIntegrationBatchRetriesACandidateThatOnlyConflictedWithTheRemovedOne(t *testing.T) { + k := newBatchKit(t, batchNode{name: "a", files: map[string]string{"a.txt": "a\n", "bad.txt": "bad\n", "shared.txt": "a\n"}}, + batchNode{name: "b", files: map[string]string{"b.txt": "b\n", "shared.txt": "b\n"}}) + k.acceptByCommit("a") + k.acceptByCommit("b") + res, err := k.sched.IntegrateBatch(context.Background(), k.batchIn(), IntegrationBatchDeps{Verify: stubVerifier(writeStubVerifier(t)), Update: updateIntegrationRef}) + if err != nil { + t.Fatalf("batch: %v", err) + } + if len(res.Merged) != 1 || res.Merged[0].NodeID != "b" { + t.Fatalf("merged %+v; want b alone, integrated on the retry", res.Merged) + } + if len(res.Split) != 1 || res.Split[0].NodeID != "a" || res.Split[0].Reason != "verification_failed" { + t.Fatalf("split %+v; want a with verification_failed", res.Split) + } +} + +// c3: the expected-head guard. The branch moved between the batch's read and its update, so the update is refused, the +// branch keeps the value the other writer gave it, and no ref_moved stage is written. +func TestIntegrationBatchRefusesAHeadThatMovedUnderIt(t *testing.T) { + k := newBatchKit(t, batchNode{name: "a", files: map[string]string{"a.txt": "a\n"}}) + k.acceptByCommit("a") + other := k.repo.git("rev-parse", "dev") + k.repo.write("other.txt", "other\n") + k.repo.git("add", "other.txt") + k.repo.git("commit", "-q", "-m", "other writer") + moved := k.repo.git("rev-parse", "HEAD") + k.repo.git("checkout", "-q", "dev") + deps := IntegrationBatchDeps{Verify: stubVerifier(writeStubVerifier(t)), Update: func(ctx context.Context, checkout, ref, newCommit, oldCommit string) error { + if _, err := runGit(ctx, checkout, nil, "update-ref", "refs/heads/"+ref, moved, ""); err != nil { + t.Fatal(err) + } + return updateIntegrationRef(ctx, checkout, ref, newCommit, oldCommit) + }} + _, err := k.sched.IntegrateBatch(context.Background(), k.batchIn(), deps) + if refusalReasonOf(err) != "stale_mark_context" { + t.Fatalf("reason %q (err %v); want stale_mark_context", refusalReasonOf(err), err) + } + if tip, _ := k.branchTip("dev-int"); tip != moved { + t.Fatalf("the branch is %s; the other writer's %s was overwritten (base %s)", tip, moved, other) + } + if n := countStage(k.stageRows(), "ref_moved"); n != 0 { + t.Fatalf("%d ref_moved stages after a refused update", n) + } +} + +// d11: a verifier given a tree other than the merged one. The stub names the base's tree in its record, so no +// candidate passes, nothing is merged and the branch is not created. +func TestIntegrationBatchRefusesARecordOfAnotherTree(t *testing.T) { + k := newBatchKit(t, batchNode{name: "a", files: map[string]string{"a.txt": "a\n"}}) + k.acceptByCommit("a") + t.Setenv("CRW_TEST_WRONG_TREE", "1") + res, err := k.sched.IntegrateBatch(context.Background(), k.batchIn(), IntegrationBatchDeps{Verify: stubVerifier(writeStubVerifier(t)), Update: updateIntegrationRef}) + if err != nil { + t.Fatalf("batch: %v", err) + } + if len(res.Merged) != 0 || len(res.Split) != 1 || res.Split[0].Reason != "verification_failed" { + t.Fatalf("merged %+v split %+v; want nothing merged and a verification_failed split", res.Merged, res.Split) + } + if _, found := k.branchTip("dev-int"); found { + t.Fatal("the integration branch was created for a tree the verifier did not verify") + } +} + +// D-B: the integration is parent-only. A candidate of another relationship's parent is refused and nothing is written. +func TestIntegrationBatchRefusesANonParentActor(t *testing.T) { + k := newBatchKit(t, batchNode{name: "a", files: map[string]string{"a.txt": "a\n"}}) + k.acceptByCommit("a") + in := k.batchIn() + in.Actor = "intruder" + if _, err := k.sched.IntegrateBatch(context.Background(), in, IntegrationBatchDeps{Verify: stubVerifier(writeStubVerifier(t)), Update: updateIntegrationRef}); err == nil { + t.Fatal("a non-parent integrated a candidate") + } + if _, found := k.branchTip("dev-int"); found { + t.Fatal("the integration branch was created by a non-parent") + } + if n := len(k.stageRows()); n != 0 { + t.Fatalf("%d stage rows were written for a refused batch", n) + } +} + +// D-D: a candidate whose spec changed after its acceptance is stale, so the batch leaves it out of the candidates and +// merges the rest. The stale node is not a refusal of the batch: it is not ready. +func TestIntegrationBatchLeavesOutAStaleCandidate(t *testing.T) { + k := newBatchKit(t, batchNode{name: "a", files: map[string]string{"a.txt": "a\n"}}, batchNode{name: "b", files: map[string]string{"b.txt": "b\n"}}) + k.acceptByCommit("a") + k.acceptByCommit("b") + k.invRevise("g", "a", "g-r2", invTitle("changed after the acceptance")) + res, err := k.sched.IntegrateBatch(context.Background(), k.batchIn(), IntegrationBatchDeps{Verify: stubVerifier(writeStubVerifier(t)), Update: updateIntegrationRef}) + if err != nil { + t.Fatalf("batch: %v", err) + } + if len(res.Merged) != 1 || res.Merged[0].NodeID != "b" { + t.Fatalf("merged %+v; want b alone (a is stale)", res.Merged) + } +} + +// D-B: the batch goes through the coordinator-epoch fence dag-accept uses. A session that holds another epoch is refused +// before anything is read or written. +func TestIntegrationBatchRefusesAStaleEpoch(t *testing.T) { + k := newBatchKit(t, batchNode{name: "a", files: map[string]string{"a.txt": "a\n"}}) + k.acceptByCommit("a") + k.sched.ExpectedEpoch = 7 + _, err := k.sched.IntegrateBatch(context.Background(), k.batchIn(), IntegrationBatchDeps{Verify: stubVerifier(writeStubVerifier(t)), Update: updateIntegrationRef}) + if refusalReasonOf(err) != "stale_coordinator_epoch" { + t.Fatalf("reason %q (err %v); want stale_coordinator_epoch", refusalReasonOf(err), err) + } + if _, found := k.branchTip("dev-int"); found { + t.Fatal("the integration branch was created under a stale epoch") + } +} + +// P1-1: a mark that could not be written leaves its batch visible as ref_moved with a pending mark, and the next run +// completes it from the frozen row without an error. The candidate is made stale between the verification and the +// mark, so its mark is refused; restoring it lets the completion write the mark. +func TestIntegrationBatchCompletesAPendingMarkFromItsFrozenRow(t *testing.T) { + k := newBatchKit(t, batchNode{name: "a", files: map[string]string{"a.txt": "a\n"}}) + k.acceptByCommit("a") + deps := IntegrationBatchDeps{Verify: stubVerifier(writeStubVerifier(t)), Update: updateIntegrationRef, AfterMove: func(context.Context) error { + k.invRevise("g", "a", "g-r2", invTitle("changed before the mark")) + return nil + }} + res, err := k.sched.IntegrateBatch(context.Background(), k.batchIn(), deps) + if err != nil { + t.Fatalf("batch: %v", err) + } + if len(res.Pending) != 1 || res.Pending[0] != "a" { + t.Fatalf("pending %+v; want a", res.Pending) + } + if countStage(k.stageRows(), "mark_pending") != 1 { + t.Fatal("the pending mark was not recorded") + } + in := k.batchIn() + if err := k.sched.completePendingMarks(context.Background(), in); err != nil { + t.Fatalf("completion while the candidate is still stale: %v", err) + } + if countStage(k.stageRows(), "marked") != 0 { + t.Fatal("a stale candidate's mark was written") + } + k.invRevise("g", "a", "g-r3", func(n doc) {}) + if err := k.sched.completePendingMarks(context.Background(), in); err != nil { + t.Fatalf("completion: %v", err) + } + if countStage(k.stageRows(), "marked") != 1 { + t.Fatal("the pending mark was not completed from its frozen row") + } +} + +// P1-2: a batch that split every candidate moved nothing; running the same batch again is the same identity and must +// not collide with its own intent rows. The second run, with the verifier fixed, integrates the candidate. +func TestIntegrationBatchReRunsAfterAnUnmovedBatch(t *testing.T) { + k := newBatchKit(t, batchNode{name: "a", files: map[string]string{"a.txt": "a\n"}}) + k.acceptByCommit("a") + t.Setenv("CRW_TEST_WRONG_TREE", "1") + first, err := k.sched.IntegrateBatch(context.Background(), k.batchIn(), IntegrationBatchDeps{Verify: stubVerifier(writeStubVerifier(t)), Update: updateIntegrationRef}) + if err != nil || len(first.Merged) != 0 { + t.Fatalf("first run: %+v, %v; want nothing merged", first, err) + } + t.Setenv("CRW_TEST_WRONG_TREE", "") + second, err := k.sched.IntegrateBatch(context.Background(), k.batchIn(), IntegrationBatchDeps{Verify: stubVerifier(writeStubVerifier(t)), Update: updateIntegrationRef}) + if err != nil { + t.Fatalf("re-run of the same batch: %v", err) + } + if len(second.Merged) != 1 || second.Merged[0].NodeID != "a" { + t.Fatalf("re-run merged %+v; want a", second.Merged) + } +} diff --git a/internal/relay/dagsched/integration_bisect_test.go b/internal/relay/dagsched/integration_bisect_test.go new file mode 100644 index 0000000000..47374dfbc6 --- /dev/null +++ b/internal/relay/dagsched/integration_bisect_test.go @@ -0,0 +1,64 @@ +package dagsched + +import ( + "context" + "os" + "path/filepath" + "testing" +) + +// writePairVerifier is a verify command that exits 1 only when both x.txt and y.txt are present in the worktree it runs in, +// so a failure needs two candidates together. +func writePairVerifier(t *testing.T) string { + t.Helper() + stub := writeStubVerifier(t) + script := filepath.Join(t.TempDir(), "verify-pair.sh") + body := "#!/bin/sh\nif [ -e x.txt ] && [ -e y.txt ]; then\n echo 'x.txt and y.txt together fail' >&2\n exit 1\nfi\nexec " + stub + "\n" + if err := os.WriteFile(script, []byte(body), 0o700); err != nil { + t.Fatal(err) + } + return script +} + +// CRW-965 (parent decision D5): a candidate the bisection leaves out only because it breaks a prefix is reported as not +// proven failing, never as failed; it gets no mark and stays ready, and the next batch integrates it. +func TestIntegrationBatchReportsACandidateThatOnlyBreaksTheSetAsNotProvenFailing(t *testing.T) { + k := newBatchKit(t, + batchNode{name: "a", files: map[string]string{"x.txt": "x\n"}}, + batchNode{name: "b", files: map[string]string{"y.txt": "y\n"}}) + k.acceptByCommit("a") + k.acceptByCommit("b") + ctx := context.Background() + res, err := k.sched.IntegrateBatch(ctx, k.batchIn(), IntegrationBatchDeps{Verify: stubVerifier(writePairVerifier(t)), Update: updateIntegrationRef}) + if err != nil { + t.Fatalf("the batch: %v", err) + } + if len(res.Merged) != 1 || res.Merged[0].NodeID != "a" { + t.Fatalf("a alone is the verified set: %+v", res.Merged) + } + var reasons []string + for _, s := range res.Split { + if s.NodeID == "b" { + reasons = append(reasons, s.Reason) + } + } + if len(reasons) != 1 || reasons[0] != "not_proven_failing" { + t.Fatalf("b breaks the set but fails alone nowhere: its split reason must be not_proven_failing, got %v", reasons) + } + for _, m := range k.stageRows() { + if m.Stage == "marked" && m.NodeID == "b" { + t.Fatal("a candidate left out by the bisection must get no mark") + } + } + if _, ok := k.branchTip("dev-int"); !ok { + t.Fatal("the branch should hold the verified set") + } + // the next batch verifies the set without the pair rule (the rule has since been dropped), so b integrates + next, err := k.sched.IntegrateBatch(ctx, k.batchIn(), IntegrationBatchDeps{Verify: stubVerifier(writeStubVerifier(t)), Update: updateIntegrationRef}) + if err != nil { + t.Fatalf("the next batch: %v", err) + } + if len(next.Merged) != 1 || next.Merged[0].NodeID != "b" { + t.Fatalf("the next batch integrates b, which stayed ready: %+v", next.Merged) + } +} diff --git a/internal/relay/dagsched/integration_boundary_test.go b/internal/relay/dagsched/integration_boundary_test.go new file mode 100644 index 0000000000..eff236073e --- /dev/null +++ b/internal/relay/dagsched/integration_boundary_test.go @@ -0,0 +1,69 @@ +package dagsched + +import ( + "context" + "runtime" + "testing" +) + +// CRW-965 (parent decisions D1 and D2): the record judge compares the tool values with the record's own pins and the +// platform, and leaves goFlags and goEnv to the writer. These tests pin both sides of that boundary. +func TestVerificationJudgeLeavesGoFlagsAndGoEnvToTheWriter(t *testing.T) { + k := newBatchKit(t, batchNode{name: "a", files: map[string]string{"a.txt": "a\n"}}) + head := k.heads["a"] + keys, err := CommitVerificationKeys(context.Background(), k.repo.path, head) + if err != nil { + t.Fatal(err) + } + want := VerificationKeys{Tree: k.treeOf(head), Base: k.base, CiDigest: keys.CiDigest, Dependencies: keys.Dependencies, OS: runtime.GOOS, Arch: runtime.GOARCH} + seal := func(tools, pins map[string]string) []byte { + raw, err := SealVerificationRecord(VerificationRecord{Runner: "local", Repository: "owner/repo", BaseCommit: k.base, HeadCommit: head, + TreeHash: want.Tree, CiDigest: keys.CiDigest, Tools: tools, Pins: pins, GoFlags: "-p=99", GoEnv: "GOFOO=bar", + PinMismatch: []string{}, Dependencies: keys.Dependencies, OS: runtime.GOOS, Arch: runtime.GOARCH, Result: "pass", Jobs: nil}) + if err != nil { + t.Fatal(err) + } + return raw + } + // goFlags, goEnv and an unpinned tool value differ from anything this host runs: the record is judged by the keys, and reusable + if _, err := JudgeVerificationRecord(seal(map[string]string{"go": "go0.0.1"}, map[string]string{}), want); err != nil { + t.Fatalf("goFlags, goEnv and an unpinned tool are not judged by value: %v", err) + } + // a tool value that contradicts its own pin is refused + _, err = JudgeVerificationRecord(seal(map[string]string{"go": "go0.0.1"}, map[string]string{"go": "go1.27.1"}), want) + if refusalReasonOf(err) != "disposition_conflict" { + t.Fatal("a tool that differs from its own pin must refuse the record as disposition_conflict") + } +} + +// CRW-965 (parent decision D2): a stored verification stands for the keys the verifier would judge now only when every key +// matches, the platform included. A missing key or another platform verifies again. +func TestVerifiedKeysHoldOnlyForTheSamePlatform(t *testing.T) { + now := map[string]string{"tree": "t", "ci_digest": "c", "dependency_go_sum": "g", "dependency_web_lock": "", "os": "linux", "arch": "amd64"} + same := map[string]string{} + for key, value := range now { + same[key] = value + } + if !verifiedKeysHold(same, now) { + t.Fatal("the same keys and platform reuse the stored verification") + } + for _, key := range []string{"os", "arch", "ci_digest", "tree"} { + other := map[string]string{} + for k, v := range same { + other[k] = v + } + other[key] = "other" + if verifiedKeysHold(other, now) { + t.Fatalf("a stored verification with another %s must verify again", key) + } + } + missing := map[string]string{} + for k, v := range same { + if k != "arch" { + missing[k] = v + } + } + if verifiedKeysHold(missing, now) { + t.Fatal("a row without the arch key is an older row and verifies again") + } +} diff --git a/internal/relay/dagsched/integration_contained_refs_test.go b/internal/relay/dagsched/integration_contained_refs_test.go new file mode 100644 index 0000000000..d64a1d926a --- /dev/null +++ b/internal/relay/dagsched/integration_contained_refs_test.go @@ -0,0 +1,87 @@ +package dagsched + +import ( + "context" + "errors" + "testing" + + "github.com/thisisjun786/codex-relay-workflow/internal/relay/dag" +) + +// CRW-965 (revision of parent decision D4): a commit-accepted node whose head a later batch found already contained moves +// no ref in that batch, but it still completes on the integration branch that contains it. Y is contained in X's history; +// the named batch merges X, the next run marks Y with no merge and no verification, and observing Y on the branch then +// satisfies Y's integrated edge to Z. +func TestContainedNodeCompletesOnTheBranchThatContainsIt(t *testing.T) { + k := newBatchKit(t, batchNode{name: "y", files: map[string]string{"y.txt": "y\n"}}) + k.addChildNode("x", k.heads["y"], "x.txt") + k.putPlan("g", int(k.snapshot("g").Revision), "g-z", addRelNode("z", dag.NodeImplementation), addEdge("yz", "y", "z", dag.EdgeIntegrated, doc{"target_repository": k.repo.path})) + k.acceptByCommit("y") + k.acceptByCommit("x") + ctx := context.Background() + k.sched.Tips = newLegacyLocalTipReader(t) + k.sched.Ancestry = GitAncestry{}.Ancestry + runs := 0 + base := stubVerifier(writeStubVerifier(t)) + counting := IntegrationBatchDeps{Verify: func(ctx context.Context, dir string, env []string) error { + runs++ + return base(ctx, dir, env) + }, Update: updateIntegrationRef} + first := k.batchIn() + first.Nodes = []string{"x"} + if _, err := k.sched.IntegrateBatch(ctx, first, counting); err != nil { + t.Fatalf("named batch: %v", err) + } + res, err := k.sched.IntegrateBatch(ctx, k.batchIn(), counting) + if err != nil { + t.Fatalf("next batch: %v", err) + } + if runs != 1 || len(res.Merged) != 0 { + t.Fatalf("verifier ran %d times and merged %d; the contained node needs neither", runs, len(res.Merged)) + } + acc, found, err := loadActiveAcceptance(ctx, k.sched.Store.Q(ctx), "g", "y") + if err != nil || !found { + t.Fatalf("the active acceptance of y: %v, %v", found, err) + } + refs, err := integrationRefsOf(ctx, k.sched.Store.Q(ctx), acc.AcceptanceID) + if err != nil || len(refs) != 1 || refs[0] != "dev-int" { + t.Fatalf("the completion refs of a contained node: %v (err %v); want dev-int", refs, err) + } + obs, err := k.sched.ObserveIntegration(ctx, "g", "y", "parent", []Target{{Repository: k.repo.path, BaseRef: "dev-int"}}) + if err != nil || !obs.Integrated { + t.Fatalf("observe y on the integration branch: %+v, %v; want integrated", obs, err) + } + snap := k.snapshot("g") + for _, e := range snap.Edges { + if e.EdgeID == "yz" { + st, err := k.sched.edgeStatus(ctx, k.sched.Store.Q(ctx), "g", snap, e) + if err != nil || !st.Satisfied { + t.Fatalf("the integrated edge y to z: %+v (err %v); want satisfied once y is observed", st, err) + } + return + } + } + t.Fatal("the plan has no edge yz") +} + +// CRW-965 (revision of parent decision D4): a node whose only batch failed before its move names no ref, so its edge +// waits; the failed attempt leaves nothing that a completion could read. +func TestNodeWhoseOnlyBatchFailedBeforeItsMoveNamesNoRef(t *testing.T) { + k := newBatchKit(t, batchNode{name: "p", files: map[string]string{"p.txt": "p\n"}}) + k.acceptByCommit("p") + ctx := context.Background() + crash := IntegrationBatchDeps{Verify: stubVerifier(writeStubVerifier(t)), Update: func(context.Context, string, string, string, string) error { + return errors.New("simulated refusal before the branch moved") + }} + if _, err := k.sched.IntegrateBatch(ctx, k.batchIn(), crash); err == nil { + t.Fatal("the failing batch should not complete") + } + acc, found, err := loadActiveAcceptance(ctx, k.sched.Store.Q(ctx), "g", "p") + if err != nil || !found { + t.Fatalf("the active acceptance of p: %v, %v", found, err) + } + refs, err := integrationRefsOf(ctx, k.sched.Store.Q(ctx), acc.AcceptanceID) + if err != nil || len(refs) != 0 { + t.Fatalf("the completion refs of a node whose batch never moved: %v (err %v); want none", refs, err) + } +} diff --git a/internal/relay/dagsched/integration_contained_test.go b/internal/relay/dagsched/integration_contained_test.go new file mode 100644 index 0000000000..08f9802a6e --- /dev/null +++ b/internal/relay/dagsched/integration_contained_test.go @@ -0,0 +1,164 @@ +package dagsched + +import ( + "context" + "errors" + "testing" + + "github.com/thisisjun786/codex-relay-workflow/internal/relay/dag" +) + +// addChildNode adds an implementation node whose commit is a child of parent, reports it and records its verified head, +// as newBatchKit does for its own nodes. +func (k *batchKit) addChildNode(name, parent, file string) { + k.t.Helper() + k.repo.git("checkout", "-q", "-B", "feature-"+name, parent) + k.repo.write(file, name+"\n") + k.repo.git("add", file) + k.repo.git("commit", "-q", "-m", "node "+name) + k.heads[name] = k.repo.git("rev-parse", "HEAD") + k.repo.git("checkout", "-q", "dev") + snap := k.snapshot("g") + k.putPlan("g", int(snap.Revision), "g-"+name, addRelNode(name, dag.NodeImplementation)) + r := k.reportNode("g", name, acceptOpts{}) + k.exec("INSERT INTO dag_verified_heads (event_id, relationship_id, execution_generation, verdict_turn_id, head_sha, recorded_by_task_id, recorded_at) VALUES (?,?,?,?,?,?,?)", + r.Event, r.Acceptance.RelationshipID, 1, "verdict-turn", k.heads[name], "parent", k.clock()) +} + +// CRW-965 (parent decision, case 1): a batch that moved the branch and died before it recorded the move leaves the +// candidate contained in a head the relay verified. The next batch marks it from its frozen row, with no merge and no +// verifier call. +func TestContainedCandidateIsMarkedAfterABatchDiedAfterMovingTheBranch(t *testing.T) { + k := newBatchKit(t, batchNode{name: "a", files: map[string]string{"a.txt": "a\n"}}) + k.acceptByCommit("a") + ctx := context.Background() + dying := IntegrationBatchDeps{Verify: stubVerifier(writeStubVerifier(t)), Update: func(ctx context.Context, checkout, ref, newCommit, oldCommit string) error { + if err := updateIntegrationRef(ctx, checkout, ref, newCommit, oldCommit); err != nil { + return err + } + return errors.New("simulated death after the branch moved") + }} + if _, err := k.sched.IntegrateBatch(ctx, k.batchIn(), dying); err == nil { + t.Fatal("the dying batch should fail after it moved the branch") + } + moved, found := k.branchTip("dev-int") + if !found { + t.Fatal("the dying batch should have moved the branch") + } + runs := 0 + base := stubVerifier(writeStubVerifier(t)) + counting := IntegrationBatchDeps{Verify: func(ctx context.Context, dir string, env []string) error { + runs++ + return base(ctx, dir, env) + }, Update: updateIntegrationRef} + res, err := k.sched.IntegrateBatch(ctx, k.batchIn(), counting) + if err != nil { + t.Fatalf("next batch: %v", err) + } + if runs != 0 || len(res.Merged) != 0 { + t.Fatalf("verifier ran %d times and merged %d; a contained candidate needs neither", runs, len(res.Merged)) + } + if tip, _ := k.branchTip("dev-int"); tip != moved { + t.Fatalf("the branch moved from %s to %s; no new merge commit is expected", moved, tip) + } + if len(res.AlreadyContained) != 1 || res.AlreadyContained[0].NodeID != "a" || res.AlreadyContained[0].MarkedEvent == "" { + t.Fatalf("want a marked already_contained candidate a: %+v", res.AlreadyContained) + } +} + +// CRW-965 (parent decision, cases 2 and 4): a named batch merges X, whose history contains Y's head. The next run marks Y +// as already contained, and the stage rows agree with the events the run reports. +func TestContainedCandidateOfANamedBatchIsMarkedOnTheNextRun(t *testing.T) { + k := newBatchKit(t, batchNode{name: "y", files: map[string]string{"y.txt": "y\n"}}) + k.addChildNode("x", k.heads["y"], "x.txt") + k.acceptByCommit("y") + k.acceptByCommit("x") + ctx := context.Background() + runs := 0 + base := stubVerifier(writeStubVerifier(t)) + counting := IntegrationBatchDeps{Verify: func(ctx context.Context, dir string, env []string) error { + runs++ + return base(ctx, dir, env) + }, Update: updateIntegrationRef} + first := k.batchIn() + first.Nodes = []string{"x"} + if _, err := k.sched.IntegrateBatch(ctx, first, counting); err != nil { + t.Fatalf("named batch: %v", err) + } + res, err := k.sched.IntegrateBatch(ctx, k.batchIn(), counting) + if err != nil { + t.Fatalf("next batch: %v", err) + } + if runs != 1 { + t.Fatalf("verifier ran %d times; only the named batch may verify", runs) + } + var y *IntegrationBatchContained + for i := range res.AlreadyContained { + if res.AlreadyContained[i].NodeID == "y" { + y = &res.AlreadyContained[i] + } + } + if y == nil || y.MarkedEvent == "" { + t.Fatalf("Y should be marked as already contained: %+v", res.AlreadyContained) + } + marked := 0 + for _, r := range k.stageRows() { + if r.BatchID == res.BatchID && r.Stage == "marked" { + marked++ + } + } + reported := 0 + for _, c := range res.AlreadyContained { + if c.MarkedEvent != "" { + reported++ + } + } + if marked != reported { + t.Fatalf("%d marked stage rows for the batch, %d marks reported", marked, reported) + } +} + +// CRW-965 (parent decision, case 3): a branch head no recorded batch verified does not launder a candidate. Y is reported +// as contained_unverified, is not marked, and stays a ready candidate. +func TestContainedCandidateWaitsWhenTheBranchHeadWasNeverVerified(t *testing.T) { + k := newBatchKit(t, batchNode{name: "y", files: map[string]string{"y.txt": "y\n"}}) + k.addChildNode("x", k.heads["y"], "x.txt") + k.acceptByCommit("y") + k.acceptByCommit("x") + ctx := context.Background() + k.repo.git("update-ref", "refs/heads/dev-int", k.heads["x"]) + runs := 0 + counting := IntegrationBatchDeps{Verify: func(ctx context.Context, dir string, env []string) error { + runs++ + return stubVerifier(writeStubVerifier(t))(ctx, dir, env) + }, Update: updateIntegrationRef} + res, err := k.sched.IntegrateBatch(ctx, k.batchIn(), counting) + if err != nil { + t.Fatalf("batch: %v", err) + } + if runs != 0 || len(res.AlreadyContained) != 0 { + t.Fatalf("an unverified branch head must not be marked or verified: runs %d, already contained %+v", runs, res.AlreadyContained) + } + found := false + for _, c := range res.ContainedUnverified { + if c.NodeID == "y" { + found = true + } + } + if !found { + t.Fatalf("Y should be reported contained_unverified: %+v", res.ContainedUnverified) + } + cands, err := k.sched.AcceptedCandidates(ctx, "g") + if err != nil { + t.Fatal(err) + } + ready := false + for _, c := range cands { + if c.NodeID == "y" { + ready = true + } + } + if !ready { + t.Fatal("Y should stay a ready candidate") + } +} diff --git a/internal/relay/dagsched/integration_currency_test.go b/internal/relay/dagsched/integration_currency_test.go new file mode 100644 index 0000000000..fbd861ba78 --- /dev/null +++ b/internal/relay/dagsched/integration_currency_test.go @@ -0,0 +1,70 @@ +package dagsched + +import ( + "context" + "testing" +) + +// CRW-965 (parent decision D3): a candidate that stops being current between its verification and the swap refuses the +// batch under merge_candidate_moved. The branch does not move and nothing is marked; the next batch leaves that candidate +// out and verifies the rest again. +func TestIntegrationBatchRefusesACandidateThatChangedAfterVerification(t *testing.T) { + changes := map[string]func(k *batchKit){ + "cancelled": func(k *batchKit) { + k.exec("UPDATE relationships SET status = 'cancelled' WHERE relationship_id = (SELECT relationship_id FROM dag_node_executions WHERE plan_id = 'g' AND node_id = 'a')") + }, + "superseded": func(k *batchKit) { + k.exec("UPDATE dag_acceptances SET state = 'superseded' WHERE plan_id = 'g' AND node_id = 'a' AND state = 'active'") + }, + "closed": func(k *batchKit) { + k.exec("UPDATE relationships SET status = 'closed' WHERE relationship_id = (SELECT relationship_id FROM dag_node_executions WHERE plan_id = 'g' AND node_id = 'a')") + }, + } + for name, change := range changes { + t.Run(name, func(t *testing.T) { + k := newBatchKit(t, + batchNode{name: "a", files: map[string]string{"a.txt": "a\n"}}, + batchNode{name: "b", files: map[string]string{"b.txt": "b\n"}}) + k.acceptByCommit("a") + k.acceptByCommit("b") + ctx := context.Background() + base := stubVerifier(writeStubVerifier(t)) + changed := false + deps := IntegrationBatchDeps{Verify: func(ctx context.Context, dir string, env []string) error { + if err := base(ctx, dir, env); err != nil { + return err + } + if !changed { + changed = true + change(k) + } + return nil + }, Update: updateIntegrationRef} + _, err := k.sched.IntegrateBatch(ctx, k.batchIn(), deps) + if refusalReasonOf(err) != "merge_candidate_moved" { + t.Fatalf("a candidate that changed after the verification must refuse the batch as merge_candidate_moved: %v", err) + } + if _, found := k.branchTip("dev-int"); found { + t.Fatal("the branch moved although a candidate was no longer current") + } + if countStage(k.stageRows(), "marked") != 0 { + t.Fatal("a refused batch must not mark any candidate") + } + verifies := 0 + counting := IntegrationBatchDeps{Verify: func(ctx context.Context, dir string, env []string) error { + verifies++ + return base(ctx, dir, env) + }, Update: updateIntegrationRef} + res, err := k.sched.IntegrateBatch(ctx, k.batchIn(), counting) + if err != nil { + t.Fatalf("the next batch: %v", err) + } + if len(res.Merged) != 1 || res.Merged[0].NodeID != "b" { + t.Fatalf("the next batch should integrate only b: %+v", res.Merged) + } + if verifies != 1 { + t.Fatalf("the next batch verifies the surviving tree once; it ran %d times", verifies) + } + }) + } +} diff --git a/internal/relay/dagsched/integration_dupnode_test.go b/internal/relay/dagsched/integration_dupnode_test.go new file mode 100644 index 0000000000..dc70ad29dd --- /dev/null +++ b/internal/relay/dagsched/integration_dupnode_test.go @@ -0,0 +1,25 @@ +package dagsched + +import ( + "context" + "testing" +) + +// CRW-965 review: a node named twice (--node a --node a) is one candidate. Selecting it twice would merge its head twice +// in one batch and write its merged mark twice. +func TestIntegrationBatchCountsADuplicateNodeOnce(t *testing.T) { + k := newBatchKit(t, batchNode{name: "a", files: map[string]string{"a.txt": "a\n"}}) + k.acceptByCommit("a") + in := k.batchIn() + in.Nodes = []string{"a", "a"} + res, err := k.sched.IntegrateBatch(context.Background(), in, IntegrationBatchDeps{Verify: stubVerifier(writeStubVerifier(t)), Update: updateIntegrationRef}) + if err != nil { + t.Fatalf("batch: %v", err) + } + if len(res.Merged) != 1 { + t.Fatalf("merged %d candidates for a node named twice; want 1: %+v", len(res.Merged), res) + } + if n := countStage(k.stageRows(), "marked"); n != 1 { + t.Fatalf("%d merged marks written; want 1", n) + } +} diff --git a/internal/relay/dagsched/integration_fence_test.go b/internal/relay/dagsched/integration_fence_test.go new file mode 100644 index 0000000000..e4dd187b0f --- /dev/null +++ b/internal/relay/dagsched/integration_fence_test.go @@ -0,0 +1,69 @@ +package dagsched + +import ( + "context" + "errors" + "testing" +) + +// CRW-965 (parent decision, D6): an epoch claimed by another coordinator after the verification and before the swap refuses +// the batch, and the branch does not move. +func TestIntegrationBatchRefusesAnEpochClaimedBeforeTheSwap(t *testing.T) { + k := newBatchKit(t, batchNode{name: "a", files: map[string]string{"a.txt": "a\n"}}) + k.acceptByCommit("a") + base := stubVerifier(writeStubVerifier(t)) + deps := IntegrationBatchDeps{Verify: func(ctx context.Context, dir string, env []string) error { + if err := base(ctx, dir, env); err != nil { + return err + } + k.session("g", "parent", "nonce-replacement") + return nil + }, Update: updateIntegrationRef} + _, err := k.sched.IntegrateBatch(context.Background(), k.batchIn(), deps) + if !isStale(err) { + t.Fatalf("a batch whose epoch was claimed before the swap must be refused as stale: %v", err) + } + if _, found := k.branchTip("dev-int"); found { + t.Fatal("the branch moved although the batch's epoch was no longer current") + } +} + +// CRW-965 (parent decision, D6): a crash between the swap and the commit leaves the planned intent. The next run records the +// move once, without a second move, and marks the candidate the branch already holds. +func TestIntegrationBatchReconcilesAPlannedMoveAfterACrash(t *testing.T) { + k := newBatchKit(t, batchNode{name: "a", files: map[string]string{"a.txt": "a\n"}}) + k.acceptByCommit("a") + ctx := context.Background() + crashing := IntegrationBatchDeps{Verify: stubVerifier(writeStubVerifier(t)), Update: func(ctx context.Context, checkout, ref, newCommit, oldCommit string) error { + if err := updateIntegrationRef(ctx, checkout, ref, newCommit, oldCommit); err != nil { + return err + } + return errors.New("simulated crash between the swap and the commit") + }} + if _, err := k.sched.IntegrateBatch(ctx, k.batchIn(), crashing); err == nil { + t.Fatal("the crashing batch should fail after the swap") + } + moved, found := k.branchTip("dev-int") + if !found { + t.Fatal("the swap should have moved the branch") + } + runs := 0 + base := stubVerifier(writeStubVerifier(t)) + counting := IntegrationBatchDeps{Verify: func(ctx context.Context, dir string, env []string) error { + runs++ + return base(ctx, dir, env) + }, Update: updateIntegrationRef} + res, err := k.sched.IntegrateBatch(ctx, k.batchIn(), counting) + if err != nil { + t.Fatalf("next run: %v", err) + } + if len(res.Reconciled) != 1 { + t.Fatalf("the next run should reconcile the planned move once: %+v", res.Reconciled) + } + if tip, _ := k.branchTip("dev-int"); tip != moved || runs != 0 { + t.Fatalf("the branch moved to %s (was %s) and the verifier ran %d times; reconciliation makes no second move", tip, moved, runs) + } + if len(res.AlreadyContained) != 1 || res.AlreadyContained[0].MarkedEvent == "" { + t.Fatalf("the reconciled candidate should be marked: %+v", res.AlreadyContained) + } +} diff --git a/internal/relay/dagsched/integration_findings_test.go b/internal/relay/dagsched/integration_findings_test.go new file mode 100644 index 0000000000..5f7ccdff68 --- /dev/null +++ b/internal/relay/dagsched/integration_findings_test.go @@ -0,0 +1,173 @@ +package dagsched + +import ( + "context" + "errors" + "testing" +) + +// revalidateCriteria re-rules a node's acceptance under a new criteria set, the way the scheduler leaves a revalidation: the +// canonical criteria of its relationship and one revalidation row that names the new digest (CRW-965, parent decision d2). +func (k *batchKit) revalidateCriteria(t *testing.T, node, digest string) { + t.Helper() + ctx := context.Background() + acc, found, err := loadActiveAcceptance(ctx, k.sched.Store.Q(ctx), "g", node) + if err != nil || !found { + t.Fatalf("the active acceptance of %s: %v, %v", node, found, err) + } + rel, found, err := currentRelationshipOf(ctx, k.sched.Store.Q(ctx), "g", node) + if err != nil || !found { + t.Fatalf("the relationship of %s: %v, %v", node, found, err) + } + var event string + if err := k.s.DB.QueryRow("SELECT event_id FROM events WHERE relationship_id = ?", rel.ID).Scan(&event); err != nil { + t.Fatal(err) + } + var seq int + if err := k.s.DB.QueryRow("SELECT COUNT(*) FROM dag_acceptance_revalidations WHERE acceptance_id = ?", acc.AcceptanceID).Scan(&seq); err != nil { + t.Fatal(err) + } + k.exec("UPDATE canonical_criteria SET set_digest = ? WHERE relationship_id = ?", digest, rel.ID) + k.exec("INSERT INTO dag_acceptance_revalidations (revalidation_id, acceptance_id, criteria_set_digest, event_id, verdict_turn_id, reval_seq, revalidated_by, revalidated_at) VALUES (?, ?, ?, ?, 'vt', ?, 'parent', 't')", + "rv-"+acc.AcceptanceID+"-"+digest, acc.AcceptanceID, digest, event, seq+1) +} + +// CRW-965 (criteria c2 as read by the parent): a contained candidate whose criteria set changed is verified again with no +// merge, and only an unchanged criteria set is covered by the stored verification. +func TestContainedCandidateIsVerifiedAgainWhenItsCriteriaChanged(t *testing.T) { + k := newBatchKit(t, batchNode{name: "a", files: map[string]string{"a.txt": "a\n"}}) + k.acceptByCommit("a") + ctx := context.Background() + runs := 0 + base := stubVerifier(writeStubVerifier(t)) + deps := IntegrationBatchDeps{Verify: func(ctx context.Context, dir string, env []string) error { + runs++ + return base(ctx, dir, env) + }, Update: updateIntegrationRef} + in := k.batchIn() + in.Nodes = []string{"a"} + if _, err := k.sched.IntegrateBatch(ctx, in, deps); err != nil { + t.Fatalf("batch a: %v", err) + } + if runs != 1 { + t.Fatalf("the first batch verifies once; it ran %d times", runs) + } + k.invRevise("g", "a", "g-r2", func(n doc) { n["criteria_set_digest"] = dig("changed a") }) + k.revalidateCriteria(t, "a", dig("changed a")) + if _, err := k.sched.IntegrateBatch(ctx, in, deps); err != nil { + t.Fatalf("batch a after its criteria change: %v", err) + } + if runs != 2 { + t.Fatalf("a contained candidate whose criteria changed must be verified again: the verifier ran %d times", runs) + } + if _, err := k.sched.IntegrateBatch(ctx, in, deps); err != nil { + t.Fatalf("batch a again: %v", err) + } + if runs != 2 { + t.Fatalf("a contained candidate whose criteria set is verified needs no verification: the verifier ran %d times", runs) + } +} + +// CRW-965 (parent decision D3, the criteria part): a candidate revalidated while its merged tree is being verified is not +// published on the old verification. The swap is refused, and the branch does not move. +func TestIntegrationBatchRefusesACandidateRevalidatedDuringVerification(t *testing.T) { + k := newBatchKit(t, batchNode{name: "a", files: map[string]string{"a.txt": "a\n"}}) + k.acceptByCommit("a") + base := stubVerifier(writeStubVerifier(t)) + changed := false + deps := IntegrationBatchDeps{Verify: func(ctx context.Context, dir string, env []string) error { + if err := base(ctx, dir, env); err != nil { + return err + } + if !changed { + changed = true + k.invRevise("g", "a", "g-r2", func(n doc) { n["criteria_set_digest"] = dig("changed a") }) + k.revalidateCriteria(t, "a", dig("changed a")) + } + return nil + }, Update: updateIntegrationRef} + _, err := k.sched.IntegrateBatch(context.Background(), k.batchIn(), deps) + if refusalReasonOf(err) != "merge_candidate_moved" { + t.Fatalf("a candidate revalidated during the verification must refuse the swap: %v", err) + } + if _, found := k.branchTip("dev-int"); found { + t.Fatal("the branch moved on the old verification of a revalidated candidate") + } +} + +// CRW-965 (parent decision D6, the recovery): a move the reconciliation recorded after a crash is pushable, because its +// planned intent carries the verified head. +func TestPushAfterAReconciledMoveIsAllowed(t *testing.T) { + k := newBatchKit(t, batchNode{name: "a", files: map[string]string{"a.txt": "a\n"}}) + remote := bareRemote(t) + k.pushRepo(t, remote) + k.acceptByCommit("a") + ctx := context.Background() + crashing := IntegrationBatchDeps{Verify: stubVerifier(writeStubVerifier(t)), Update: func(ctx context.Context, checkout, ref, newCommit, oldCommit string) error { + if err := updateIntegrationRef(ctx, checkout, ref, newCommit, oldCommit); err != nil { + return err + } + return errors.New("simulated crash between the swap and the commit") + }} + if _, err := k.sched.IntegrateBatch(ctx, k.batchIn(), crashing); err == nil { + t.Fatal("the crashing batch should fail after the swap") + } + moved, found := k.branchTip("dev-int") + if !found { + t.Fatal("the swap should have moved the branch") + } + res, err := k.sched.IntegrateBatch(ctx, k.batchIn(), IntegrationBatchDeps{Verify: stubVerifier(writeStubVerifier(t)), Update: updateIntegrationRef}) + if err != nil || len(res.Reconciled) != 1 { + t.Fatalf("the next run reconciles the move: %+v, %v", res, err) + } + push, err := PushIntegration(ctx, k.repo.path, "origin", "dev", "dev-int", k.sched.VerifiedMoveOnto) + if err != nil || push.Outcome != PushPushed { + t.Fatalf("the reconciled head is a verified move and must push: %+v, %v", push, err) + } + if got := remoteBranch(t, remote, "dev"); got != moved { + t.Fatalf("remote dev is %s; want the reconciled head %s", got, moved) + } +} + +// CRW-965 (parent decision D4, the attempt that moved nothing): an attempt that failed before its move on one ref adds no +// completion target, and a later candidate that lands on another ref completes only there. +func TestIntegrationTargetsIgnoreAnAttemptThatMovedNothing(t *testing.T) { + k := newBatchKit(t, + batchNode{name: "a", files: map[string]string{"a.txt": "a\n"}}, + batchNode{name: "b", files: map[string]string{"b.txt": "b\n"}}) + k.acceptByCommit("a") + k.acceptByCommit("b") + ctx := context.Background() + refused := errors.New("simulated refusal before the branch moved") + failing := k.batchIn() + failing.IntegrationRef = "dev-r" + failing.Nodes = []string{"a"} + if _, err := k.sched.IntegrateBatch(ctx, failing, IntegrationBatchDeps{Verify: stubVerifier(writeStubVerifier(t)), Update: func(context.Context, string, string, string, string) error { + return refused + }}); err == nil { + t.Fatal("the failing attempt should not complete") + } + other := k.batchIn() + other.IntegrationRef = "dev-r" + other.Nodes = []string{"b"} + if _, err := k.sched.IntegrateBatch(ctx, other, IntegrationBatchDeps{Verify: stubVerifier(writeStubVerifier(t)), Update: updateIntegrationRef}); err != nil { + t.Fatalf("b on dev-r: %v", err) + } + landing := k.batchIn() + landing.IntegrationRef = "dev-s" + landing.Nodes = []string{"a"} + if _, err := k.sched.IntegrateBatch(ctx, landing, IntegrationBatchDeps{Verify: stubVerifier(writeStubVerifier(t)), Update: updateIntegrationRef}); err != nil { + t.Fatalf("a on dev-s: %v", err) + } + acc, found, err := loadActiveAcceptance(ctx, k.sched.Store.Q(ctx), "g", "a") + if err != nil || !found { + t.Fatalf("the active acceptance of a: %v, %v", found, err) + } + refs, err := integrationRefsOf(ctx, k.sched.Store.Q(ctx), acc.AcceptanceID) + if err != nil { + t.Fatal(err) + } + if len(refs) != 1 || refs[0] != "dev-s" { + t.Fatalf("a completes on dev-s only; the attempt on dev-r moved nothing for it: %v", refs) + } +} diff --git a/internal/relay/dagsched/integration_nits_test.go b/internal/relay/dagsched/integration_nits_test.go new file mode 100644 index 0000000000..db75590173 --- /dev/null +++ b/internal/relay/dagsched/integration_nits_test.go @@ -0,0 +1,55 @@ +package dagsched + +import ( + "context" + "testing" +) + +// CRW-965 (review): a retried all-contained batch with the same identity reports the marks it already recorded and inserts +// no stage row twice. +func TestRetriedContainedBatchDoesNotInsertItsMarkTwice(t *testing.T) { + k := newBatchKit(t, batchNode{name: "a", files: map[string]string{"a.txt": "a\n"}}) + k.acceptByCommit("a") + ctx := context.Background() + deps := IntegrationBatchDeps{Verify: stubVerifier(writeStubVerifier(t)), Update: updateIntegrationRef} + first := k.batchIn() + first.Nodes = []string{"a"} + if _, err := k.sched.IntegrateBatch(ctx, first, deps); err != nil { + t.Fatalf("first batch: %v", err) + } + res, err := k.sched.IntegrateBatch(ctx, k.batchIn(), deps) + if err != nil { + t.Fatalf("contained batch: %v", err) + } + again, err := k.sched.IntegrateBatch(ctx, k.batchIn(), deps) + if err != nil { + t.Fatalf("retried contained batch: %v", err) + } + if again.BatchID != res.BatchID { + t.Fatalf("the retry has another identity: %s and %s", again.BatchID, res.BatchID) + } + marked := 0 + for _, r := range k.stageRows() { + if r.BatchID == res.BatchID && r.Stage == "marked" { + marked++ + } + } + if marked > 1 { + t.Fatalf("%d marked stage rows for one batch; the retry must not insert its mark again", marked) + } +} + +// CRW-965 (review): a batch for a checkout does not take a candidate accepted for another repository. +func TestBatchDoesNotTakeACandidateAcceptedForAnotherCheckout(t *testing.T) { + k := newBatchKit(t, batchNode{name: "a", files: map[string]string{"a.txt": "a\n"}}) + k.acceptByCommit("a") + other := newGitRepo(t) + in := k.batchIn() + in.Checkout = other.path + if _, err := k.sched.IntegrateBatch(context.Background(), in, IntegrationBatchDeps{Verify: stubVerifier(writeStubVerifier(t)), Update: updateIntegrationRef}); err == nil { + t.Fatal("a batch for another checkout must have no candidate to integrate") + } + if _, found := k.branchTip("dev-int"); found { + t.Fatal("the candidate of another checkout moved a branch here") + } +} diff --git a/internal/relay/dagsched/integration_push.go b/internal/relay/dagsched/integration_push.go new file mode 100644 index 0000000000..0e69e3d316 --- /dev/null +++ b/internal/relay/dagsched/integration_push.go @@ -0,0 +1,198 @@ +package dagsched + +import ( + "context" + "encoding/json" + "regexp" + "strings" + + "github.com/thisisjun786/codex-relay-workflow/internal/contract" +) + +// VerifiedMoveOnto is whether the relay moved integrationRef onto commit after verifying it (CRW-965, parent decision D6). The +// one source is the move itself: a ref_moved stage row names the new head, and the planned intent of its batch carries the +// verified head and a verification digest for the same ref. A batch row is not consulted, so a move the reconciliation +// recorded after a crash counts the same as a move the batch recorded. +func (s *Scheduler) VerifiedMoveOnto(ctx context.Context, integrationRef, commit string) (bool, error) { + return s.plannedVerifiedMove(ctx, integrationRef, commit) +} + +// plannedVerifiedMove is whether a move the reconciliation recorded (a ref_moved stage with no batch row) names commit, and its +// planned intent carries the verified head and a verification digest for integrationRef (CRW-965, parent decision D6). +func (s *Scheduler) plannedVerifiedMove(ctx context.Context, integrationRef, commit string) (bool, error) { + rows, err := s.Store.Q(ctx).QueryContext(ctx, "SELECT i.detail FROM dag_integration_stages m JOIN dag_integration_stages i ON i.batch_id = m.batch_id AND i.stage = 'intent' AND i.node_id = '' WHERE m.stage = 'ref_moved' AND m.detail = ?", commit) + if err != nil { + return false, err + } + defer rows.Close() + for rows.Next() { + var raw string + if err := rows.Scan(&raw); err != nil { + return false, err + } + var intent map[string]string + if json.Unmarshal([]byte(raw), &intent) == nil && intent["verified_head"] == commit && intent["integration_ref"] == integrationRef && intent["verification_digest"] != "" { + return true, nil + } + } + return false, rows.Err() +} + +// The push of the integration branch to a remote branch (CRW-965, criteria c6 and c7). It moves the remote branch to the +// integration commit only by a fast-forward. The remote branch is read first: an equal remote is up to date; a remote that +// holds a commit the integration branch contains is advanced; a remote that holds a commit the branch does not contain is +// refused with merge_base_mismatch and nothing is pushed. There is no forced push. A remote that cannot be reached (a +// GitHub outage) defers the push, and the integration the batch already recorded stands. + +// SchemaIntegrationPush names the document dag-integrate-push prints. +const SchemaIntegrationPush = "dag-integrate-push/1" + +// Outcomes of one push. pushed moved the remote branch to the integration head; up_to_date needed nothing; deferred +// means the remote could not be reached, so the push waits and the same command can be run again later. +const ( + PushPushed = "pushed" + PushUpToDate = "up_to_date" + PushDeferred = "deferred" +) + +// PushResult is the answer of one push attempt. +type PushResult struct { + Remote, RemoteRef string + LocalHead string + RemoteHead string + Outcome string + Detail string +} + +// MovedHeadCheck answers whether the relay verified a commit and moved an integration ref onto it. +type MovedHeadCheck func(ctx context.Context, integrationRef, commit string) (bool, error) + +// PushIntegration moves a remote branch to a local integration branch's commit by a fast-forward only. The branch's tip +// must be a head the relay verified and moved the branch onto; any other tip is refused before the remote is read (CRW-965, +// parent decision D6). +func PushIntegration(ctx context.Context, checkout, remote, remoteRef, integrationRef string, moved MovedHeadCheck) (PushResult, error) { + out := PushResult{Remote: remote, RemoteRef: remoteRef} + // a name that starts with a dash would be read by git as an option (CRW-965 review): refused before git runs + if strings.HasPrefix(remote, "-") || strings.HasPrefix(remoteRef, "-") { + return out, refuse(contract.RefusalMergeTargetUnreadable, "the remote %q and the remote ref %q must not start with a dash", remote, remoteRef) + } + local, found, err := integrationBranchTip(ctx, checkout, integrationRef) + if err != nil { + return out, refuse(contract.RefusalMergeTargetUnreadable, "git could not read %s in %s: %v", integrationRef, checkout, err) + } + if !found { + return out, refuse(contract.RefusalMergeTargetUnreadable, "%s names no branch in %s", integrationRef, checkout) + } + out.LocalHead = local + verified, err := moved(ctx, integrationRef, local) + if err != nil { + return out, err + } + if !verified { + return out, refuse(contract.RefusalMergeBaseMismatch, "%s in %s points at %s, which the relay did not verify and move the branch onto: nothing is pushed", integrationRef, checkout, local) + } + pushTarget := remoteURLForPush(ctx, checkout, remote) + remoteHead, unreachableRemote, err := lsRemoteHead(ctx, checkout, pushTarget, remoteRef) + if err != nil { + if unreachableRemote { + out.Outcome, out.Detail = PushDeferred, err.Error() + return out, nil + } + return out, refuse(contract.RefusalMergeTargetUnreadable, "git could not read %s of %s: %v", remoteRef, remote, err) + } + out.RemoteHead = remoteHead + if remoteHead == local { + out.Outcome = PushUpToDate + return out, nil + } + if remoteHead != "" { + if !hasCommitIn(ctx, checkout, remoteHead) { + return out, refuse(contract.RefusalMergeBaseMismatch, "%s on %s points at %s, which %s does not hold: nothing is pushed", remoteRef, remote, remoteHead, checkout) + } + ancestor, err := isAncestorIn(ctx, checkout, remoteHead, local) + if err != nil { + return out, refuse(contract.RefusalMergeTargetUnreadable, "git could not compare %s with %s: %v", remoteHead, local, err) + } + if !ancestor { + return out, refuse(contract.RefusalMergeBaseMismatch, "%s on %s points at %s, which %s does not contain: nothing is pushed", remoteRef, remote, remoteHead, integrationRef) + } + } + if err := pushCommit(ctx, checkout, pushTarget, local, remoteRef); err != nil { + if _, ok := err.(*pushUnreachable); ok { + out.Outcome, out.Detail = PushDeferred, err.Error() + return out, nil + } + return out, refuse(contract.RefusalMergeTargetUnreadable, "git could not push %s to %s: %v", local, remote, err) + } + out.Outcome, out.RemoteHead = PushPushed, local + return out, nil +} + +// fullCommitPattern is the shape of a full object name this package accepts: 40 hex digits, or 64 for a sha256 repository. +var fullCommitPattern = regexp.MustCompile("^[0-9a-f]{40}([0-9a-f]{24})?$") + +// lsRemoteHead reads where a remote branch points. unreachable reports a failure that means "the remote could not be +// reached", which defers the push; any other failure is a refusal. +func lsRemoteHead(ctx context.Context, checkout, remote, branch string) (string, bool, error) { + code, out, err := runGitExit(ctx, checkout, nil, "ls-remote", remote, "refs/heads/"+branch) + if err != nil { + return "", code < 0 || unreachableGitMessage(err.Error()), err + } + for _, line := range strings.Split(out, "\n") { + fields := strings.Fields(line) + if len(fields) == 2 && fields[1] == "refs/heads/"+branch && fullCommitPattern.MatchString(fields[0]) { + return fields[0], false, nil + } + } + return "", false, nil +} + +// unreachableGitMessage is whether git's message means the remote could not be reached rather than that it answered and +// refused. +func unreachableGitMessage(message string) bool { + lower := strings.ToLower(message) + for _, needle := range []string{"could not resolve host", "connection refused", "connection timed out", "network is unreachable", + "could not read from remote repository", "no route to host", "operation timed out", "temporary failure in name resolution", + "could not connect to server", "failed to connect to", "returned error: 5"} { + if strings.Contains(lower, needle) { + return true + } + } + return false +} + +// pushUnreachable is the error a push returns when the remote could not be reached. +type pushUnreachable struct{ detail string } + +func (e *pushUnreachable) Error() string { return e.detail } + +// pushCommit pushes a local commit to a remote branch with no force and no leading plus, so the remote refuses the update +// unless it is a fast-forward of what it holds. +func pushCommit(ctx context.Context, checkout, remote, commit, branch string) error { + code, _, err := runGitExit(ctx, checkout, nil, "push", remote, commit+":refs/heads/"+branch) + if err == nil { + return nil + } + if code < 0 || unreachableGitMessage(err.Error()) { + return &pushUnreachable{detail: err.Error()} + } + return err +} + +// hasCommitIn reports whether a checkout holds a commit object. +func hasCommitIn(ctx context.Context, checkout, commit string) bool { + code, _, _ := runGitExit(ctx, checkout, nil, "cat-file", "-e", commit+"^{commit}") + return code == 0 +} + +// isAncestorIn reports whether one commit is contained in another in a checkout (merge-base --is-ancestor). +func isAncestorIn(ctx context.Context, checkout, ancestor, descendant string) (bool, error) { + code, _, err := runGitExit(ctx, checkout, nil, "merge-base", "--is-ancestor", ancestor, descendant) + if code == 0 { + return true, nil + } + if code == 1 { + return false, nil + } + return false, err +} diff --git a/internal/relay/dagsched/integration_push_target.go b/internal/relay/dagsched/integration_push_target.go new file mode 100644 index 0000000000..50ca57ac2f --- /dev/null +++ b/internal/relay/dagsched/integration_push_target.go @@ -0,0 +1,20 @@ +package dagsched + +import ( + "context" + "strings" +) + +// remoteURLForPush is the address a push goes to: the remote's push URL when one is configured, else the remote named. +// The remote branch is read from the same address the push writes to, so a separate push URL cannot hide a divergence +// (CRW-965 review). +func remoteURLForPush(ctx context.Context, checkout, remote string) string { + out, err := runGit(ctx, checkout, nil, "remote", "get-url", "--push", remote) + if err != nil { + return remote + } + if url := strings.TrimSpace(out); url != "" { + return url + } + return remote +} diff --git a/internal/relay/dagsched/integration_push_test.go b/internal/relay/dagsched/integration_push_test.go new file mode 100644 index 0000000000..9fa7fbcf32 --- /dev/null +++ b/internal/relay/dagsched/integration_push_test.go @@ -0,0 +1,137 @@ +package dagsched + +import ( + "context" + "os/exec" + "strings" + "testing" +) + +// The push of the integration branch (CRW-965, c6 and c7), against a temporary bare remote. A GitHub outage is +// simulated with a push URL that refuses connections while the fetch URL, which ls-remote reads, still works. + +// bareRemote is a temporary bare repository the push tests use as origin. +func bareRemote(t *testing.T) string { + t.Helper() + dir := t.TempDir() + if out, err := exec.Command("git", "init", "-q", "--bare", dir).CombinedOutput(); err != nil { + t.Fatalf("git init --bare: %v: %s", err, out) + } + return dir +} + +// remoteBranch reads the commit a bare remote's branch points at, or "" when it has none. +func remoteBranch(t *testing.T, remote, branch string) string { + t.Helper() + out, err := exec.Command("git", "-C", remote, "rev-parse", "--verify", "-q", "refs/heads/"+branch).Output() + if err != nil { + return "" + } + return strings.TrimSpace(string(out)) +} + +// pushRepo is a checkout of the kit with origin pointing at the bare remote. +func (k *batchKit) pushRepo(t *testing.T, remote string) { + t.Helper() + k.repo.git("remote", "add", "origin", remote) +} + +// TestPushFastForwardsTheSameCommit: the remote branch takes the integration commit by a fast-forward, and a second +// push is up to date (c7). +func TestPushFastForwardsTheSameCommit(t *testing.T) { + k := newBatchKit(t, batchNode{name: "a", files: map[string]string{"a.txt": "a\n"}}) + remote := bareRemote(t) + k.pushRepo(t, remote) + k.acceptByCommit("a") + res, err := k.sched.IntegrateBatch(context.Background(), k.batchIn(), IntegrationBatchDeps{Verify: stubVerifier(writeStubVerifier(t)), Update: updateIntegrationRef}) + if err != nil { + t.Fatalf("batch: %v", err) + } + first, err := PushIntegration(context.Background(), k.repo.path, "origin", "dev", "dev-int", k.sched.VerifiedMoveOnto) + if err != nil || first.Outcome != PushPushed { + t.Fatalf("first push: %+v, %v", first, err) + } + if got := remoteBranch(t, remote, "dev"); got != res.NewHead { + t.Fatalf("remote dev is %s; want the integration commit %s", got, res.NewHead) + } + second, err := PushIntegration(context.Background(), k.repo.path, "origin", "dev", "dev-int", k.sched.VerifiedMoveOnto) + if err != nil || second.Outcome != PushUpToDate { + t.Fatalf("second push: %+v, %v; want up_to_date", second, err) + } +} + +// c7: the remote dev points at a commit the integration branch does not contain. Nothing is pushed, the command reports +// merge_base_mismatch, and the remote keeps its commit. +func TestPushRefusesToOverwriteARemoteThatHasMoved(t *testing.T) { + k := newBatchKit(t, batchNode{name: "a", files: map[string]string{"a.txt": "a\n"}}) + remote := bareRemote(t) + k.pushRepo(t, remote) + k.acceptByCommit("a") + if _, err := k.sched.IntegrateBatch(context.Background(), k.batchIn(), IntegrationBatchDeps{Verify: stubVerifier(writeStubVerifier(t)), Update: updateIntegrationRef}); err != nil { + t.Fatalf("batch: %v", err) + } + k.repo.git("checkout", "-q", "-B", "elsewhere", k.base) + k.repo.write("other.txt", "other\n") + k.repo.git("add", "other.txt") + k.repo.git("commit", "-q", "-m", "other") + other := k.repo.git("rev-parse", "HEAD") + k.repo.git("push", "-q", remote, other+":refs/heads/dev") + _, err := PushIntegration(context.Background(), k.repo.path, "origin", "dev", "dev-int", k.sched.VerifiedMoveOnto) + if refusalReasonOf(err) != "merge_base_mismatch" { + t.Fatalf("divergent push: %v; want merge_base_mismatch", err) + } + if got := remoteBranch(t, remote, "dev"); got != other { + t.Fatalf("the remote dev moved to %s; it must keep %s", got, other) + } +} + +// c6: a GitHub outage. ls-remote reads the remote, the push cannot reach it: the push is deferred, the integration the +// batch recorded stands, the acceptance and the integrated count go through, and the same push completes later. +func TestOutageDefersOnlyThePushAndTheIntegrationCompletes(t *testing.T) { + k := newBatchKit(t, batchNode{name: "a", files: map[string]string{"a.txt": "a\n"}}, batchNode{name: "b", files: map[string]string{"b.txt": "b\n"}}) + remote := bareRemote(t) + k.pushRepo(t, remote) + k.repo.git("config", "remote.origin.pushurl", "http://127.0.0.1:1/codex-relay-workflow.git") + k.acceptByCommit("a") + k.acceptByCommit("b") + res, err := k.sched.IntegrateBatch(context.Background(), k.batchIn(), IntegrationBatchDeps{Verify: stubVerifier(writeStubVerifier(t)), Update: updateIntegrationRef}) + if err != nil || len(res.Merged) != 2 { + t.Fatalf("batch during the outage: %+v, %v", res, err) + } + // integration is observed on the local integration branch, the target the node has no edge for + k.sched.Tips = newLegacyLocalTipReader(t) + k.sched.Ancestry = GitAncestry{}.Ancestry + for _, node := range []string{"a", "b"} { + obs, err := k.sched.ObserveIntegration(context.Background(), "g", node, "parent", []Target{{Repository: k.repo.path, BaseRef: "dev-int"}}) + if err != nil || !obs.Integrated { + t.Fatalf("observe %s on the integration branch: %+v, %v; want integrated", node, obs, err) + } + } + progress, err := k.sched.ReadProgress(context.Background(), "g") + if err != nil { + t.Fatal(err) + } + if progress.Cumulative.Integrated.Nodes != 2 { + t.Fatalf("integrated count %d during the outage; want 2", progress.Cumulative.Integrated.Nodes) + } + deferred, err := PushIntegration(context.Background(), k.repo.path, "origin", "dev", "dev-int", k.sched.VerifiedMoveOnto) + if err != nil || deferred.Outcome != PushDeferred { + t.Fatalf("push during the outage: %+v, %v; want deferred", deferred, err) + } + if got := remoteBranch(t, remote, "dev"); got != "" { + t.Fatalf("the remote dev was written during the outage: %s", got) + } + // the outage ends: the same push completes, and the integrated count is unchanged + k.repo.git("config", "--unset", "remote.origin.pushurl") + done, err := PushIntegration(context.Background(), k.repo.path, "origin", "dev", "dev-int", k.sched.VerifiedMoveOnto) + if err != nil || done.Outcome != PushPushed { + t.Fatalf("push after the outage: %+v, %v; want pushed", done, err) + } + if got := remoteBranch(t, remote, "dev"); got != res.NewHead { + t.Fatalf("remote dev is %s; want %s", got, res.NewHead) + } + after, err := k.sched.ReadProgress(context.Background(), "g") + if err != nil || after.Cumulative.Integrated.Nodes != 2 { + t.Fatalf("integrated count after the push: %v, %v; want 2", after.Cumulative.Integrated.Nodes, err) + } +} diff --git a/internal/relay/dagsched/integration_push_verified_test.go b/internal/relay/dagsched/integration_push_verified_test.go new file mode 100644 index 0000000000..c5683f2084 --- /dev/null +++ b/internal/relay/dagsched/integration_push_verified_test.go @@ -0,0 +1,48 @@ +package dagsched + +import ( + "context" + "testing" +) + +// CRW-965 (parent decision D6): the push takes only a head the relay verified and moved the branch onto. A tip with one +// local commit on top of the verified move, and a tip of another branch, are refused and the remote does not move. +func TestPushRefusesATipThatTheRelayDidNotMove(t *testing.T) { + cases := map[string]func(k *batchKit, other string){ + "an extra local commit on the verified move": func(k *batchKit, other string) { + k.repo.git("checkout", "-q", "dev-int") + k.repo.write("extra.txt", "extra\n") + k.repo.git("add", "extra.txt") + k.repo.git("commit", "-q", "-m", "extra") + k.repo.git("checkout", "-q", "dev") + }, + "a tip of an unrelated branch": func(k *batchKit, other string) { + k.repo.git("update-ref", "refs/heads/dev-int", other) + }, + } + for name, move := range cases { + t.Run(name, func(t *testing.T) { + k := newBatchKit(t, batchNode{name: "a", files: map[string]string{"a.txt": "a\n"}}) + remote := bareRemote(t) + k.pushRepo(t, remote) + k.acceptByCommit("a") + if _, err := k.sched.IntegrateBatch(context.Background(), k.batchIn(), IntegrationBatchDeps{Verify: stubVerifier(writeStubVerifier(t)), Update: updateIntegrationRef}); err != nil { + t.Fatalf("batch: %v", err) + } + k.repo.git("checkout", "-q", "-B", "unrelated", k.base) + k.repo.write("unrelated.txt", "unrelated\n") + k.repo.git("add", "unrelated.txt") + k.repo.git("commit", "-q", "-m", "unrelated") + other := k.repo.git("rev-parse", "HEAD") + k.repo.git("checkout", "-q", "dev") + move(k, other) + _, err := PushIntegration(context.Background(), k.repo.path, "origin", "dev", "dev-int", k.sched.VerifiedMoveOnto) + if refusalReasonOf(err) != "merge_base_mismatch" { + t.Fatalf("a tip the relay did not move must be refused before the push: %v", err) + } + if got := remoteBranch(t, remote, "dev"); got != "" { + t.Fatalf("the remote moved to %s although the tip was not a verified move", got) + } + }) + } +} diff --git a/internal/relay/dagsched/integration_rerun_test.go b/internal/relay/dagsched/integration_rerun_test.go new file mode 100644 index 0000000000..fe39350646 --- /dev/null +++ b/internal/relay/dagsched/integration_rerun_test.go @@ -0,0 +1,42 @@ +package dagsched + +import ( + "context" + "reflect" + "testing" +) + +// CRW-965 (review finding): a batch that runs again after a candidate already reached the integration branch (before the +// containment observation) must not merge that candidate again or report a merge commit for it. Only the new candidate +// is merged, verified and reported. +func TestIntegrationBatchDoesNotMergeACandidateTheBranchAlreadyHolds(t *testing.T) { + k := newBatchKit(t, batchNode{name: "a", files: map[string]string{"a.txt": "a\n"}}, batchNode{name: "b", files: map[string]string{"b.txt": "b\n"}}) + runs := 0 + base := stubVerifier(writeStubVerifier(t)) + deps := IntegrationBatchDeps{Verify: func(ctx context.Context, dir string, env []string) error { + runs++ + return base(ctx, dir, env) + }, Update: updateIntegrationRef} + ctx := context.Background() + k.acceptByCommit("a") + k.acceptByCommit("b") + first := k.batchIn() + first.Nodes = []string{"a"} + if _, err := k.sched.IntegrateBatch(ctx, first, deps); err != nil { + t.Fatalf("first batch: %v", err) + } + res, err := k.sched.IntegrateBatch(ctx, k.batchIn(), deps) + if err != nil { + t.Fatalf("second batch: %v", err) + } + var merged []string + for _, m := range res.Merged { + merged = append(merged, m.NodeID) + } + if !reflect.DeepEqual(merged, []string{"b"}) { + t.Fatalf("the second batch merged %v; want only [b]: %+v", merged, res) + } + if runs != 2 { + t.Fatalf("verifier ran %d times; want 2 (one per batch)", runs) + } +} diff --git a/internal/relay/dagsched/integration_targets_test.go b/internal/relay/dagsched/integration_targets_test.go new file mode 100644 index 0000000000..78e81ba7ba --- /dev/null +++ b/internal/relay/dagsched/integration_targets_test.go @@ -0,0 +1,40 @@ +package dagsched + +import ( + "context" + "errors" + "testing" +) + +// CRW-965 (parent decision D4): only an integration ref a batch has moved is a completion target. A batch that failed +// before its move, on another ref, adds nothing to the refs an acceptance completes on. +func TestIntegrationTargetsCountOnlyMovedRefs(t *testing.T) { + k := newBatchKit(t, batchNode{name: "a", files: map[string]string{"a.txt": "a\n"}}) + k.acceptByCommit("a") + ctx := context.Background() + acc, found, err := loadActiveAcceptance(ctx, k.sched.Store.Q(ctx), "g", "a") + if err != nil || !found { + t.Fatalf("the active acceptance of a: %v, %v", found, err) + } + failing := k.batchIn() + failing.IntegrationRef = "dev-failed" + crash := IntegrationBatchDeps{Verify: stubVerifier(writeStubVerifier(t)), Update: func(context.Context, string, string, string, string) error { + return errors.New("simulated refusal before the branch moved") + }} + if _, err := k.sched.IntegrateBatch(ctx, failing, crash); err == nil { + t.Fatal("the failing batch should not complete") + } + if _, found := k.branchTip("dev-failed"); found { + t.Fatal("the failing batch must not have moved dev-failed") + } + if _, err := k.sched.IntegrateBatch(ctx, k.batchIn(), IntegrationBatchDeps{Verify: stubVerifier(writeStubVerifier(t)), Update: updateIntegrationRef}); err != nil { + t.Fatalf("the moving batch: %v", err) + } + refs, err := integrationRefsOf(ctx, k.sched.Store.Q(ctx), acc.AcceptanceID) + if err != nil { + t.Fatal(err) + } + if len(refs) != 1 || refs[0] != "dev-int" { + t.Fatalf("the completion targets are the moved ref only: %v", refs) + } +} diff --git a/internal/relay/dagsched/targets.go b/internal/relay/dagsched/targets.go index 0057f5d8ce..28e018b254 100644 --- a/internal/relay/dagsched/targets.go +++ b/internal/relay/dagsched/targets.go @@ -17,13 +17,34 @@ type Target struct{ Repository, BaseRef string } // A terminal node has no outgoing edge: its target arrives with its first observation (dag-integration-observe --target). func (s *Scheduler) nodeTargets(ctx context.Context, q store.Querier, snap dag.Snapshot, a Acceptance) ([]Target, error) { seen := map[Target]bool{} + // a commit-accepted node completes on the integration branch its batches moved, not on the base ref its edge names (CRW-965, D4) + commit, err := commitAccepted(ctx, q, a.AcceptanceID) + if err != nil { + return nil, err + } + var refs []string + if commit { + if refs, err = integrationRefsOf(ctx, q, a.AcceptanceID); err != nil { + return nil, err + } + } for _, e := range snap.Edges { - if e.FromNodeID != a.NodeID || e.TargetRepository == "" || e.TargetBaseRef == "" { + if e.FromNodeID != a.NodeID || e.TargetRepository == "" { continue } - if e.Kind == dag.EdgeIntegrated || (e.Kind == dag.EdgeArtifactVerified && e.PinsCodeHead) { - seen[Target{e.TargetRepository, e.TargetBaseRef}] = true + if e.Kind != dag.EdgeIntegrated && !(e.Kind == dag.EdgeArtifactVerified && e.PinsCodeHead) { + continue + } + if commit { + for _, ref := range refs { + seen[Target{e.TargetRepository, ref}] = true + } + continue + } + if e.TargetBaseRef == "" { + continue } + seen[Target{e.TargetRepository, e.TargetBaseRef}] = true } rows, err := q.QueryContext(ctx, "SELECT DISTINCT repository, base_ref FROM dag_integration_observations WHERE acceptance_id = ?", a.AcceptanceID) if err != nil { diff --git a/internal/relay/dagsched/verification_record.go b/internal/relay/dagsched/verification_record.go new file mode 100644 index 0000000000..598db9b64c --- /dev/null +++ b/internal/relay/dagsched/verification_record.go @@ -0,0 +1,224 @@ +package dagsched + +import ( + "bytes" + "context" + "crypto/sha256" + "encoding/hex" + "encoding/json" + "strings" + + "github.com/thisisjun786/codex-relay-workflow/internal/contract" +) + +// VerificationRecord is the verification-record/1 document CRW-964's writer (crw-dev ci local) leaves: the +// members below are the exact camelCase names that writer emits (internal/dev/ci/local_record.go). The digest is +// the sha256 of the record's canonical serialization: the document as JSON with sorted keys, no HTML escaping, the +// digest member removed and no trailing newline. The reader recomputes it from the record's own bytes. +type VerificationRecord struct { + Schema string `json:"schema"` + Runner string `json:"runner"` + Repository string `json:"repository"` + BaseCommit string `json:"baseCommit"` + HeadCommit string `json:"headCommit"` + TreeHash string `json:"treeHash"` + CiDigest string `json:"ciDigest"` + Tools map[string]string `json:"tools"` + Pins map[string]string `json:"pins"` + GoFlags string `json:"goFlags"` + GoEnv string `json:"goEnv"` + PinMismatch []string `json:"pinMismatch"` + Dependencies map[string]string `json:"dependencies"` + OS string `json:"os"` + Arch string `json:"arch"` + Result string `json:"result"` + Jobs []json.RawMessage `json:"jobs"` + Digest string `json:"digest"` +} + +// VerificationRecordResultPass is the one result word a reusable record carries (CRW-964 writes "pass"). +const VerificationRecordResultPass = "pass" + +// verificationDependencyFiles are the dependency files the record digests, by the names the record uses. +var verificationDependencyFiles = []string{"go.sum", "web/package-lock.json"} + +// verificationCIFile is the workflow file whose digest the record carries. +const verificationCIFile = ".github/workflows/ci.yml" + +// verificationRequired are the members a reusable record must carry. runner, pins, jobs and headCommit are +// not in the list: the reuse keys do not decide on them (headCommit deliberately, since the same tree under +// another commit is the same verification). +var verificationRequired = []string{"schema", "repository", "baseCommit", "treeHash", "ciDigest", "tools", "goFlags", "goEnv", + "pinMismatch", "dependencies", "os", "arch", "result", "digest"} + +// VerificationKeys are what a reusable record must agree with: the tree it names, the base the range covers, the +// ci.yml and dependency digests of that commit, and the platform the relay host runs on. +type VerificationKeys struct { + Tree string + Base string + CiDigest string + Dependencies map[string]string + OS, Arch string +} + +// DecodeVerificationRecord reads the document and refuses what is not a verification-record/1: a document that +// is not a JSON object, or whose schema is another. Whether it is reusable is JudgeVerificationRecord's question. +func DecodeVerificationRecord(raw []byte) (VerificationRecord, error) { + var record VerificationRecord + decoder := json.NewDecoder(bytes.NewReader(raw)) + if err := decoder.Decode(&record); err != nil { + return record, refuse(contract.RefusalMalformedReceipt, "the verification record is not a JSON document: %v", err) + } + if decoder.More() { + return record, refuse(contract.RefusalMalformedReceipt, "the verification record is one JSON object") + } + if record.Schema != VerificationRecordSchema { + return record, refuse(contract.RefusalMalformedReceipt, "the verification record names schema %q; this path reads %s", record.Schema, VerificationRecordSchema) + } + return record, nil +} + +// canonicalVerificationRecord is the canonical serialization of a record's bytes: the object without its digest +// member, sorted keys (a map encodes sorted), no HTML escaping, no trailing newline. +func canonicalVerificationRecord(raw []byte) ([]byte, error) { + var fields map[string]any + if err := json.Unmarshal(raw, &fields); err != nil { + return nil, err + } + delete(fields, "digest") + var out bytes.Buffer + encoder := json.NewEncoder(&out) + encoder.SetEscapeHTML(false) + if err := encoder.Encode(fields); err != nil { + return nil, err + } + return bytes.TrimSuffix(out.Bytes(), []byte("\n")), nil +} + +// VerificationRecordDigest is the digest the record's bytes must carry: sha256: of their canonical form. +func VerificationRecordDigest(raw []byte) (string, error) { + canonical, err := canonicalVerificationRecord(raw) + if err != nil { + return "", err + } + sum := sha256.Sum256(canonical) + return "sha256:" + hex.EncodeToString(sum[:]), nil +} + +// SealVerificationRecord fills in a record's digest the way CRW-964's writer does and returns its bytes, for the +// writer's own tests and for fixtures. A record built by hand must be sealed before a reader accepts it. +func SealVerificationRecord(record VerificationRecord) ([]byte, error) { + record.Schema = VerificationRecordSchema + record.Digest = "" + raw, err := json.Marshal(record) + if err != nil { + return nil, err + } + digest, err := VerificationRecordDigest(raw) + if err != nil { + return nil, err + } + var fields map[string]any + if err := json.Unmarshal(raw, &fields); err != nil { + return nil, err + } + fields["digest"] = digest + return json.Marshal(fields) +} + +// JudgeVerificationRecord decides whether a record may be reused for the keys given. Every failure is refused: +// a record of another tree under revision_mismatch, every other reason under disposition_conflict with its own +// detail. The checks run in the order a reader would meet them: presence, digest, result, pins, then the keys. +func JudgeVerificationRecord(raw []byte, want VerificationKeys) (VerificationRecord, error) { + record, err := DecodeVerificationRecord(raw) + if err != nil { + return record, err + } + var members map[string]json.RawMessage + if err := json.Unmarshal(raw, &members); err != nil { + return record, refuse(contract.RefusalMalformedReceipt, "the verification record is not a JSON object: %v", err) + } + for _, name := range verificationRequired { + if _, ok := members[name]; !ok { + return record, refuse(contract.RefusalDispositionConflict, "the verification record has no %s member: a record without every reuse key is not reusable", name) + } + } + digest, err := VerificationRecordDigest(raw) + if err != nil { + return record, refuse(contract.RefusalMalformedReceipt, "the verification record cannot be canonicalised: %v", err) + } + if record.Digest != digest { + return record, refuse(contract.RefusalDispositionConflict, "the verification record's digest %s does not match its contents (%s)", record.Digest, digest) + } + if record.Result != VerificationRecordResultPass { + return record, refuse(contract.RefusalDispositionConflict, "the verification record's result is %q: only %q is reused", record.Result, VerificationRecordResultPass) + } + if len(record.PinMismatch) > 0 { + return record, refuse(contract.RefusalDispositionConflict, "the verification record carries a tool pin mismatch (%s), so it is not reusable", strings.Join(record.PinMismatch, ", ")) + } + // a tool that differs from its own pin makes the record internally inconsistent (CRW-965, parent decision D1 refined) + for name, pinned := range record.Pins { + if tool, ok := record.Tools[name]; !ok || tool != pinned { + return record, refuse(contract.RefusalDispositionConflict, "the verification record's tool %s is %q but its pin is %q: the record is not reusable", name, tool, pinned) + } + } + if !strings.EqualFold(record.TreeHash, want.Tree) { + return record, refuse(contract.RefusalRevisionMismatch, "the verification record names tree %s and the tree judged is %s", record.TreeHash, want.Tree) + } + if record.BaseCommit != want.Base { + return record, refuse(contract.RefusalDispositionConflict, "the verification record covers base %s and the base is %s", record.BaseCommit, want.Base) + } + if record.CiDigest != want.CiDigest { + return record, refuse(contract.RefusalDispositionConflict, "the verification record's ci.yml digest %s is not the commit's %s", record.CiDigest, want.CiDigest) + } + for _, file := range verificationDependencyFiles { + if record.Dependencies[file] != want.Dependencies[file] { + return record, refuse(contract.RefusalDispositionConflict, "the verification record's %s digest %q is not the commit's %q", file, record.Dependencies[file], want.Dependencies[file]) + } + } + if record.OS != want.OS || record.Arch != want.Arch { + return record, refuse(contract.RefusalDispositionConflict, "the verification record ran on %s/%s and this host is %s/%s", record.OS, record.Arch, want.OS, want.Arch) + } + return record, nil +} + +// CommitVerificationKeys reads, in a local checkout, what a record must agree with for one commit: its tree, the +// digest of its ci.yml, and the digests of its dependency files. A file the commit does not have digests to "". +func CommitVerificationKeys(ctx context.Context, checkout, commit string) (VerificationKeys, error) { + var keys VerificationKeys + tree, err := runGit(ctx, checkout, nil, "rev-parse", commit+"^{tree}") + if err != nil { + return keys, refuse(contract.RefusalMergeTargetUnreadable, "git could not read the tree of %s in %s: %v", commit, checkout, err) + } + keys.Tree = strings.TrimSpace(tree) + keys.CiDigest, err = commitFileDigest(ctx, checkout, commit, verificationCIFile) + if err != nil { + return keys, err + } + keys.Dependencies = map[string]string{} + for _, file := range verificationDependencyFiles { + digest, err := commitFileDigest(ctx, checkout, commit, file) + if err != nil { + return keys, err + } + keys.Dependencies[file] = digest + } + return keys, nil +} + +// commitFileDigest is sha256: of a file as a commit has it, or "" when the commit has no such file. +func commitFileDigest(ctx context.Context, checkout, commit, file string) (string, error) { + code, _, err := runGitExit(ctx, checkout, nil, "cat-file", "-e", commit+":"+file) + if err != nil && code < 0 { + return "", refuse(contract.RefusalMergeTargetUnreadable, "git could not read %s of %s: %v", file, commit, err) + } + if code != 0 { + return "", nil + } + body, err := runGit(ctx, checkout, nil, "show", commit+":"+file) + if err != nil { + return "", refuse(contract.RefusalMergeTargetUnreadable, "git could not read %s of %s: %v", file, commit, err) + } + sum := sha256.Sum256([]byte(body)) + return "sha256:" + hex.EncodeToString(sum[:]), nil +} diff --git a/internal/relay/integrate/cli.go b/internal/relay/integrate/cli.go new file mode 100644 index 0000000000..6502a011ef --- /dev/null +++ b/internal/relay/integrate/cli.go @@ -0,0 +1,218 @@ +package integrate + +import ( + "bytes" + "context" + "errors" + "fmt" + "os" + "os/exec" + "strings" + + "github.com/thisisjun786/codex-relay-workflow/internal/contract" + "github.com/thisisjun786/codex-relay-workflow/internal/relay/dagsched" + "github.com/thisisjun786/codex-relay-workflow/internal/relay/dispatch" + "github.com/thisisjun786/codex-relay-workflow/internal/relay/store" +) + +// The relay commands of the integration batch (CRW-965). dag-integrate merges the plan's ready accepted candidates onto +// a local integration branch and verifies the merged tree; dag-integrate-push moves the remote branch to that commit by a +// fast-forward. Neither opens a pull request or touches the merge lane. + +// SchemaIntegrate names the document dag-integrate prints. +const SchemaIntegrate = "dag-integrate/1" + +func init() { + dispatch.Register(nil, + dispatch.Command{Name: "dag-integrate", Run: runIntegrate}, + dispatch.Command{Name: "dag-integrate-push", Run: runPush}, + ) +} + +func runIntegrate(ctx context.Context, services dispatch.Services, args dispatch.Args) (any, error) { + epoch := args.Integer("expect-epoch") + if epoch < 0 { + return nil, &dispatch.UsageError{Detail: "--expect-epoch is a whole number, 0 or more", Code: contract.ExitUsage} + } + argv, err := splitCommand(args.Text("verify")) + if err != nil { + return nil, err + } + s, err := store.Open(ctx, services.Selection.DBPath(), services.SocketPath) + if err != nil { + return nil, err + } + defer func() { _ = s.Close() }() + sched := &dagsched.Scheduler{Store: s, ExpectedEpoch: epoch} + result, err := sched.IntegrateBatch(ctx, dagsched.IntegrationBatchInput{ + Plan: args.Text("plan"), Actor: args.Text("actor"), Checkout: args.Text("checkout"), + IntegrationRef: args.Text("integration-ref"), BaseRef: args.Text("base"), Nodes: args.Strings("node"), + }, dagsched.IntegrationBatchDeps{Verify: commandVerifier(argv), Update: updateRef}) + if err != nil { + return nil, err + } + merged := make([]any, len(result.Merged)) + for i, m := range result.Merged { + merged[i] = contract.OrderedObject{{Key: "node_id", Value: m.NodeID}, {Key: "acceptance_id", Value: m.AcceptanceID}, {Key: "head_sha", Value: m.HeadSHA}, {Key: "merge_commit", Value: m.MergeCommit}} + } + split := make([]any, len(result.Split)) + for i, c := range result.Split { + split[i] = contract.OrderedObject{{Key: "node_id", Value: c.NodeID}, {Key: "acceptance_id", Value: c.AcceptanceID}, {Key: "head_sha", Value: c.HeadSHA}, {Key: "reason", Value: c.Reason}} + } + pending := make([]any, len(result.Pending)) + for i, p := range result.Pending { + pending[i] = p + } + targets := make([]any, len(result.Targets)) + for i, t := range result.Targets { + targets[i] = t + } + events := make([]any, len(result.MarkedEvents)) + for i, e := range result.MarkedEvents { + events[i] = e + } + already := make([]any, len(result.AlreadyContained)) + for i, c := range result.AlreadyContained { + already[i] = contract.OrderedObject{{Key: "node_id", Value: c.NodeID}, {Key: "acceptance_id", Value: c.AcceptanceID}, {Key: "head_sha", Value: c.HeadSHA}, {Key: "contained_in", Value: c.ContainedIn}, {Key: "marked_event", Value: c.MarkedEvent}, {Key: "reason", Value: c.Reason}} + } + unverified := make([]any, len(result.ContainedUnverified)) + for i, c := range result.ContainedUnverified { + unverified[i] = contract.OrderedObject{{Key: "node_id", Value: c.NodeID}, {Key: "acceptance_id", Value: c.AcceptanceID}, {Key: "head_sha", Value: c.HeadSHA}, {Key: "contained_in", Value: c.ContainedIn}} + } + reconciled := make([]any, len(result.Reconciled)) + for i, b := range result.Reconciled { + reconciled[i] = b + } + abandoned := make([]any, len(result.Abandoned)) + for i, b := range result.Abandoned { + abandoned[i] = b + } + return contract.OrderedObject{ + {Key: "ok", Value: true}, {Key: "schema", Value: SchemaIntegrate}, {Key: "plan_id", Value: result.Plan}, {Key: "batch_id", Value: result.BatchID}, + {Key: "checkout", Value: result.Checkout}, {Key: "integration_ref", Value: result.Ref}, {Key: "base_ref", Value: result.BaseRef}, + {Key: "old_head", Value: result.OldHead}, {Key: "new_head", Value: result.NewHead}, + {Key: "merged", Value: merged}, {Key: "split", Value: split}, {Key: "pending_marks", Value: pending}, + {Key: "verification", Value: contract.OrderedObject{{Key: "result", Value: result.Verification.Result}, {Key: "tree", Value: result.Verification.TreeHash}, {Key: "digest", Value: result.VerificationDigest}}}, + {Key: "marked_events", Value: events}, {Key: "already_contained", Value: already}, {Key: "contained_unverified", Value: unverified}, {Key: "reconciled_batches", Value: reconciled}, {Key: "abandoned_batches", Value: abandoned}, {Key: "targets", Value: targets}, + }, nil +} + +func runPush(ctx context.Context, services dispatch.Services, args dispatch.Args) (any, error) { + // the store answers whether the relay moved the branch onto the tip (CRW-965, parent decision D6) + s, err := store.Open(ctx, services.Selection.DBPath(), services.SocketPath) + if err != nil { + return nil, err + } + defer func() { _ = s.Close() }() + sched := &dagsched.Scheduler{Store: s} + result, err := dagsched.PushIntegration(ctx, args.Text("checkout"), args.Text("remote"), args.Text("remote-ref"), args.Text("integration-ref"), sched.VerifiedMoveOnto) + if err != nil { + return nil, err + } + return contract.OrderedObject{ + {Key: "ok", Value: result.Outcome != dagsched.PushDeferred}, {Key: "schema", Value: dagsched.SchemaIntegrationPush}, + {Key: "remote", Value: result.Remote}, {Key: "remote_ref", Value: result.RemoteRef}, + {Key: "local_head", Value: result.LocalHead}, {Key: "remote_head", Value: result.RemoteHead}, + {Key: "outcome", Value: result.Outcome}, {Key: "detail", Value: result.Detail}, + }, nil +} + +// commandVerifier runs the one verification command, given as an argument vector (never a shell), in the directory +// the batch names, with the batch's CRW_VERIFY_* variables added to the inherited environment. A non-zero exit is a +// failing run. +func commandVerifier(argv []string) dagsched.IntegrationBatchVerifier { + return func(ctx context.Context, dir string, env []string) error { + cmd := exec.CommandContext(ctx, argv[0], argv[1:]...) + cmd.Dir = dir + // inherited GIT_* variables would redirect the command's git reads to another repository, so they are dropped + for _, kv := range os.Environ() { + if !strings.HasPrefix(kv, "GIT_") { + cmd.Env = append(cmd.Env, kv) + } + } + cmd.Env = append(cmd.Env, env...) + var stderr bytes.Buffer + cmd.Stderr = &stderr + if err := cmd.Run(); err != nil { + var exitErr *exec.ExitError + if !errors.As(err, &exitErr) { + // the command never ran: a host failure, not a verification result + return &dagsched.IntegrationVerifierHostError{Detail: fmt.Sprintf("the verification command could not start: %v", err)} + } + return fmt.Errorf("the verification command failed: %v: %s", err, strings.TrimSpace(stderr.String())) + } + return nil + } +} + +// splitCommand splits a command line into an argument vector. Single and double quotes group words and a backslash +// escapes the next character; an unterminated quote or escape, or an empty command, is refused, so a command the caller +// typed is never handed to a shell. +func splitCommand(command string) ([]string, error) { + var argv []string + var current strings.Builder + started := false + var quote rune + escaped := false + for _, r := range command { + switch { + case escaped: + current.WriteRune(r) + started = true + escaped = false + case r == '\\' && quote != '\'': + escaped = true + case quote != 0: + if r == quote { + quote = 0 + } else { + current.WriteRune(r) + } + case r == '\'' || r == '"': + quote, started = r, true + case r == ' ' || r == '\t' || r == '\n': + if started { + argv = append(argv, current.String()) + current.Reset() + started = false + } + default: + current.WriteRune(r) + started = true + } + } + if escaped || quote != 0 { + return nil, refuse(contract.RefusalMalformedReceipt, "--verify carries an unterminated quote or escape") + } + if started { + argv = append(argv, current.String()) + } + if len(argv) == 0 || argv[0] == "" { + return nil, refuse(contract.RefusalMalformedReceipt, "--verify names no command") + } + return argv, nil +} + +// refuse is a refusal under an existing reason, with the relay's exit code 2. +func refuse(reason contract.RefusalReason, format string, args ...any) error { + return &store.RefusedError{Reason: string(reason), Detail: fmt.Sprintf(format, args...)} +} + +// updateRef moves a local branch to newCommit only when it still holds oldCommit: git's own compare-and-swap. The +// all-zero oldCommit creates the branch and refuses when it already exists, so a ref that moved under the batch fails +// the update instead of being overwritten. +func updateRef(ctx context.Context, checkout, ref, newCommit, oldCommit string) error { + cmd := exec.CommandContext(ctx, "git", "-C", checkout, "update-ref", "refs/heads/"+ref, newCommit, oldCommit) + // inherited GIT_* variables would redirect the write to another repository (finding d4) + for _, kv := range os.Environ() { + if !strings.HasPrefix(kv, "GIT_") { + cmd.Env = append(cmd.Env, kv) + } + } + var stderr bytes.Buffer + cmd.Stderr = &stderr + if err := cmd.Run(); err != nil { + return fmt.Errorf("git update-ref refs/heads/%s: %v: %s", ref, err, strings.TrimSpace(stderr.String())) + } + return nil +} diff --git a/internal/relay/integrate/cli_split_test.go b/internal/relay/integrate/cli_split_test.go new file mode 100644 index 0000000000..bdebc47213 --- /dev/null +++ b/internal/relay/integrate/cli_split_test.go @@ -0,0 +1,35 @@ +package integrate + +import ( + "reflect" + "testing" +) + +// CRW-965 revision (d5): an escaped character always starts an argument. The escaped branch used to write the character +// without marking the argument as started, so an escaped character followed by whitespace or the end was lost. +func TestSplitCommandKeepsAnEscapedCharacterAsAnArgument(t *testing.T) { + cases := []struct { + name, command string + want []string + }{ + {"an escaped digit before the end", "/opt/verify \\1", []string{"/opt/verify", "1"}}, + {"an escaped space as the whole argument", "/opt/verify \\ ", []string{"/opt/verify", " "}}, + {"an escaped character outside quotes inside a word", "/opt/a\\b", []string{"/opt/ab"}}, + {"an escaped double quote inside double quotes", "/opt/\"a\\\"b\"", []string{"/opt/a\"b"}}, + {"an empty quoted argument survives", "/opt/verify \"\"", []string{"/opt/verify", ""}}, + {"an escaped character followed by a word", "/opt/verify \\1 two", []string{"/opt/verify", "1", "two"}}, + } + for _, c := range cases { + got, err := splitCommand(c.command) + if err != nil || !reflect.DeepEqual(got, c.want) { + t.Errorf("%s: splitCommand(%q) = %q (err %v); want %q", c.name, c.command, got, err, c.want) + } + } +} + +// A trailing escape has nothing to escape and is refused, as before. +func TestSplitCommandRefusesATrailingEscape(t *testing.T) { + if _, err := splitCommand("/opt/verify \\"); err == nil { + t.Fatal("a trailing escape should be refused") + } +} diff --git a/internal/relay/integrate/livestate_test.go b/internal/relay/integrate/livestate_test.go new file mode 100644 index 0000000000..98c96f5054 --- /dev/null +++ b/internal/relay/integrate/livestate_test.go @@ -0,0 +1,7 @@ +package integrate + +// Linking testsupport sets CRW_REFUSE_LIVE_STATE=1 for this test binary and every process it +// starts, so no test here opens a store under a live relay state directory (decisions.md 46). +// internal/testsupport's livestate_test.go holds this package to the rule because it links +// internal/relay/store. +import _ "github.com/thisisjun786/codex-relay-workflow/internal/testsupport" diff --git a/internal/relay/mergeturn/target.go b/internal/relay/mergeturn/target.go index 92bbbc18cb..d14375bef7 100644 --- a/internal/relay/mergeturn/target.go +++ b/internal/relay/mergeturn/target.go @@ -192,11 +192,39 @@ func gitDirOf(repository string) string { if resolved, err := filepath.EvalSymlinks(repository); err == nil { repository = resolved } - gitdir := filepath.Join(repository, ".git") - if _, err := os.Stat(gitdir); os.IsNotExist(err) { + dotGit := filepath.Join(repository, ".git") + info, err := os.Stat(dotGit) + if os.IsNotExist(err) { return repository } - return gitdir + if err != nil || info.IsDir() { + return dotGit + } + return linkedWorktreeGitDir(repository, dotGit) +} + +// linkedWorktreeGitDir is the git directory a linked worktree's .git file names (CRW-965: its .git is a file, not +// a directory). It runs git with every GIT_ variable removed, so a poisoned environment cannot name another +// repository, and bounds the call so a stuck git cannot hang the lane. A failed or empty answer keeps the .git path. +func linkedWorktreeGitDir(repository, dotGit string) string { + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + env := make([]string, 0) + for _, v := range os.Environ() { + if !strings.HasPrefix(v, "GIT_") { + env = append(env, v) + } + } + cmd := exec.CommandContext(ctx, "git", "-C", repository, "rev-parse", "--absolute-git-dir") + cmd.Env = append(env, "GIT_TERMINAL_PROMPT=0", "LC_ALL=C") + out, err := cmd.Output() + if err != nil { + return dotGit + } + if dir := strings.TrimSpace(string(out)); dir != "" { + return dir + } + return dotGit } // repositoryKeyTimeout bounds the one git call that names a local repository. A repository that does not answer diff --git a/internal/relay/store/dag_integration_stages.go b/internal/relay/store/dag_integration_stages.go new file mode 100644 index 0000000000..6df37a65a2 --- /dev/null +++ b/internal/relay/store/dag_integration_stages.go @@ -0,0 +1,49 @@ +package store + +import "context" + +// IntegrationStageRow is one dag_integration_stages row (CRW-965). A batch writes its stages append-only: the +// intent (the frozen candidates, the base and the old head, written before any git call), ref_moved (the branch +// moved to the verified commit), marked (a node's merged mark was written from its frozen acceptance) and +// mark_pending (a mark that could not be written, completed later by the next run). Batch-level stages carry an +// empty node and acceptance. +type IntegrationStageRow struct { + StageID string + BatchID string + PlanID string + Stage string + NodeID string + AcceptanceID string + EventID string + RevisionHash string + Generation int64 + HeadSHA string + Detail string + RecordedBy string + RecordedAt string +} + +const integrationStageColumns = "stage_id, batch_id, plan_id, stage, node_id, acceptance_id, event_id, revision_hash, generation, head_sha, detail, recorded_by, recorded_at" + +func scanIntegrationStage(row scanner) (IntegrationStageRow, error) { + var r IntegrationStageRow + err := row.Scan(&r.StageID, &r.BatchID, &r.PlanID, &r.Stage, &r.NodeID, &r.AcceptanceID, &r.EventID, &r.RevisionHash, &r.Generation, &r.HeadSHA, &r.Detail, &r.RecordedBy, &r.RecordedAt) + return r, err +} + +// RecordIntegrationStage appends one integration stage row. +func RecordIntegrationStage(ctx context.Context, s *Store, r IntegrationStageRow) error { + _, err := s.exec(ctx, "INSERT INTO dag_integration_stages ("+integrationStageColumns+") VALUES (?,?,?,?,?,?,?,?,?,?,?,?,?)", + r.StageID, r.BatchID, r.PlanID, r.Stage, r.NodeID, r.AcceptanceID, r.EventID, r.RevisionHash, r.Generation, r.HeadSHA, r.Detail, r.RecordedBy, r.RecordedAt) + return err +} + +// IntegrationStagesOfPlan reads every stage row of a plan in the order they were written. A store that predates +// the table holds none. +func IntegrationStagesOfPlan(ctx context.Context, s *Store, plan string) ([]IntegrationStageRow, error) { + present, err := dagZoneTable(ctx, s, "dag_integration_stages") + if err != nil || !present { + return nil, err + } + return queryRows(ctx, s, scanIntegrationStage, "SELECT "+integrationStageColumns+" FROM dag_integration_stages WHERE plan_id = ? ORDER BY rowid", plan) +} diff --git a/internal/relay/store/dag_zone.go b/internal/relay/store/dag_zone.go index a57c83df78..b70b3f4469 100644 --- a/internal/relay/store/dag_zone.go +++ b/internal/relay/store/dag_zone.go @@ -846,4 +846,72 @@ BEGIN SELECT RAISE(ABORT, 'merge_trains.train_id is NULL: a train is addressed b expired_reason TEXT NOT NULL )`, `CREATE INDEX IF NOT EXISTS dag_user_decisions_fingerprint ON dag_user_decisions (fingerprint)`, + + // CRW-965: the pull-request-less acceptance and integration path. Two appended tables, because a + // shipped statement is never edited: the verification record an acceptance was taken on (the body + // itself, so a repeat call can prove it is the same output and the batch can reuse the record for a + // tree it already verified), and one row per integration batch that moved the local integration + // branch. Both are append-only. + `CREATE TABLE IF NOT EXISTS dag_acceptance_verifications ( + acceptance_id TEXT PRIMARY KEY REFERENCES dag_acceptances (acceptance_id), + record_digest TEXT NOT NULL CHECK (record_digest <> ''), + record_json TEXT NOT NULL CHECK (record_json <> ''), + head_commit TEXT NOT NULL CHECK (head_commit <> ''), + tree_sha TEXT NOT NULL CHECK (tree_sha <> ''), + base_commit TEXT NOT NULL CHECK (base_commit <> ''), + recorded_by TEXT NOT NULL CHECK (recorded_by <> ''), + coordinator_epoch INTEGER NOT NULL CHECK (coordinator_epoch >= 0), + recorded_at TEXT NOT NULL +)`, + `CREATE TRIGGER IF NOT EXISTS dag_acceptance_verifications_acceptance_id_not_null BEFORE INSERT ON dag_acceptance_verifications +WHEN NEW.acceptance_id IS NULL +BEGIN SELECT RAISE(ABORT, 'dag_acceptance_verifications.acceptance_id is NULL: a row is addressed by a non-empty id'); END`, + `CREATE TRIGGER IF NOT EXISTS dag_acceptance_verifications_no_update BEFORE UPDATE ON dag_acceptance_verifications +BEGIN SELECT RAISE(ABORT, 'dag_acceptance_verifications rows are append-only: never updated'); END`, + `CREATE TRIGGER IF NOT EXISTS dag_acceptance_verifications_no_delete BEFORE DELETE ON dag_acceptance_verifications +BEGIN SELECT RAISE(ABORT, 'dag_acceptance_verifications rows are append-only: never deleted'); END`, + `CREATE TABLE IF NOT EXISTS dag_integration_batches ( + batch_id TEXT PRIMARY KEY CHECK (batch_id <> ''), + plan_id TEXT NOT NULL REFERENCES dag_plans (plan_id), + repository TEXT NOT NULL CHECK (repository <> ''), + integration_ref TEXT NOT NULL CHECK (integration_ref <> ''), + base_ref TEXT NOT NULL CHECK (base_ref <> ''), + old_head TEXT NOT NULL, + new_head TEXT NOT NULL CHECK (new_head <> ''), + merged_json TEXT NOT NULL, + split_json TEXT NOT NULL, + verification_json TEXT NOT NULL, + recorded_by TEXT NOT NULL CHECK (recorded_by <> ''), + coordinator_epoch INTEGER NOT NULL CHECK (coordinator_epoch >= 0), + recorded_at TEXT NOT NULL +)`, + `CREATE TRIGGER IF NOT EXISTS dag_integration_batches_batch_id_not_null BEFORE INSERT ON dag_integration_batches +WHEN NEW.batch_id IS NULL +BEGIN SELECT RAISE(ABORT, 'dag_integration_batches.batch_id is NULL: a row is addressed by a non-empty id'); END`, + `CREATE TRIGGER IF NOT EXISTS dag_integration_batches_no_update BEFORE UPDATE ON dag_integration_batches +BEGIN SELECT RAISE(ABORT, 'dag_integration_batches rows are append-only: never updated'); END`, + `CREATE TRIGGER IF NOT EXISTS dag_integration_batches_no_delete BEFORE DELETE ON dag_integration_batches +BEGIN SELECT RAISE(ABORT, 'dag_integration_batches rows are append-only: never deleted'); END`, + `CREATE TABLE IF NOT EXISTS dag_integration_stages ( + stage_id TEXT PRIMARY KEY CHECK (stage_id <> ''), + batch_id TEXT NOT NULL CHECK (batch_id <> ''), + plan_id TEXT NOT NULL REFERENCES dag_plans (plan_id), + stage TEXT NOT NULL CHECK (stage IN ('intent', 'ref_moved', 'marked', 'mark_pending')), + node_id TEXT NOT NULL, + acceptance_id TEXT NOT NULL, + event_id TEXT NOT NULL, + revision_hash TEXT NOT NULL, + generation INTEGER NOT NULL CHECK (generation >= 0), + head_sha TEXT NOT NULL, + detail TEXT NOT NULL, + recorded_by TEXT NOT NULL CHECK (recorded_by <> ''), + recorded_at TEXT NOT NULL +)`, + `CREATE TRIGGER IF NOT EXISTS dag_integration_stages_stage_id_not_null BEFORE INSERT ON dag_integration_stages +WHEN NEW.stage_id IS NULL +BEGIN SELECT RAISE(ABORT, 'dag_integration_stages.stage_id is NULL: a row is addressed by a non-empty id'); END`, + `CREATE TRIGGER IF NOT EXISTS dag_integration_stages_no_update BEFORE UPDATE ON dag_integration_stages +BEGIN SELECT RAISE(ABORT, 'dag_integration_stages rows are append-only: never updated'); END`, + `CREATE TRIGGER IF NOT EXISTS dag_integration_stages_no_delete BEFORE DELETE ON dag_integration_stages +BEGIN SELECT RAISE(ABORT, 'dag_integration_stages rows are append-only: never deleted'); END`, } diff --git a/internal/relay/store/dag_zone_null_key_test.go b/internal/relay/store/dag_zone_null_key_test.go new file mode 100644 index 0000000000..3441a762b6 --- /dev/null +++ b/internal/relay/store/dag_zone_null_key_test.go @@ -0,0 +1,25 @@ +package store + +import ( + "strings" + "testing" +) + +// CRW-965 review: a SQLite rowid table's TEXT PRIMARY KEY admits NULL, and a CHECK on the empty string lets it through. +// The three appended key columns of the commit path are guarded by a BEFORE INSERT trigger, as the merge train's id is. +// A NULL key is refused by that trigger's own message, not by a generic constraint. +func TestDAGZoneRefusesANullKeyOfTheCommitPath(t *testing.T) { + path := zonePreDAGStore(t) + zoneOpenClose(t, path) + db := zoneRawDB(t, path) + for _, insert := range []string{ + "INSERT INTO dag_acceptance_verifications (acceptance_id) VALUES (NULL)", + "INSERT INTO dag_integration_batches (batch_id) VALUES (NULL)", + "INSERT INTO dag_integration_stages (stage_id) VALUES (NULL)", + } { + err := zoneExec(t, db, insert) + if err == nil || !strings.Contains(err.Error(), "is NULL") { + t.Fatalf("%s: got %v; want the trigger's refusal of a NULL key", insert, err) + } + } +} diff --git a/internal/relay/store/dag_zone_test.go b/internal/relay/store/dag_zone_test.go index 809e21a374..493710692a 100644 --- a/internal/relay/store/dag_zone_test.go +++ b/internal/relay/store/dag_zone_test.go @@ -78,6 +78,10 @@ var zoneInventory = map[string][]string{ // CRW-736 (appended statement): the user-decision record, one column per field of crw-user-decision/1 // with the object and array fields as JSON text. "dag_user_decisions": {"decision_id", "fingerprint", "kind", "context", "options_json", "recommendation_json", "blocking_json", "needed_by", "origin_json", "source_json", "authority_json", "state", "raised_at", "raised_via", "seen_json", "answered_at", "answered_by", "answered_via", "answer_text", "applied_at", "applied_event", "applied_generation", "withdrawn_reason", "expired_reason"}, + // CRW-965 (appended statements): the verification record an acceptance was taken on, and one row per integration batch. + "dag_acceptance_verifications": {"acceptance_id", "record_digest", "record_json", "head_commit", "tree_sha", "base_commit", "recorded_by", "coordinator_epoch", "recorded_at"}, + "dag_integration_batches": {"batch_id", "plan_id", "repository", "integration_ref", "base_ref", "old_head", "new_head", "merged_json", "split_json", "verification_json", "recorded_by", "coordinator_epoch", "recorded_at"}, + "dag_integration_stages": {"stage_id", "batch_id", "plan_id", "stage", "node_id", "acceptance_id", "event_id", "revision_hash", "generation", "head_sha", "detail", "recorded_by", "recorded_at"}, } // rawDB opens path without any of the store's open rules, as an operator's sqlite3 would. diff --git a/internal/relay/store/domain_rows.go b/internal/relay/store/domain_rows.go index 0910ef9520..b84bd2be32 100644 --- a/internal/relay/store/domain_rows.go +++ b/internal/relay/store/domain_rows.go @@ -1022,3 +1022,126 @@ func VerifiedHead(ctx context.Context, s *Store, eventID string) (VerifiedHeadRo } return row, true, nil } + +// AcceptanceVerificationRow is one dag_acceptance_verifications row, every column in DDL order +// (CRW-965): the verification record a pull-request-less acceptance was taken on. +type AcceptanceVerificationRow struct { + AcceptanceID string + RecordDigest string + RecordJSON string + HeadCommit string + TreeSHA string + BaseCommit string + RecordedBy string + CoordinatorEpoch int64 + RecordedAt string +} + +const acceptanceVerificationColumns = "acceptance_id, record_digest, record_json, head_commit, tree_sha, base_commit, recorded_by, coordinator_epoch, recorded_at" + +func scanAcceptanceVerification(row scanner) (AcceptanceVerificationRow, error) { + var r AcceptanceVerificationRow + err := row.Scan(&r.AcceptanceID, &r.RecordDigest, &r.RecordJSON, &r.HeadCommit, &r.TreeSHA, &r.BaseCommit, &r.RecordedBy, &r.CoordinatorEpoch, &r.RecordedAt) + return r, err +} + +// RecordAcceptanceVerification appends the verification record of a pull-request-less acceptance. One +// acceptance holds one row: a second insert is the table's PRIMARY KEY refusal, which the caller reads +// as the replay it is. +func RecordAcceptanceVerification(ctx context.Context, s *Store, r AcceptanceVerificationRow) error { + _, err := s.exec(ctx, "INSERT INTO dag_acceptance_verifications ("+acceptanceVerificationColumns+") VALUES (?,?,?,?,?,?,?,?,?)", + r.AcceptanceID, r.RecordDigest, r.RecordJSON, r.HeadCommit, r.TreeSHA, r.BaseCommit, r.RecordedBy, r.CoordinatorEpoch, r.RecordedAt) + return err +} + +// AcceptanceVerification reads the verification record an acceptance was taken on. found is false when +// the acceptance has none, or when the store predates the table: neither is an error. +func AcceptanceVerification(ctx context.Context, s *Store, acceptanceID string) (AcceptanceVerificationRow, bool, error) { + present, err := dagZoneTable(ctx, s, "dag_acceptance_verifications") + if err != nil || !present { + return AcceptanceVerificationRow{}, false, err + } + row, err := queryRow(ctx, s, scanAcceptanceVerification, "SELECT "+acceptanceVerificationColumns+" FROM dag_acceptance_verifications WHERE acceptance_id = ?", acceptanceID) + if errors.Is(err, sql.ErrNoRows) { + return AcceptanceVerificationRow{}, false, nil + } + if err != nil { + return AcceptanceVerificationRow{}, false, err + } + return row, true, nil +} + +// IntegrationBatchRow is one dag_integration_batches row, every column in DDL order (CRW-965): one +// integration batch that moved the local integration branch. +type IntegrationBatchRow struct { + BatchID string + PlanID string + Repository string + IntegrationRef string + BaseRef string + OldHead string + NewHead string + MergedJSON string + SplitJSON string + VerificationJSON string + RecordedBy string + CoordinatorEpoch int64 + RecordedAt string +} + +const integrationBatchColumns = "batch_id, plan_id, repository, integration_ref, base_ref, old_head, new_head, merged_json, split_json, verification_json, recorded_by, coordinator_epoch, recorded_at" + +func scanIntegrationBatch(row scanner) (IntegrationBatchRow, error) { + var r IntegrationBatchRow + err := row.Scan(&r.BatchID, &r.PlanID, &r.Repository, &r.IntegrationRef, &r.BaseRef, &r.OldHead, &r.NewHead, &r.MergedJSON, &r.SplitJSON, + &r.VerificationJSON, &r.RecordedBy, &r.CoordinatorEpoch, &r.RecordedAt) + return r, err +} + +// RecordIntegrationBatch appends one integration batch row. +func RecordIntegrationBatch(ctx context.Context, s *Store, r IntegrationBatchRow) error { + _, err := s.exec(ctx, "INSERT INTO dag_integration_batches ("+integrationBatchColumns+") VALUES (?,?,?,?,?,?,?,?,?,?,?,?,?)", + r.BatchID, r.PlanID, r.Repository, r.IntegrationRef, r.BaseRef, r.OldHead, r.NewHead, r.MergedJSON, r.SplitJSON, r.VerificationJSON, + r.RecordedBy, r.CoordinatorEpoch, r.RecordedAt) + return err +} + +// IntegrationBatch reads the batch recorded under an id. found is false when the store holds none, or +// when it predates the table. +func IntegrationBatch(ctx context.Context, s *Store, batchID string) (IntegrationBatchRow, bool, error) { + present, err := dagZoneTable(ctx, s, "dag_integration_batches") + if err != nil || !present { + return IntegrationBatchRow{}, false, err + } + row, err := queryRow(ctx, s, scanIntegrationBatch, "SELECT "+integrationBatchColumns+" FROM dag_integration_batches WHERE batch_id = ?", batchID) + if errors.Is(err, sql.ErrNoRows) { + return IntegrationBatchRow{}, false, nil + } + if err != nil { + return IntegrationBatchRow{}, false, err + } + return row, true, nil +} + +// IntegrationBatchesOfPlan lists a plan's integration batches, newest first (CRW-965): the record a +// reader consults to see which trees the local integration branch has already been verified at. +func IntegrationBatchesOfPlan(ctx context.Context, s *Store, planID string) ([]IntegrationBatchRow, error) { + present, err := dagZoneTable(ctx, s, "dag_integration_batches") + if err != nil || !present { + return nil, err + } + rows, err := s.Querier(ctx).QueryContext(ctx, "SELECT "+integrationBatchColumns+" FROM dag_integration_batches WHERE plan_id = ? ORDER BY recorded_at DESC, batch_id DESC", planID) + if err != nil { + return nil, err + } + defer rows.Close() + var out []IntegrationBatchRow + for rows.Next() { + row, err := scanIntegrationBatch(rows) + if err != nil { + return nil, err + } + out = append(out, row) + } + return out, rows.Err() +} diff --git a/internal/relay/store/testdata/dag_zone_shipped.json b/internal/relay/store/testdata/dag_zone_shipped.json index 5573935e2d..1bf810f6bd 100644 --- a/internal/relay/store/testdata/dag_zone_shipped.json +++ b/internal/relay/store/testdata/dag_zone_shipped.json @@ -15,6 +15,7 @@ "table dag_acceptance_forge": "CREATE TABLE dag_acceptance_forge ( acceptance_id TEXT PRIMARY KEY REFERENCES dag_acceptances (acceptance_id), forge_repository TEXT NOT NULL CHECK (forge_repository \u003c\u003e''), pr_number INTEGER NOT NULL CHECK (pr_number \u003e= 1) )", "table dag_acceptance_refreshes": "CREATE TABLE dag_acceptance_refreshes ( refresh_id TEXT PRIMARY KEY CHECK (refresh_id \u003c\u003e''), acceptance_id TEXT NOT NULL REFERENCES dag_acceptances (acceptance_id), refresh_seq INTEGER NOT NULL CHECK (refresh_seq \u003e= 1), relationship_id TEXT NOT NULL CHECK (relationship_id \u003c\u003e''), execution_generation INTEGER NOT NULL CHECK (execution_generation \u003e= 1), event_id TEXT NOT NULL CHECK (event_id \u003c\u003e''), revision_hash TEXT NOT NULL CHECK (revision_hash \u003c\u003e''), head_sha TEXT NOT NULL CHECK (head_sha \u003c\u003e''), verified_head_sha TEXT NOT NULL CHECK (verified_head_sha \u003c\u003e''), base_repository TEXT NOT NULL CHECK (base_repository \u003c\u003e''), base_ref TEXT NOT NULL CHECK (base_ref \u003c\u003e''), base_tip_sha TEXT NOT NULL CHECK (base_tip_sha \u003c\u003e''), proof_json TEXT NOT NULL CHECK (proof_json \u003c\u003e''), resolved_paths_json TEXT NOT NULL, recorded_by_task_id TEXT NOT NULL, coordinator_epoch INTEGER NOT NULL CHECK (coordinator_epoch \u003e= 0), recorded_at TEXT NOT NULL, UNIQUE (acceptance_id, refresh_seq), UNIQUE (acceptance_id, head_sha) )", "table dag_acceptance_revalidations": "CREATE TABLE dag_acceptance_revalidations ( revalidation_id TEXT PRIMARY KEY, acceptance_id TEXT NOT NULL REFERENCES dag_acceptances (acceptance_id), criteria_set_digest TEXT NOT NULL, event_id TEXT NOT NULL, verdict_turn_id TEXT NOT NULL, reval_seq INTEGER NOT NULL CHECK (reval_seq \u003e= 1), revalidated_by TEXT NOT NULL, revalidated_at TEXT NOT NULL, UNIQUE (acceptance_id, reval_seq) )", + "table dag_acceptance_verifications": "CREATE TABLE dag_acceptance_verifications ( acceptance_id TEXT PRIMARY KEY REFERENCES dag_acceptances (acceptance_id), record_digest TEXT NOT NULL CHECK (record_digest \u003c\u003e''), record_json TEXT NOT NULL CHECK (record_json \u003c\u003e''), head_commit TEXT NOT NULL CHECK (head_commit \u003c\u003e''), tree_sha TEXT NOT NULL CHECK (tree_sha \u003c\u003e''), base_commit TEXT NOT NULL CHECK (base_commit \u003c\u003e''), recorded_by TEXT NOT NULL CHECK (recorded_by \u003c\u003e''), coordinator_epoch INTEGER NOT NULL CHECK (coordinator_epoch \u003e= 0), recorded_at TEXT NOT NULL )", "table dag_acceptances": "CREATE TABLE dag_acceptances ( acceptance_id TEXT PRIMARY KEY, plan_id TEXT NOT NULL, node_id TEXT NOT NULL, manifest_digest TEXT NOT NULL, relationship_id TEXT NOT NULL, execution_generation INTEGER NOT NULL, event_id TEXT NOT NULL, revision_hash TEXT NOT NULL, criteria_set_digest TEXT NOT NULL, verdict TEXT NOT NULL CHECK (verdict ='verified'), head_sha TEXT, repository TEXT, pr_number INTEGER, output_manifest_ref TEXT, evidence_digest TEXT, ack_tier TEXT NOT NULL CHECK (ack_tier \u003c\u003e'unverified'), verdict_turn_id TEXT NOT NULL, rule_version_json TEXT NOT NULL, accepted_by_task_id TEXT NOT NULL, coordinator_epoch INTEGER NOT NULL CHECK (coordinator_epoch \u003e= 0), accepted_at TEXT NOT NULL, supersedes_acceptance_id TEXT, state TEXT NOT NULL CHECK (state IN ('active','superseded','revoked')) )", "table dag_base_refreshes": "CREATE TABLE dag_base_refreshes ( refresh_id TEXT PRIMARY KEY CHECK (refresh_id \u003c\u003e''), acceptance_id TEXT NOT NULL REFERENCES dag_acceptances (acceptance_id), refresh_seq INTEGER NOT NULL CHECK (refresh_seq \u003e= 1), relationship_id TEXT NOT NULL CHECK (relationship_id \u003c\u003e''), execution_generation INTEGER NOT NULL CHECK (execution_generation \u003e= 1), event_id TEXT NOT NULL CHECK (event_id \u003c\u003e''), revision_hash TEXT NOT NULL CHECK (revision_hash \u003c\u003e''), head_sha TEXT NOT NULL CHECK (head_sha \u003c\u003e''), base_repository TEXT NOT NULL CHECK (base_repository \u003c\u003e''), base_ref TEXT NOT NULL CHECK (base_ref \u003c\u003e''), base_tip_sha TEXT NOT NULL CHECK (base_tip_sha \u003c\u003e''), proof_json TEXT NOT NULL CHECK (proof_json \u003c\u003e''), resolved_paths_json TEXT NOT NULL, recorded_by_task_id TEXT NOT NULL, coordinator_epoch INTEGER NOT NULL CHECK (coordinator_epoch \u003e= 0), recorded_at TEXT NOT NULL, UNIQUE (acceptance_id, refresh_seq), UNIQUE (acceptance_id, event_id, head_sha) )", "table dag_cap_basis": "CREATE TABLE dag_cap_basis ( limit_id TEXT NOT NULL, limit_revision INTEGER NOT NULL, w_minutes REAL NOT NULL, w_source TEXT NOT NULL, s_minutes REAL NOT NULL, s_source TEXT NOT NULL, decided_by TEXT NOT NULL, decided_at TEXT NOT NULL, PRIMARY KEY (limit_id, limit_revision) )", @@ -28,7 +29,9 @@ "table dag_edges": "CREATE TABLE dag_edges ( plan_id TEXT NOT NULL REFERENCES dag_plans (plan_id), edge_id TEXT NOT NULL, introduced_rev INTEGER NOT NULL, retired_rev INTEGER, from_node_id TEXT NOT NULL, to_node_id TEXT NOT NULL, kind TEXT NOT NULL CHECK (kind IN ('artifact_verified','integrated','decision')), target_repository TEXT, target_base_ref TEXT, pins_code_head INTEGER NOT NULL DEFAULT 0 CHECK (pins_code_head IN (0, 1)), decision_subject TEXT, decision_digest TEXT, required_authority TEXT, PRIMARY KEY (plan_id, edge_id), FOREIGN KEY (plan_id, introduced_rev) REFERENCES dag_plan_revisions (plan_id, revision_no), FOREIGN KEY (plan_id, retired_rev) REFERENCES dag_plan_revisions (plan_id, revision_no), CHECK (retired_rev IS NULL OR retired_rev \u003e introduced_rev), CHECK (from_node_id \u003c\u003e to_node_id), CHECK (kind \u003c\u003e'integrated' OR (target_repository IS NOT NULL AND target_base_ref IS NOT NULL)), CHECK (pins_code_head = 0 OR (target_repository IS NOT NULL AND target_base_ref IS NOT NULL)), CHECK (kind \u003c\u003e'decision' OR (decision_subject IS NOT NULL AND decision_digest IS NOT NULL AND required_authority IS NOT NULL AND required_authority \u003c\u003e'[]')) )", "table dag_generation_withdrawals": "CREATE TABLE dag_generation_withdrawals ( relationship_id TEXT NOT NULL CHECK (relationship_id \u003c\u003e''), execution_generation INTEGER NOT NULL CHECK (execution_generation \u003e= 2), plan_id TEXT NOT NULL REFERENCES dag_plans (plan_id), node_id TEXT NOT NULL CHECK (node_id \u003c\u003e''), dispatch_request_id TEXT NOT NULL CHECK (dispatch_request_id \u003c\u003e''), opened_reason TEXT NOT NULL, restored_generation INTEGER NOT NULL CHECK (restored_generation \u003e= 1), reason TEXT NOT NULL CHECK (reason \u003c\u003e''), withdrawn_by_task_id TEXT NOT NULL CHECK (withdrawn_by_task_id \u003c\u003e''), coordinator_epoch INTEGER NOT NULL DEFAULT 0 CHECK (coordinator_epoch \u003e= 0), withdrawn_at TEXT NOT NULL, PRIMARY KEY (relationship_id, execution_generation), CHECK (restored_generation \u003c execution_generation) )", "table dag_input_manifests": "CREATE TABLE dag_input_manifests ( manifest_digest TEXT PRIMARY KEY, node_id TEXT NOT NULL, body_json TEXT NOT NULL, rule_version_json TEXT NOT NULL, coordinator_epoch INTEGER NOT NULL DEFAULT 0 CHECK (coordinator_epoch \u003e= 0), created_at TEXT NOT NULL )", + "table dag_integration_batches": "CREATE TABLE dag_integration_batches ( batch_id TEXT PRIMARY KEY CHECK (batch_id \u003c\u003e''), plan_id TEXT NOT NULL REFERENCES dag_plans (plan_id), repository TEXT NOT NULL CHECK (repository \u003c\u003e''), integration_ref TEXT NOT NULL CHECK (integration_ref \u003c\u003e''), base_ref TEXT NOT NULL CHECK (base_ref \u003c\u003e''), old_head TEXT NOT NULL, new_head TEXT NOT NULL CHECK (new_head \u003c\u003e''), merged_json TEXT NOT NULL, split_json TEXT NOT NULL, verification_json TEXT NOT NULL, recorded_by TEXT NOT NULL CHECK (recorded_by \u003c\u003e''), coordinator_epoch INTEGER NOT NULL CHECK (coordinator_epoch \u003e= 0), recorded_at TEXT NOT NULL )", "table dag_integration_observations": "CREATE TABLE dag_integration_observations ( observation_id TEXT PRIMARY KEY, acceptance_id TEXT NOT NULL, repository TEXT NOT NULL, base_ref TEXT NOT NULL, subject_sha TEXT NOT NULL, tip_sha TEXT NOT NULL, is_ancestor INTEGER NOT NULL CHECK (is_ancestor IN (0, 1)), method TEXT NOT NULL, merge_turn_id TEXT, observed_seq INTEGER NOT NULL, reverted_by TEXT, observed_at TEXT NOT NULL, UNIQUE (acceptance_id, repository, base_ref, observed_seq) )", + "table dag_integration_stages": "CREATE TABLE dag_integration_stages ( stage_id TEXT PRIMARY KEY CHECK (stage_id \u003c\u003e''), batch_id TEXT NOT NULL CHECK (batch_id \u003c\u003e''), plan_id TEXT NOT NULL REFERENCES dag_plans (plan_id), stage TEXT NOT NULL CHECK (stage IN ('intent','ref_moved','marked','mark_pending')), node_id TEXT NOT NULL, acceptance_id TEXT NOT NULL, event_id TEXT NOT NULL, revision_hash TEXT NOT NULL, generation INTEGER NOT NULL CHECK (generation \u003e= 0), head_sha TEXT NOT NULL, detail TEXT NOT NULL, recorded_by TEXT NOT NULL CHECK (recorded_by \u003c\u003e''), recorded_at TEXT NOT NULL )", "table dag_landing_results": "CREATE TABLE dag_landing_results ( result_id TEXT PRIMARY KEY, plan_id TEXT NOT NULL REFERENCES dag_plans (plan_id), node_id TEXT NOT NULL CHECK (node_id \u003c\u003e''), kind TEXT NOT NULL CHECK (kind IN ('dev_green','dev_red','reverted','duplicate','discarded')), commit_sha TEXT NOT NULL DEFAULT'', evidence TEXT NOT NULL CHECK (evidence \u003c\u003e''), recorded_by TEXT NOT NULL CHECK (recorded_by \u003c\u003e''), coordinator_epoch INTEGER NOT NULL DEFAULT 0 CHECK (coordinator_epoch \u003e= 0), recorded_at TEXT NOT NULL )", "table dag_merge_checks": "CREATE TABLE dag_merge_checks ( check_id TEXT PRIMARY KEY, acceptance_id TEXT NOT NULL REFERENCES dag_acceptances (acceptance_id), check_seq INTEGER NOT NULL CHECK (check_seq \u003e= 1), head_sha TEXT NOT NULL, observed_head_sha TEXT NOT NULL, base_tip_sha TEXT NOT NULL, checks_base_sha TEXT, checks_digest TEXT NOT NULL, evidence_json TEXT NOT NULL, failed_required_json TEXT NOT NULL, round_no INTEGER NOT NULL CHECK (round_no BETWEEN 1 AND 2), outcome TEXT NOT NULL CHECK (outcome IN ('eligible','retry_same_sha','evicted','checks_pending','stale_head','stale_base','stale_criteria','predecessor_not_landed')), reason TEXT NOT NULL, recorded_at TEXT NOT NULL, UNIQUE (acceptance_id, check_seq) )", "table dag_node_executions": "CREATE TABLE dag_node_executions ( plan_id TEXT NOT NULL, node_id TEXT NOT NULL, relationship_id TEXT NOT NULL, execution_generation INTEGER NOT NULL, manifest_digest TEXT NOT NULL, kind TEXT NOT NULL CHECK (kind IN ('initial','correction','redefinition','parent_handover','child_replacement')), managed_request_id TEXT, PRIMARY KEY (relationship_id, execution_generation) )", @@ -55,12 +58,21 @@ "table merge_trains": "CREATE TABLE merge_trains ( train_id TEXT PRIMARY KEY CHECK (train_id \u003c\u003e''), target_key TEXT NOT NULL CHECK (target_key \u003c\u003e''), repository TEXT NOT NULL CHECK (repository \u003c\u003e''), base_ref TEXT NOT NULL CHECK (base_ref \u003c\u003e''), base_sha TEXT NOT NULL CHECK (base_sha \u003c\u003e''), leader_task_id TEXT NOT NULL CHECK (leader_task_id \u003c\u003e''), created_at TEXT NOT NULL )", "trigger dag_acceptance_refreshes_no_delete": "CREATE TRIGGER dag_acceptance_refreshes_no_delete BEFORE DELETE ON dag_acceptance_refreshes BEGIN SELECT RAISE(ABORT,'dag_acceptance_refreshes rows are append-only: never deleted'); END", "trigger dag_acceptance_refreshes_no_update": "CREATE TRIGGER dag_acceptance_refreshes_no_update BEFORE UPDATE ON dag_acceptance_refreshes BEGIN SELECT RAISE(ABORT,'dag_acceptance_refreshes rows are append-only: never updated'); END", + "trigger dag_acceptance_verifications_acceptance_id_not_null": "CREATE TRIGGER dag_acceptance_verifications_acceptance_id_not_null BEFORE INSERT ON dag_acceptance_verifications WHEN NEW.acceptance_id IS NULL BEGIN SELECT RAISE(ABORT,'dag_acceptance_verifications.acceptance_id is NULL: a row is addressed by a non-empty id'); END", + "trigger dag_acceptance_verifications_no_delete": "CREATE TRIGGER dag_acceptance_verifications_no_delete BEFORE DELETE ON dag_acceptance_verifications BEGIN SELECT RAISE(ABORT,'dag_acceptance_verifications rows are append-only: never deleted'); END", + "trigger dag_acceptance_verifications_no_update": "CREATE TRIGGER dag_acceptance_verifications_no_update BEFORE UPDATE ON dag_acceptance_verifications BEGIN SELECT RAISE(ABORT,'dag_acceptance_verifications rows are append-only: never updated'); END", "trigger dag_base_refreshes_no_delete": "CREATE TRIGGER dag_base_refreshes_no_delete BEFORE DELETE ON dag_base_refreshes BEGIN SELECT RAISE(ABORT,'dag_base_refreshes rows are append-only: never deleted'); END", "trigger dag_base_refreshes_no_update": "CREATE TRIGGER dag_base_refreshes_no_update BEFORE UPDATE ON dag_base_refreshes BEGIN SELECT RAISE(ABORT,'dag_base_refreshes rows are append-only: never updated'); END", "trigger dag_edges_no_delete": "CREATE TRIGGER dag_edges_no_delete BEFORE DELETE ON dag_edges BEGIN SELECT RAISE(ABORT,'dag_edges rows are never deleted'); END", "trigger dag_edges_retire_only": "CREATE TRIGGER dag_edges_retire_only BEFORE UPDATE ON dag_edges WHEN OLD.retired_rev IS NOT NULL OR NEW.retired_rev IS NULL OR NEW.plan_id IS NOT OLD.plan_id OR NEW.edge_id IS NOT OLD.edge_id OR NEW.introduced_rev IS NOT OLD.introduced_rev OR NEW.from_node_id IS NOT OLD.from_node_id OR NEW.to_node_id IS NOT OLD.to_node_id OR NEW.kind IS NOT OLD.kind OR NEW.target_repository IS NOT OLD.target_repository OR NEW.target_base_ref IS NOT OLD.target_base_ref OR NEW.pins_code_head IS NOT OLD.pins_code_head OR NEW.decision_subject IS NOT OLD.decision_subject OR NEW.decision_digest IS NOT OLD.decision_digest OR NEW.required_authority IS NOT OLD.required_authority BEGIN SELECT RAISE(ABORT,'dag_edges rows are immutable except for their one retirement'); END", "trigger dag_generation_withdrawals_no_delete": "CREATE TRIGGER dag_generation_withdrawals_no_delete BEFORE DELETE ON dag_generation_withdrawals BEGIN SELECT RAISE(ABORT,'dag_generation_withdrawals rows are append-only: never deleted'); END", "trigger dag_generation_withdrawals_no_update": "CREATE TRIGGER dag_generation_withdrawals_no_update BEFORE UPDATE ON dag_generation_withdrawals BEGIN SELECT RAISE(ABORT,'dag_generation_withdrawals rows are append-only: never updated'); END", + "trigger dag_integration_batches_batch_id_not_null": "CREATE TRIGGER dag_integration_batches_batch_id_not_null BEFORE INSERT ON dag_integration_batches WHEN NEW.batch_id IS NULL BEGIN SELECT RAISE(ABORT,'dag_integration_batches.batch_id is NULL: a row is addressed by a non-empty id'); END", + "trigger dag_integration_batches_no_delete": "CREATE TRIGGER dag_integration_batches_no_delete BEFORE DELETE ON dag_integration_batches BEGIN SELECT RAISE(ABORT,'dag_integration_batches rows are append-only: never deleted'); END", + "trigger dag_integration_batches_no_update": "CREATE TRIGGER dag_integration_batches_no_update BEFORE UPDATE ON dag_integration_batches BEGIN SELECT RAISE(ABORT,'dag_integration_batches rows are append-only: never updated'); END", + "trigger dag_integration_stages_no_delete": "CREATE TRIGGER dag_integration_stages_no_delete BEFORE DELETE ON dag_integration_stages BEGIN SELECT RAISE(ABORT,'dag_integration_stages rows are append-only: never deleted'); END", + "trigger dag_integration_stages_no_update": "CREATE TRIGGER dag_integration_stages_no_update BEFORE UPDATE ON dag_integration_stages BEGIN SELECT RAISE(ABORT,'dag_integration_stages rows are append-only: never updated'); END", + "trigger dag_integration_stages_stage_id_not_null": "CREATE TRIGGER dag_integration_stages_stage_id_not_null BEFORE INSERT ON dag_integration_stages WHEN NEW.stage_id IS NULL BEGIN SELECT RAISE(ABORT,'dag_integration_stages.stage_id is NULL: a row is addressed by a non-empty id'); END", "trigger dag_nodes_no_delete": "CREATE TRIGGER dag_nodes_no_delete BEFORE DELETE ON dag_nodes BEGIN SELECT RAISE(ABORT,'dag_nodes rows are never deleted'); END", "trigger dag_nodes_retire_only": "CREATE TRIGGER dag_nodes_retire_only BEFORE UPDATE ON dag_nodes WHEN OLD.retired_rev IS NOT NULL OR NEW.retired_rev IS NULL OR NEW.plan_id IS NOT OLD.plan_id OR NEW.node_id IS NOT OLD.node_id OR NEW.introduced_rev IS NOT OLD.introduced_rev OR NEW.slice_digest IS NOT OLD.slice_digest OR NEW.issue_key IS NOT OLD.issue_key OR NEW.node_kind IS NOT OLD.node_kind OR NEW.title IS NOT OLD.title OR NEW.criteria_set_digest IS NOT OLD.criteria_set_digest OR NEW.supersedes_node_id IS NOT OLD.supersedes_node_id BEGIN SELECT RAISE(ABORT,'dag_nodes rows are immutable except for their one retirement'); END", "trigger dag_plan_revisions_no_delete": "CREATE TRIGGER dag_plan_revisions_no_delete BEFORE DELETE ON dag_plan_revisions BEGIN SELECT RAISE(ABORT,'dag_plan_revisions rows are append-only: never deleted'); END", diff --git a/plugins/crw/.codex-plugin/plugin.json b/plugins/crw/.codex-plugin/plugin.json index 0a8d57a22a..b121f1f612 100644 --- a/plugins/crw/.codex-plugin/plugin.json +++ b/plugins/crw/.codex-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "crw", - "version": "0.4.0+f2a3e3855df6", + "version": "0.4.0+712166dbf1d3", "description": "Linear-driven workflow skills for Codex: define an initiative, plan projects and issues, run delivery, and check the result against its canonical criteria.", "author": { "name": "thisisjun786", diff --git a/plugins/crw/skills/crw-run/references/merge-readiness.md b/plugins/crw/skills/crw-run/references/merge-readiness.md index 7cac6ede04..46df8aedf8 100644 --- a/plugins/crw/skills/crw-run/references/merge-readiness.md +++ b/plugins/crw/skills/crw-run/references/merge-readiness.md @@ -453,6 +453,15 @@ other finding does; a warning with no such finding behind it is recorded in the goes on. Do not turn a warning into a gate by waiting for a rerun, a fix or a statement that only the warning asked for. +## Integrate a node by its commit (no pull request) + +A node built on the commit path has no pull request, so the merge gates above do not apply to it. Its acceptance is dag-accept with --commit, --base, --checkout and --verification (see docs/relay/dag-scheduler.md, "Commit acceptance and local integration"). The relay checks in that checkout that the commit is the receipt head of the node's generation, that it descends from the base, and that the verification record is reusable for this exact tree. + +The integration is one batch. dag-integrate merges the ready accepted candidates onto the local integration branch in a temporary worktree, verifies the merged tree with the --verify command (for example, a script whose first line is #!/bin/sh and that runs crw-dev ci local --commit "$CRW_VERIFY_HEAD" --base "$CRW_VERIFY_BASE" --record "$CRW_VERIFY_RECORD" --runner local), and moves the branch only if that verification passed. A candidate that breaks the merged tree, or that depends on one, is left out and returned to its parent with the reason; a candidate that only conflicted with a removed candidate integrates on the retry. Then dag-integrate-push fast-forwards the remote dev to the same commit. Never force it: a remote dev that holds a commit the integration branch does not contain is refused with merge_base_mismatch, and the parent resolves that divergence first. When the remote cannot be reached the push is deferred (outcome deferred); run dag-integrate-push again once it answers, because the batch and the merged marks already stand and are not repeated. A merged mark that could not be written is shown as mark_pending and completed by the next dag-integrate run. Observe the landing with dag-integration-observe --target on the integration branch. + +A batch leaves out a candidate that only breaks the set with others: it is reported as `not_proven_failing` and stays ready for the next batch. Only a candidate that fails when verified alone is reported as `verification_failed`. A candidate whose criteria changed after its acceptance is not ready until it is revalidated, and the relay does not re-verify an integration branch for a criteria change by itself. `dag-integrate-push` pushes only a tip the relay moved the branch onto (a verified batch row for that head); a commit added on top of it is refused with `merge_base_mismatch`, and the remote is not moved. + + ## Judge a finding by its impact Whether a finding blocks is decided by what it does to this change at its current