Make the parent re-evaluate its candidates instead of being told to - #87
Merged
Merged
Conversation
Run ended the paragraph with "A status-only request wakes nothing", and nothing distinguished that from a checkpoint asked inside an authorized ongoing initiative. Read literally, the second fell under the first: a parent answered with a report and stopped, leaving work its existing approval already covered. Qualify the sentence instead of replacing it. A standalone status-only request still wakes nothing. A checkpoint inside an authorized ongoing initiative is handed to the responsible parent and handled there as a resume of the authorized run, on the path the Resume row above already describes, re-evaluating outstanding child results, pending decision requests, cleared blockers and empty execution slots before continuing the successors that approval covers. This adds no scheduler and no second writer: the same parent, children and assignments are reused, and nothing that already has an owner is dispatched. The handoff stays a manual push rather than automatic continuation, so OPS-8.1's rule that no future wake-up is guaranteed is untouched, and an explicit status-only, report-only, read-only or pause limit on the checkpoint still wins. Confined to the "Keep a project run moving" paragraph.
The four-request table classified every "how is it going" as a read that wakes nothing. That is the same defect the Keep a project run moving paragraph had: read literally, a checkpoint arriving inside an execution approval that is still in force fell under it, so the supervision reported and stopped while approved work sat unfinished. Qualify the one row. The read stays the default branch. Where this task supervises the initiative and its approval is still in force, a checkpoint also hands the pending approved work to the responsible parent on the resume path that parent already has, and reports what was actually done. The binding answer is unchanged and is now stated for both branches: a checkpoint creates no supervisor, moves no ownership, and starts nothing for work that already has an owner. An explicit status-only, report-only, read-only, plan-only, pause or no-contact limit keeps the row's original no-wake reading. Confined to that row.
The scan that fills empty capacity already existed, but nothing said when it runs or what a hold means, and the two reads that would answer it had just landed with no instruction naming them. So a parent learned a child had stopped only if somebody told it, and "record a concrete dependency, conflict, ownership, or capacity reason" let one sentence cover a blocker, an unproved owner and a capacity nobody measured, which recover differently. Name the forcing events inline, where they are always loaded: initial dispatch, a child's completion, blocker or decision request, an integration landing, and every resume or recovery. Before the turn ends the scan has run or the reason it could not be read is recorded, because a final answer describing what would be dispatched is not the scan. Join the two reads to the rules that were already written. An issue with a responsible child is not a candidate, and doctor --issue answers that from one non-constructing read before anything is created. A child stopped for a person is invisible to the reviewable reads, so dispositions-show enumerates them rather than their silence being read as progress; it reports what the child emitted, so OPS-8.1 still owns the emit and observation paths. references/reevaluation.md carries what needs a table rather than a sentence: the read behind each event, what the stopped-children reader does and does not answer, the closed decision set with the condition that clears each value and the row it rests on, and the per-candidate record with what each absence means. Capacity words stay in start-policy, reader vocabulary in relay, the turn-ending prohibition in OPS-8.1. The set has no escalation value, so a review round count has nowhere to land. It separates no capacity from capacity nobody measured, an unproved owner from two the store reports, and an unreadable disposition from a contested one. task-packet gains the field a checkpoint reads, since the record list had none and criterion 10 was unobservable without it.
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}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Seven findings, all real. The file restated relay.md's own vocabulary after saying it would not, and the copies had already drifted from it; it is now the pass-specific mappings and links, and relay.md keeps its semantics. Three factual corrections. The forcing-event table disagreed with the always-loaded list in SKILL.md: it dropped initial dispatch and promoted capacity, edit overlap and new attachments to triggers, when those are inputs read inside every pass. It listed linkage-attach, which mutates, as a read. The ownership procedure used the wrong axis. assignment-find reports an ambiguous project parent through its scope reading, not several responsible children for one issue, so defer:ownership_ambiguous as written could never be reached: holds true already ends the evaluation. It is now defer:scope_ambiguous on the axis that command actually answers. A declared supersession does not clear a contested execution disposition; only a fresh generation does. And the set could not record a real contradiction in the criteria, which the same paragraph said may travel upward, so defer:criteria_contradiction is added. Retaining no review-round escalation value stays deliberate. The file also ended with a blank line, which git diff --check refuses.
defer:scope_ambiguous could not occur. The owning project parent is only read where a live assignment exists, and that is exactly the case doctor --issue ends as skip:already_owned; with holds false the scope reading is always unscoped. So the value sat in a closed set promising a condition a later pass would re-read, and nothing could ever set it. Ownership is one question with one read. doctor --issue constructs nothing and settles it: holds true reuses the existing child, holds false is dispatchable on that axis, and holds null or an unusable state directory is unproved rather than free. The second, store-constructing lookup is gone with the value it existed to produce.
The rationale claimed a further lookup could only repeat doctor. That is wrong: a project parent can be read without any assignment, on a different axis. The pass stops at one read because issue-child ownership is what decides whether this candidate may be dispatched, not because nothing else could be asked.
The ownership read was written as if a relay always answers. On an explicitly direct assignment there is no store, so doctor --issue reports holds null, and the rule turned that into defer:ownership_unverified for every ready issue. A direct run would have dispatched nothing at all, which is the opposite of what the pass exists for. Scope both new reads to the execution mode this skill already determines, using the same "where a relay holds the assignment" condition the rest of the file uses. A direct assignment takes its owner from the coordination record and the existing task, as everywhere else here, and is never held for the absence of an answer from a relay it never used.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
The previous commit swung too far. Saying defer:ownership_unverified does not apply to a direct assignment removed the only ownership hold the closed set has, so a resumed direct run that cannot read its coordination record had no valid decision: dispatch and open a second writer, or decline and record nothing. Separate the two things that were conflated. The absence of a relay is not a hold, because a direct run never uses one. A failed read is a hold in either mode, because direct mode removes the relay lookup and not the obligation to establish an owner, and an inaccessible record is not evidence that no writer exists. The decision's evidence now names whichever read failed.
The criterion is that repeated review cost must not send an already approved correction back to the user, and the skill never said so. It rested on the closed decision set having no escalation value, which governs candidate holds rather than corrections, so nothing reached the section that actually sends one. State it where corrections are returned: the next round goes to the same task with what changed, and the parent judges whether the evidence supports continuing. New scope, new authority and a real contradiction in the criteria stay decisions somebody else owns; a round count is not one, and neither is the cost of having looked again.
The pass named merge-turn-show, region-show, capacity-show and linkage-outstanding and left a reader to infer what their selectors, refusals and absences meant. Both reviews raised it against this delivery, and it was the weakest part of it: naming a command without saying what its silence means is how a coordinator reads an empty answer as a settled one. Document them where relay command usage already lives. An unregistered merge turn answers ok false rather than an empty record, so a mistyped id is not read as a released window. capacity-show carries headroom only when a scope is given, and only for dimensions with a declared limit, so an undeclared scope is unbounded by the store rather than known to have room. A region nobody proposed has no row, so an absent overlap is unknown rather than none. And linkage-outstanding is scoped to the project rather than the current parent, because filtering by parent once let a replacement look as though it had no outstanding work and a second replacement take the project without acknowledging anything. The three that go through the enforcement wrapper also carry unenforcedIndexes, which tells a reader deciding whether to act on a contested owner that the database is not stopping a second one either.
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.
Problem
The scan that fills empty capacity with ready successors already existed (
Establish the project baseline), but two things were missing, and together they meant a parent waited to be told.Nothing said when the scan runs. The trigger list named initial dispatch, a completion, a blocker and an integration, and omitted resume and recovery — and nothing made the scan a duty before the turn ends, so a parent could end on a final answer describing what it would dispatch.
Nothing said how a parent learns a child stopped.
assignment-findand the verification reads answer aboutready_for_reviewand nothing else, so a child that recordedblocked_needs_inputwas invisible and a coordinator reading only those concluded nothing was wrong. The reads that answer this landed very recently (#71, #73, #86) and no skill instruction named any of them.And
Record a concrete dependency, conflict, ownership, or capacity reason for deferring an otherwise ready issuelet one sentence cover a blocked prerequisite, an owner nobody could prove, and a capacity nobody measured. Those recover differently, so a later pass could not tell which condition to re-read.What changed
Inline in
SKILL.md, because it has to be loaded every turn: the forcing-event set now includes resume and recovery; the scan is a duty before the turn ends, or the reason it could not be read is recorded; an issue that already has a responsible child is not a candidate anddoctor --issueanswers that from one non-constructing read before anything is created; stopped children are enumerated withdispositions-showinstead of their silence being read as progress; and the deferral reason comes from a closed set.references/reevaluation.md(new, table-only) carries what needs a table rather than a sentence: the read behind each forcing event, what the stopped-children reader does and does not answer, the closed decision set with the condition that clears each value and the row it rests on, and the per-candidate record with what each absence means.references/task-packet.mdgains the coordination-record field a checkpoint reads. The record list was closed and had no entry for candidates, decisions or holds, so that state was unobservable.Before / after
A child records
blocked_needs_inputand the parent is mid-run.Design constraints kept
No scheduler, no new store, no cursor, no second opinion about anything the relay already decides. Words that already have owners stay there: capacity bounds and
unmeasuredinstart-policy.md, reader vocabulary inrelay.md(which explicitly reserves it), the turn-ending prohibition in OPS-8.1, record location intask-packet.md.The closed set has no escalation value, so a review-round count has nowhere to land — the parent rules on criteria and correction scope and returns the work to the same child.
Ownership uses one read where one read suffices:
doctor --issueis non-constructing and deliberately narrow (first live relationship, silent about a second).assignment-find --issuereports ambiguity but constructs a store, so it is ordered second and only afterstoreAgreement: same.Validation
python3 scripts/ci/plugin.pyexit 0;python3 scripts/ci/validate.pyexit 0 (Validated 9 skills, local link paths and Python syntax);quick_validate.py plugins/crw/skills/crw-runexit 0;git diff --checkclean. Independent adversarial review was run against the committed diff and the landed payloads.No gate interprets the meaning of instruction prose; that is human review, and this PR does not claim otherwise.
Scope and what this does not establish
This lands source. It does not install anything and it is not evidence of live behaviour. The issue's criterion requiring an installed relay and observed multi-issue runs is a separate step a merge does not produce.
Commits
ee5d802and8e4917don this branch are the earlier Phase A wording fixes for the same issue, already reviewed.Four readers named for the remaining forcing events (
merge-turn-show,linkage-*,region-*,capacity-show) have no skill-level usage documentation yet; the file says so rather than implying coverage. Documenting them is a follow-up.Review rounds on this PR
Five independent review rounds on the built diff, ending PASS, plus two defects found by Devin on pushed heads and fixed:
2601417relay.md's own vocabulary after saying it would not, and the copies had drifted; the forcing-event table disagreed with SKILL.md and listed a mutating command as a read; two payload claims were wrong; supersession does not clear a contested disposition; the set could not record a criteria contradictionfb9313dfb9313ddefer:scope_ambiguouswas unreachable: the project-scope reading only exists when an assignment does, whichholds: truealready ends5d63f55,e617b0825bf819doctor --issuereturnsholds: null, so every ready issue becamedefer:ownership_unverifiedand a direct run would dispatch nothing25bf819scopes both reads to the determined execution mode77824b477824b4separates relay absence (not a hold) from a failed read (a hold in either mode)Both Devin threads are resolved by Devin itself (
4057944064,4057998554).65e07aeadds the last criterion this phase owed: a review-round count is not a reason to send an approved correction upward. It had been resting on the closed decision set having no escalation value, which governs candidate holds rather than corrections, so it never reached the section that actually returns one.Update (
6eb36b7): the four readers the pass names —merge-turn-show,region-show,capacity-show,linkage-outstanding— now have a usage contract inrelay.md, taken from their handlers and parsers. This was the one finding both the independent reviewer and Devin raised that I had deferred as a follow-up; it is closed here instead, so no follow-up issue is needed for it.reevaluation.mdno longer says they are undocumented.Head
6eb36b7is green on run35537572242:dev-gatepass, all jobs pass,release-gateskipping,Devin Reviewpass, no new findings.