Repository navigation
CRW-906: record a hand-opened correction of an accepted, still-current result - #834
Merged
Merged
Conversation
…t result recordHandOpened refused every generation opened by hand for a node whose result the plan had already accepted and that is still current, so a blocking defect found in an accepted member before it landed could not be corrected or merged. The binder now admits that case when the reason the generation was opened under states a correction (needs_changes_revision, the generation-open default, or accepted_result_correction, which names this route), keeping every check it already had: the generation right after the one the acceptance stands on, the manifest prepared for the node's current slice and criteria, opened under CorrectionRequestID, bound to a dispatch turn, and the inputs as they stand now. The acceptance stays active until dag-accept --supersedes takes the new result. The route is noted in the generation's own reason column: no schema, refusal name, CLI flag or specs.json change.
|
You have reached your Codex usage limits for security reviews. Please try again later. |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 271802a307
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…oute table point A refusal that told the coordinator an accepted current result has no recorded correction route, and the normative route table that repeated it, are stale now that the route exists. The refusal names the steps and the reason that states the correction, and the route table points at the procedure. The ruling refusal's own test asserts the new text.
Owner
Author
|
CI on the current head Head Workflow run
Local gates on the same revision: The previous head |
…te gate The new reason is a correction reason like the default, not a route of its own: a stale node whose route says correct records under it, and one whose route is revalidate is still refused whatever reason it carries. Both cases are pinned so the admission cannot be read as a loosening of the gate.
…on is open The issue's decision 2 promises that a node under correction is not merged on its old head. The bundle open and the single lane refuse a pull request head that differs from the accepted head, and that guard holds only once the child has pushed: between opening (or recording) the correction generation and that first push the pull request still shows the accepted head H, so a bundle open, a bundle verify, a bundle land and the single-lane judgement all carried H. The merge gate now reads the relationship's live execution generation against the generation its active acceptance stands on (acceptance.UnderCorrection, whose two values are one statement). While the live one is later, the node is under correction and the head the acceptance stands on is refused disposition_conflict, naming the open generation: dag-merge-judge and dag-merge-request write no row and no turn, and trainStandRefusal (open, verify and land) refuses a bundle member. A correction accepted over with dag-accept --supersedes, or withdrawn, puts the node back in the lane, and a generation recorded by dag-base-refresh moves the stand generation with the live one, so a base refresh is not a correction. No refusal name, column, schema, CLI flag or specs.json entry is added.
…, and re-read the members inside the train's writes The pre-merge evaluation of this head found two more places the issue's decision 2 promise was still open, both about when the correction state is read rather than about the gate itself. A merge turn can have been granted before the correction generation was opened, and the lane's own commands are what actually carry the head to the base: merge-turn-check moves the turn to merging and merge-turn-land records the merge, and neither read the correction state. Both now ask acceptance.UnderCorrection inside the transaction that writes, so a generation opened while the check was reading the forge is seen; the refusal is the existing disposition_conflict and names the open generation, the check writes no current row and leaves the turn holding, and the land records no landing and leaves the turn merging. The bundle's open and verify read each member's acceptance before the transaction that writes, and a generation opened in that gap left the reading stale: open rechecked only the turns and the membership, and verify only the train state. Both now re-read every member's correction state on the transaction's own snapshot, so an under-correction member is neither opened nor verified. TrainLand already rechecked inside its transaction. The two refusals and the re-read live in one new file (mergeturn/undercorrection.go), so the lane and the bundle word the same answer. No refusal name, column, schema, CLI flag or specs.json entry is added.
…-result-correction # Conflicts: # internal/relay/mergeturn/train.go # plugins/crw/.codex-plugin/plugin.json
…eason, and resolve a lane turn's relationship from its pull request Two defects found in the generation-1 head, both inside this issue's promise. The record carried a reason the contract refused. The correction of an accepted, still-current result is recorded under accepted_result_correction, and Generation.contract emits it while Relationship.ContractRecord includes every generation, so a relationship opened by the documented route serialized a generations[].reason the shipped contract/schema/relationship.json did not allow. The enum gains that value; the 6 existing relationship cases were re-judged against the revised bytes with jsonschema 4.19.2 Draft7Validator and kept their verdicts, 6 new cases cover each reason the registry accepts, the empty reason, an unknown one and a mistyped one, and the schema pins in the store tests move with it. A turn that names no relationship bypassed the correction gate. The single lane lets a claim carry a forge pull request instead, and underCorrectionRefusal returned nil when the turn omitted RelationshipID, so the accepted head could still be carried and landed while its correction was open. The relationship is now resolved from the turn's pull request through the acceptance's recorded forge identity (dag_acceptance_forge), so both routes meet the same gate; a turn whose pull request has no accepted result is left to the lane's own rules.
…-result-correction # Conflicts: # plugins/crw/.codex-plugin/plugin.json
…, and keep it from starting over an in-flight merge The pre-merge evaluation of d4ca75c found three ways the promise of decision 2 (a node under correction is not merged on its old head) could still be broken. A lane turn that named neither --relationship nor --pr held only the head, and the gate exempted it. Both selectors are optional on merge-turn-request, so the head is the identity left: a turn whose head is an active acceptance's recorded head of its own repository resolves to that acceptance's relationship. A turn whose head no acceptance stands on is still left to the lane's own rules. The pull-request fallback compared the forge slug with a case-sensitive equality, while GitHub slugs are case-insensitive and every other repository identity comparison in the relay folds case: a turn spelling the repository differently read as no accepted result. The lookup folds case now. A correction could be recorded while a merge turn of the same relationship was already merging, of unknown effect, or landed. The relay reads the forge, not the merge, so such a turn may already have carried the accepted head to the base: recording a correction over it would name a head on its way to landing as the one being repaired, and no later refusal can undo a merge the forge already made. The correction is refused and the turn is resolved first, the same conservative reading the verdict writer makes when a ruling rests on a merge turn. A turn that only waits or holds the lane is not this case: the lane's own check and land already refuse it under correction, so it cannot progress. The documentation said the under-correction refusal precedes every judgement rule including an evicted head; the refusal is made where the head would otherwise be judged, so an already-evicted head keeps its eviction. The text now says what the code does.
…ded member's correction at land The pre-merge evaluation of 6b66b30 found two more ways the promise of decision 2 (a node under correction is not merged on its old head) could still be broken. Both paths existed on dev without the gate, and both fall inside the promise this issue makes. A recorded base refresh moves the head the acceptance stands on without changing the accepted head, so a lane turn naming neither --relationship nor --pr and holding the refreshed head resolved to no acceptance. The head lookup reads dag_base_refreshes beside dag_acceptances now: the head a turn holds is the accepted head or the head of a refresh of it, and either belongs to the same acceptance. merge-train-land excluded a member whose turn left the lane from the landing, but the member's code is still in the merge commit the bundle lands, so a tree that still carried a result under correction could be recorded landed. The excluded members' correction state is read inside the same transaction now. The CRW-897 carve-out keeps holding for what it is about: a revoked acceptance or a moved head of a member that left does not refuse the rest of the bundle.
…hing acceptance, and hold an excluded member to its stand The pre-merge evaluation of 30087f6 found three more ways the promise of decision 2 (a node under correction is not merged on its old head) could still be broken. d1: the in-flight-merge guard read only merge_turns.relationship_id, so a turn requested without --relationship escaped it. --relationship and --pr are both optional on merge-turn-request, and a successful merge-turn-check has already authorized the merge of H before the correction is recorded, so the later Land refusal cannot undo what the forge was authorized to do. The guard now resolves the turn by the relationship, the pull request or the head it holds, the same three identities the lane's own gate resolves. d2: the selector fallback asked one arbitrarily chosen acceptance (newest, or relationship order). Acceptance uniqueness is per node and output, not per repository and pull request or per repository and head, so two accepted nodes can name the same commit. Every matching relationship is read now, and the turn is refused when any of them has a correction open over the head it holds. d3: an excluded member's hold cleared as soon as its corrected result was accepted over. Exclusion changes the accounting, not the bundle tree: the verified tree still carries the head it was verified on, so once the acceptance stands on a corrected head the bundle no longer holds what the plan accepts for that member. The excluded member is now held to its stand, and a bundle that no longer carries it is refused for rebuild and re-verification. The CRW-897 carve-out keeps holding for what it is about: a revoked acceptance or a moved head of a member that left does not refuse the rest of the bundle.
…ch a live bundle or a moved stand The pre-merge evaluation of e1a9072 found three more ways the promise of decision 2 (a node under correction is not merged on its old head) could still be broken. d1: naming a relationship suppressed the other identities. Acceptance uniqueness is per node and output, so two accepted nodes can accept the same commit: a turn that names the relationship which is NOT being corrected is still carrying a head another relationship is repairing. The lane gate now resolves the relationship AND the pull-request and head identities, so a named relationship adds to the set instead of replacing it. d2: the in-flight guard read merge turns only. A verified bundle is the other way the accepted head may already be on the base: the parent merges the bundle on the forge and records it with merge-train-land afterwards, so between those two the members still hold or wait in the lane while the forge already carries the head. A live (opened or verified) bundle carrying the node's stand head now refuses the correction, and the way on is merge-train-close. A bundle that landed, was done or was abandoned carries nothing live. d3: the recheck inside the write transaction asked only whether a correction is open. A correction opened AND accepted over inside the gap makes the live and stand generations equal again, so the check now compares the head the bundle carries with the head the acceptance stands on. That covers verify and open, and the excluded-member path uses the same comparison.
…e a replaced head The pre-merge evaluation of 141c105 found three more ways the promise of decision 2 (a node under correction is not merged on its old head) could still be broken. d1: the bundle gate read only the relationship named on each member. Two accepted nodes can name the same commit or the same pull request, so a member whose own relationship is live can still carry a head another relationship is repairing. Every member is now asked through the lane's own gate with its own identities, so the bundle applies the same rule the single lane does. d2: after dag-accept --supersedes the old head belongs to no active acceptance, so a turn held for it escaped every lookup. A turn holding a head an acceptance recorded and that a later acceptance replaced is now refused, naming the relationship. d3: the new lookups matched dag_acceptances.repository, but an acceptance written before the forge rule keeps whatever target it was accepted against, a local checkout included, while dag_acceptance_forge holds the owner/name a merge turn is requested against. Both the lane gate and the in-flight guard match on the forge identity now, falling back to the acceptance's own repository when no forge row exists. d4: the refusals told the coordinator to open the generation again, which would leave the unrecorded generation behind. They now say to record this same generation once the bundle is closed or the turn resolved, and docs/relay/dag-scheduler.md states that precondition.
…ix the links to dag-scheduler Round 6 of the correction review. The lane's gates and the bundle's landing now resolve the correction state of every member, the refreshed heads of an acceptance survive dag-accept --supersedes, and a legacy acceptance is matched by its forge identity in the in-flight guard. The coordinator's skill references describe the hand-opened correction route for an accepted, current result. - train.go: TrainLand re-checks the surviving members' correction state inside the landing transaction. - undercorrection.go: a replaced head keeps the heads recorded by dag_base_refreshes. - revalidation.go: the in-flight guard matches the acceptance's forge identity. - correction_forge_identity_test.go: red first on the previous head, green after. - train_undercorrection_test.go: lane and landing cases for these identities. - plugins/crw/skills/crw-run references: the accepted-result correction route; plugin.json re-recorded.
The test TestTheInFlightGuardMatchesTheForgeIdentityNotTheStoredTarget calls underCorrectionRefusal, the lane gate, not the dagsched in-flight guard. Rename it to TestTheLaneGateMatchesATurnByItsForgeIdentityNotTheStoredTarget and correct the first sentence of its comment. The body is unchanged.
merge-turn-request stores the caller's head verbatim, while SameCommit treats a head as one commit in any case and with surrounding spaces. The correction gate in the lane and the in-flight guard compared heads with raw SQL equality, so a head-only turn holding the accepted head in upper case resolved to no relationship and passed. The comparisons now trim and lower-case both sides, in the lane gate, the replaced-head lookup and the in-flight guard. TestTheLaneGateMatchesAHeadRegardlessOfItsSpelling failed before this change and passes after it.
… accepted nodes A head an accepted node replaced by a correction stayed mergeable through another accepted node that still stood on it: the replaced-head check ran only when no relationship matched the turn's head. It now runs for any held head, and it counts only superseded or revoked acceptances, skipping a relationship that still holds the head actively (the happy path is unchanged). The live-bundle guard matched a bundle member by the acceptance's own relationship only, so a bundle carrying the same commit through another node was missed. It now matches the member by repository and head. The coordinator's late-thread procedure said a red or P0 thread on an accepted current result is held without a generation, which contradicted the correction route. It now says the minor thread is held and the red, P0, P1 or security thread takes the hand-opened route. Tests added red first: TestTheLaneGateRefusesAReplacedHeadThatAnotherNodeStillAccepts and TestACorrectionIsNotRecordedOverALiveBundleCarryingTheHeadThroughAnotherNode. The plugin version is re-recorded.
…as intended The replaced-head check excluded only the relationship's own head, so a superseded acceptance's base refresh could refuse a turn on the active node's current head reached through its own refresh. The exclusion now counts the active acceptance's refreshes too. The live-bundle guard again matches the acceptance's own member by relationship, wherever its train names the repository, so a legacy acceptance keeps the refusal it had. Tests added red first: TestTheLaneGateKeepsAHeadThatTheActiveAcceptanceReachesByARefresh and TestACorrectionIsNotRecordedOverALiveBundleCarryingTheHeadThroughItsOwnLegacyNode.
A bundle member whose turn left the lane and whose own acceptance was revoked skipped the correction check, so a correction another node opened over the head the bundle still carries did not refuse the landing. The excluded member now gets the same check. A revoked acceptance remains no reason to refuse the rest of the bundle, so only a superseded head counts as replaced. The scheduler procedure said a node that is not recorded as an execution reads blocked:stale_head; that is the state of the edges that consume it, and the node itself reads done:accepted. It also said a refused landed turn can be recorded again after resolve; a landed turn is refused and resolve does not return it. Test added red first: TestTrainLandRefusesAnExcludedMemberWhoseHeadAnotherNodeIsRepairing.
…esh is The under-correction refusals told the coordinator to withdraw the open generation, which the withdrawal rejects once the generation is bound or sent. They now say so. The scheduler and the coordinator's merge-readiness reference said a base-only change is refused as not stale and cannot go back through dag-correct; the scheduler's wording now names the reason it is not a correction, and the reference points a base-only change to dag-base-refresh. The plugin version is re-recorded.
SameCommit trims every surrounding whitespace, but the correction gate trimmed only spaces in SQL, so a turn holding the accepted head padded with a tab or a newline resolved to no relationship and was not refused. The turn's head is now trimmed as SameCommit trims it, and the SQL trims the ASCII whitespace characters on both sides. Test added red first: TestTheLaneGateMatchesAHeadPaddedWithTabsAndNewlines.
A correction's guards compared a stored head with the accepted head through a SQL trim that removes only ASCII spaces, so a head stored with a no-break, ideographic or other Unicode space was not found. Go's SameCommit trims Unicode whitespace. Move that definition into internal/relay/commitid, register it once per store driver as the SQL function crw_same_commit, and use it at every trim/lower head comparison in mergeturn and dagsched. Storage stays verbatim. Tests: the lane gate, the in-flight merge-turn guard and the live-bundle guard agree with SameCommit for each spelling; a source scan fails on a trim or lower applied to a head or sha column.
…ded member A landed bundle merges a tree that contains every member head, including the members whose turns were withdrawn. The live-bundle guard read only opened and verified bundles, so a correction could be recorded over a head that had already reached the base through an excluded member. Refuse the correction when a landed bundle carries the head as any member, with the existing disposition_conflict reason. Tests: the landed-bundle case of the live-bundle table now expects that refusal, and an excluded member of a landed bundle (another node's relationship) is refused; both were red before the change.
…nds does Two review findings on the replaced-head lane gate and the SQL identity function. A base refresh row kept a superseded head on its active acceptance without checking the row's digest, which acceptance.Stands checks. A row whose id is not the digest of its fields now keeps nothing: the candidate is read in Go and checked with the same digest, and only the valid acceptances are named in the query. crw_same_commit counted a BLOB whose bytes spell a commit as that commit. Only TEXT names a commit now; a BLOB, a number or NULL names none, as the commit identity in Go says. Tests: an invalid refresh row no longer keeps a superseded head (new), a BLOB and a number name no commit (new), and the existing refresh-keeps-head test uses the real digest.
…nt the accepted-current route An acceptance can record a local checkout and a forge identity. The lane gate and the correction's in-flight guard matched a turn only against the forge identity, so a turn held under the local path was missed. Both names are matched now, in the replaced-head lookup, the open-correction lookup, the in-flight guard and the live-bundle guard. docs/relay/dag-scheduler.md said only a stale node takes the manual route; an accepted node that is still current does too when the generation's reason states a correction.
…a base refresh of the same result The correction hold matched a turn's repository as stored text, so a checkout spelled through its .git directory, a symlink or a trailing segment was not held. Repository names are now compared by mergeturn.SameRepository, under every spelling an acceptance has (its repository and its forge repository). The bundle gate for an excluded member refused a member whose stand head moved by a base refresh of the same result. A head in the acceptance's chain (acceptance.Chain) is the same result; only a head outside it is a replaced result.
…ding its git directory The lane's reader joined .git onto the raw spelling, so a symlink followed by a parent segment (link/../repo) named a different git directory than the identity that resolves it. gitDirOf now resolves the repository first, and a test pins the spelling.
…or it A local repository spelling is now named by git itself (git rev-parse --git-common-dir, symlinks resolved), so a linked worktree, its administrative directory, a symlink, a trailing slash and a /. segment name the same repository, and a directory whose name ends in a space keeps its name. Forge slugs are unchanged; a path git cannot name is compared by its exact text. The docs say so.
thisisjun786
added
the
crw-lane
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
CRW-906: record a hand-opened correction of an accepted, still-current result, and hold the old head back while it is open
Problem
A bundle review found a blocking defect in a member the plan had already accepted and that had not
landed (a Codex P1 on an accepted member of a bundle pull request). Two things were missing:
dag-correct --manifest-digestrefused it withdisposition_conflictand "an accepted result that is currenthas no recorded correction route in this build", so
dag-accept --supersedesthen refused theunrecorded generation; and
pull request head that differs from the accepted head, and that guard holds only once the child
has pushed: between opening (or recording) the correction generation and that first push the pull
request still shows the accepted head H, so a bundle open, a bundle verify, a bundle land and the
single-lane judgement all carried H — exactly the head being repaired.
Re-registering the criteria to make the node stale moves the route to
revalidate, and the oldoutput cannot be ruled
verifiedagain, so that is not a way out.Behavior after this change
The correction route
A generation a coordinator opened by hand for a node whose result is accepted, still current (not
stale) and not landed is recorded as a correction when the reason the generation was opened under
states a correction. Two reasons state it:
needs_changes_revision, the valuegeneration-openwrites by default, andaccepted_result_correction, a new value of that same--reasonoption that names this route.The procedure is the stale case's, unchanged, with the reason carrying the intent:
dag-correct --prepareprints the instruction line, the manifest digest and the dispatch request id.generation-open --reason accepted_result_correction(or--reason needs_changes_revision) opensthe next generation; the instruction line is dispatched to the child and that turn is bound with
generation-bind.dag-correct --manifest-digest Drecords the generation as acorrectionexecution of the nodeand answers
opened_bygeneration_openwith the request id and the bound turn.dag-accept --supersedes <the previous acceptance>takes the corrected result.The old head is held back while the correction is open
The merge gate now reads the relationship's live execution generation against the generation its
active acceptance stands on. While the live one is later, the node is under correction and the
head the acceptance stands on is refused:
dag-merge-judge,dag-merge-request) is refuseddisposition_conflict, names the open generation and writes no row and no turn, andtrainStandRefusal).A lane turn is not exempt by omitting its selectors:
--relationshipand--prare both optional onmerge-turn-request, so a turn may name only the head, or only the pull request. It resolves to the acceptance that recorded it - the accepted head, the head of a recorded base refresh of it (dag_base_refreshes), or the pull request - matched on the acceptance's forge identity (the owner/name a merge turn is requested against; an acceptance written before that rule keeps its own stored target), with the slug folded for case, and every matching relationship is asked, because acceptance uniqueness is per node and output, so two accepted nodes can name the same pull request or the same commit: the turn is refused when any of them is correcting the head it holds. A turn whose head no active acceptance of its repository stands on is left to the lane's own rules.The bundle land reads the excluded members as well. A member whose turn left the lane is still in the merge commit the bundle lands, so its correction state is read, and it is held to the head its acceptance stands on: once
dag-accept --supersedeshas moved that acceptance to the corrected head, the old bundle no longer carries what the plan accepts for that member and is refused for rebuild and re-verification. The carve-out it must keep keeps holding: a revoked acceptance, or a moved head of a member that left, still does not refuse the rest of the bundle.The gate is about the correction being open, not about the head, so it clears by itself:
dag-accept --supersedesof the new generation's result moves the acceptance to the standgeneration and the node is judged again; a withdrawn generation puts the live generation back and
the old head may ride again; and a generation recorded by
dag-base-refreshmoves the standgeneration with the live one, so a base refresh is not a correction. The reading is taken with the
acceptance in one statement (
acceptance.ActiveWithLiveGeneration) and asked again in thetransaction that writes, so a generation opened while the pull request is being read is seen.
A correction is also not recorded over a head that may already be on its way to the base: a merge turn that is
merging, of unknown effect orlandedrefuses the correction, because the relay reads the forge and not the merge, and no later refusal can undo a merge the forge already made. The turn is resolved by every identity a claim may carry - the relationship, the pull request, or the head it holds - because a successfulmerge-turn-checkhas already authorized the merge of that head before the correction would be recorded. The turn is resolved first (merge-turn-resolve, ormerge-turn-unknownthenmerge-turn-resolve). A turn that only waits or holds the lane is not this case: the lane's own check and land already refuse it under correction, so it cannot progress.Until step 4 the previous acceptance stays active, so nothing downstream consumes the corrected
generation early, and the node reads
blocked:stale_head(its current generation is not yet recordedas an execution).
Every existing check is kept
The change admits one more case; it removes no check.
recordHandOpenedstill refuses, in this order,and still writes nothing on a refusal:
dag-correctsteps);NULLreason, a reply) — refused naming the reason it carries;correct;open is not skipped by opening another beside it);
--manifest-digest, a digest that does not digest to its name, a manifest not stored forthis node and issue, a manifest prepared for another slice or criteria set;
(
CorrectionRequestID);The merge gate keeps every rule it had, and the correction refusal sits where the head the judgement
is about is the head the acceptance stands on. A pull request at another head is still
stale_head,and the base-refresh lane tests (a refreshed node judged and merged; a head no record covers refused
as before) pass unchanged.
Acceptance criteria, as delivered
c1 —
internal/relay/dagsched/revalidation.gorecordHandOpenedadmits the accepted-current casegated by the open reason, keeping every check above. The route is noted inside the existing
generations.reasoncolumn: no schema change, no new column or table, no new refusal reason, no CLIflag, no
internal/relay/argparse/specs.jsonchange.docs/relay/dag-scheduler.mddescribes theprocedure under "Correcting a result that was accepted and is still current" and the merge gate under
"Merge eligibility".
c2 — red first, green after, on temporary stores only. The new test files
internal/relay/dagsched/correction_accepted_test.go,internal/relay/dagsched/mergejudge_undercorrection_test.go,internal/relay/mergeturn/train_undercorrection_test.goandinternal/relay/acceptance/undercorrection_test.gofail on the baseline and pass after the change. The red-first record for the merge gate is
crw-906/evidence/crw906-g2-red-first.txt; the generation-1 record iscrw-906/evidence/crw906-red-first.txt. The refusals pinned: an open reason that states nocorrection, a landed node, a generation that is not right after the acceptance's, a manifest of
another node version, an unbound generation, a missing
--manifest-digest, and (generation 2) amerge carried on the accepted head while a correction generation is open. Every existing correction,
train, acceptance and merge-judge test passes.
c3 —
make testis the hosted CI on this head. Locally: the packages this change touches passunder
-count=1, andgofmt,go vet(default andGOOS=darwin),make lint,git diff --check,crw-dev ci validate,crw-dev ci contractsandcrw-dev ci pluginall pass.c4 — one pull request into
dev, open and non-draft, English title and description; every CI jobgreen on one head; the one-shot Devin and GitHub Codex reviews finished (or skipped/silent) with their
red, P0, P1 and security findings resolved.
c5 — scope is this issue alone. Files edited outside the declared regions are listed below with
their reason. No test touches the live relay service, a production relay store or a real Codex task.
Files edited outside the declared regions
Declared regions:
internal/relay/dagsched/revalidation.go, the newinternal/relay/dagsched/correction_accepted_test.go,docs/relay/dag-scheduler.md.internal/relay/acceptance/stand.goActiveWithLiveGeneration(the acceptance and the relationship's live generation in one statement) andUnderCorrection(the CRW-906 gate). No existing function changes.internal/relay/dagsched/mergejudge.goJudgerefuses an accepted result under correction (disposition_conflict, naming the open generation) where the head judged is the head the acceptance stands on, andRequestMergeTurnasks the same question in the transaction that creates the turn.internal/relay/mergeturn/train.gotrainStandRefusal(Blocking 1's gate, reused by open, verify and land) refuses a member whose accepted result is under correction: the change is inside that function and the arguments its three call sites pass, and the read of the live generation replaces the plain acceptance read at those sites.TrainLandalso reads the correction state of the members whose turn left the lane, because their code is still in the merge commit the bundle lands.internal/relay/acceptance/undercorrection_test.gostand.go: the reading against the stand, the two cases that must not read as a correction (a recorded base refresh, a withdrawn generation), and the acceptance read apart from the generation.internal/relay/dagsched/mergejudge_undercorrection_test.godag-merge-requestrefused while the correction is open, and judged again once it is withdrawn or accepted over.internal/relay/mergeturn/train_undercorrection_test.gocontract/schema/relationship.jsongenerations[].reasonenum gainsaccepted_result_correction.Generation.contractemits the reason andRelationship.ContractRecordincludes every generation, so a relationship opened by this route serialized a value the shipped contract refused; the enum is the artifact that had to move. One line added to the enum.contract/schema/SOURCE.mdrelationship.jsonsha256 and why the enum gained the value, with the re-judgement of the existing cases.internal/relay/store/relationship_reason_schema_test.gogeneration-openaccepts and the empty reason a returning tenure leaves are valid, an unknown reason and a mistyped one are refused, and the schema still refuses a reason the relay never writes.internal/relay/store/testdata/fixtures/relationship-reason-verdicts.json.gzinternal/relay/store/schema_validator_test.goschemaPinsmoves therelationshipdigest to the revised bytes, as it does whenever a schema's cases are re-judged.internal/relay/store/schema_conformance_test.gointernal/relay/registry/registry.goreasonsslice is whatgeneration-open --reasonvalidates against. One value appended;registrycannot importdagsched, so the literal is spelled here with a comment pointing atdagsched.AcceptedResultCorrection, and a test pins the two to each other.internal/relay/dagsched/correction.goRecordCorrectiongates on the generation reason before it decides ruling vs hand-opened. Without this the new reason is refused before the binder is reached. Thedecision_replybranch above it is untouched.internal/relay/dagsched/withdraw.gointernal/relay/dagsched/revalidation_test.goinitial_assignmentand asserts the reason gate's refusal.internal/relay/dagsched/baserefresh_scenario_test.godag-correct --prepareprints, and no manifest is named), not by "not stale". The test's claim (a node is not integrated on a generation's mark without a recorded base refresh) is unchanged.internal/relay/delivery/rulingchange.gogeneration-open --reason accepted_result_correction, dispatch and bind,dag-correct --manifest-digest,dag-accept --supersedes).internal/relay/delivery/rulingchange_test.godocs/relay/README.mdgeneration-open --reason"accepts only its own two reasons" is stale; it now says the reasons that command writes.plugins/crw/skills/crw-run/references/relay.md--reasonvalues and their routes; the new value is named there with the procedure.internal/relay/mergeturn/undercorrection.gounderCorrectionRefusal(the turn-level refusal) andtrainMemberCorrectionRefusal(the per-member re-read), both readingacceptance.UnderCorrectionon the caller's querier.internal/relay/mergeturn/check.gomerge-turn-checkis what moves a held turn to merging, and a turn can have been granted before the correction was opened. It asks the correction question inside the transaction that writes, so a generation opened while the forge was being read is seen; the check writes no current row and the turn stays holding.internal/relay/mergeturn/outcome.gomerge-turn-landrecords the merge, so it asks the same question on its own transaction's snapshot; a refusal records no landing and leaves the turn merging.plugins/crw/.codex-plugin/plugin.jsoncrw-dev ci plugin --record-version) because a shipped skill file changed; never hand-edited. The version line on this head is0.4.0+e7af5fae7c4a, recorded from the 216 shipped files of the tree after thedevmerge (crw-dev ci pluginreports it equals the computed value).Contract and golden impact
The frozen relationship contract changes, deliberately and additively.
contract/schema/relationship.jsonis the record a schema-conforming consumer validates, andRelationship.ContractRecordemits every generation with its reason, so the reason this routerecords has to be one the shipped enum allows. The enum gains one value,
accepted_result_correction; thenulland the two existing reasons are unchanged, so every recordwritten before this change still validates. The database schema does not change: no column, table,
index or shipped zone statement is touched, and
generations.reasonstays the same text column.contract/schema/SOURCE.mdcarries the new digest and the note, and the two store tests that pin thecontract bytes move with it.
The 6 existing relationship cases were re-judged against the revised bytes with jsonschema 4.19.2
Draft7Validator and kept their verdicts; the re-judgement is recorded in
internal/relay/store/testdata/fixtures/relationship-reason-verdicts.json.gzbeside 6 new casescovering each reason the registry accepts, the empty reason, an unknown reason and a mistyped one.
No refusal reason is added and the refusal vocabulary does not move:
contract/schema/relay-exit-codes.jsonandinternal/contract/exit_codes_generated.goare untouched(D-02 — an existing reason with the same meaning is reused,
disposition_conflict). No other golden orfixture changes.
crw-dev ci contractsandcrw-dev ci validatepass.D-19: contract line-citation drift
The contract cites file:line at an older revision. Re-checked against this baseline:
registry/registry.go:27for the reason vocabulary — at the baseline the slice is line 26, and thischange moves it to line 28 (a two-line comment above it). The vocabulary it names gains one value.
registry/registry.go:542for generation 1 — already stale at the baseline (that line is inside thesupersedes check of
register); the generation-1 insert is at line 571 there.registry/registry.go:829forOpenGenerationIn— at the baseline it is line 826.delivery/ack.go:599for the correction generation — already stale at the baseline (that line is aclosing brace).
mergeturn/check.go:218for the report head comparison — still theRecordMergeCheckcall it cites.mergeturn/outcome.go:49-126,:94,:101forLand— unchanged in behaviour;Landis at line 51 here.mergeturn/target.go:80-103for the tip reader's local/slug branch — unchanged;Tipis at line 85.schema:732-755formerge_turns,schema:779-792formerge_turn_checks,schema:248-277forwork_reports,schema:31-42for thegenerationsprimary key and unique index andschema:21for
relationships.execution_generation— all still hold exactly.So the schema citations are intact and the Go citations had already drifted before this change; this
change adds two lines of drift to the first one.
Commands run
Red first (baseline, before the fix):
Green after (final head):
The temporary merge of this head with the latest
origin/devwas checked out into a scratch worktreeand built and vetted there:
go buildandgo vetof the changed packages pass.Hosted CI on the head is recorded in a comment on this pull request. The branch was merged with
devthree times (41d42987b,d4ca75cf9,30087f61a, merge commits) because the pull request had fallen behind: each time the only real conflict was a sibling's block appended at the same place ininternal/relay/mergeturn/train.goand the plugin version line, resolved as the union of both and a re-record of the version for the merged tree. No other code changed in the merges.Notes for review
RecordCorrectionchecks the generation's reason before it decides ruling vs hand-opened (thecorrectionOpenReasongate incorrection.go), so a generation opened under a reason that states nocorrection is refused there and never reaches the ambiguity branch. The ambiguity branch is unchanged;
it is reached only by a generation whose reason does state a correction.
normally, and an older runtime reads such a generation as it reads any hand-opened one (it accepts
only the two values it knows at
generation-open, and refuses to record a reason it does notrecognize).
--relationshipbypassed the gate through its pull request, and one omitting both selectors bypassed it entirely; that fallback compared the forge slug case-sensitively; it asked only one of several acceptances that can name the same commit; the head it held was not matched against a recorded base refresh; a correction could be recorded over an accepted head a merge turn was already carrying to the base, and that guard missed a turn naming no relationship and a live bundle carrying the head; the bundle gate trusted the relationship named on a member although another accepted node can name the same commit; a turn held for a head thatdag-accept --supersedesreplaced escaped every lookup; the lookups matched the acceptance's stored target rather than its forge identity; and the bundle land skipped, then did not hold to its stand, a member whose turn had left the lane.makes it safe, their tests, the documents that describe them, and the existing tests whose premise
it moves). No conceptually independent feature is mixed in, and nothing here is unverifiable on its
own.