Prove the plan and approval journey publishes only an approved plan - #198
Merged
Merged
Conversation
Add the acceptance fixture for issue #30: an unapproved plan, a rejected plan, a plan whose document revision has moved, and a plan replanned after approval each neither execute nor publish anything; a run requires an approval given for the plan displayed at that moment; and completion presents the revalidated verdict with the 64-hex digest of the artifact it published, distinct from the as-received input. The fixture is mutation-probed: removing the execution arming check, the revision comparison, the superseded predicate or the review clearing each makes it fail. The reviewed-digest equality in fixLifecycleStateName() and the fixPlanIsCurrent() clause in approveActionListPlan() are defensive against a state the journey offers no path to, and are documented as such rather than claimed as coverage.
mberrys
force-pushed
the
codex/issue-30-plan-approval-journey
branch
from
October 4, 2026 08:01
8d6907b to
650d982
Compare
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
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.
What changed
Issue #30 wants the plan and approval journey presentable end to end: operation scope, parameters, candidate impact, preview, approver decision, terminal result. The journey itself is already built.
ActionListPane.qmldrivesplanActionList(),approveActionListPlan()andexecuteApprovedActionListPlan(), andEditorHostalready binds an approval to the displayed plan digest, refuses a superseded one, and refuses to arm execution without that approval. What was missing is the acceptance proof the issue names: an end-to-end fixture over the governed gateway with the denied and stale cases, and an assertion on the terminal published and revalidated display.This adds that fixture to
UnitTestsEditorHost. It plans a bleed correction, asserts one plan digest on both the plan identity surface and the preview, refuses execution before any approval, honours an explicit rejection with nothing published, replaces the document revision and asserts the plan reports itself as no longer current with approval and execution both refused and nothing published, then plans and approves against the revision that is open and plans again: the run is refused until a fresh approval is given for the plan displayed at that moment, after which the correction runs tosucceeded. Completion assertions bind the presentation to Core's facts: the recheck carries a verdict from Core's own closed set and never a failure, the sign-off carries the approved plan digest and a published digest of 64 lowercase hex characters distinct from the as-received input, the lifecycle summary reports a published result, and the as-received input is never the publication target.Closes #30.
Per-criterion table:
fixReviewBindsToThePlannedDigestAndTheCurrentRevision; now also asserted across the preview and the identity surface, withreviewedPlanDigestequal to the digest displayed before approval, and refused after the plan is planned again on the same revisionsucceededand the completion presentation is asserted against Core's verdict and the signed digestsapproveActionListPlan()requiresPlannedandfixPlanIsCurrent(), andexecuteApprovedActionListPlan()requiresfixExecutionArmed(). Now asserted end to end after a real document replacement, after an explicit rejection, and after re-planningIndependent review
An independent reviewer was pointed at this fixture together with the product code it asserts on, on the ground that the same agent wrote both and the usual separation of duties was missing. It found that the fixture could still pass with two guards removed, and that the step described as denied never called
rejectActionListPlan()at all. Each finding was confirmed against the source before anything changed:approveActionListPlan()gates onstate == Planned && fixPlanIsCurrent(), so the old assertion could pass on the state check alone while the supersession clause was dead, andclearFixReview()runs ateditorhost.cpp:3351whenever a plan is accepted.Mutation probe: each guard removed in turn,
UnitTestsEditorHostrebuilt and the slot re-run, with the build exit code checked first so that a failed compile can never be read as a surviving guard.executeApprovedActionListPlandrops thefixExecutionArmed()checkfixPlanIsCurrent()ignores the document revisionfixPlannedPlanIsSuperseded()always returns falseclearFixReview()removed where a plan is accepted (editorhost.cpp:3351)fixLifecycleStateName()digest equality forced true (:680)approveActionListPlandrops the!fixPlanIsCurrent()clause (:2164)The two survivors are defensive clauses rather than coverage. Accepting a plan clears the review and a revision change clears it again, so no state reachable through the public API holds an approved review whose digest differs from the current plan's, and this journey offers no path to a
Plannedcontroller for a superseded revision. Both are named as such in the fixture comment, the changelog fragment and the commit message instead of being counted as proven.Release changelog
Topic PR:
changes/codex-issue-30-plan-approval-journey.md(Categoryadded, Audienceoperators, Breaking-Changeno).Proof
python scripts/agent/check-change.py --base origin/dev --build-dir build-localreportspass. Run asLOOP_SRC=<worktree> bash $TEMP/loop-build-parity.sh python scripts/agent/check-change.py --base origin/dev --head-branch codex/issue-30-plan-approval-journey --build-dir C:/.dev/repos/loop-build-parity --report $TEMP/issue30-check-change.json. Reportedstatus: pass, head650d982e, modulesquick, riskstandard, 25 checks with none non-pass, including the builds ofLoopEditor,LoopLibQuickandProductQuickAccessibilitySmoke, the builds of all ten quick test targets, andfocused_tests, which runs the wholeUnitTestsEditorHosttarget rather than only this slot. CI'sagent-fastjob remains the authoritative gate.loop-check-change.shleaves the Windows SDK offPATH, so every link step dies withLINK : fatal error LNK1158: cannot run 'rc.exe'and each affected target is reported asfail exit code 4294967295. The run above used a wrapper with the full toolchain onPATH.changes/<sanitized-head-branch>.mdfragment added (Category, Audience, Breaking-Change, Summary)Validation on source SHA
650d982e:UnitTestsEditorHost fixJourneyPublishesOnlyAnApprovedPlanBoundToTheDisplayedIdentity: passed, 0 failures, 7.95 s.UnitTestsEditorHosttarget inside the change-set gate'sfocused_tests: passed.python scripts/agent/check-architecture.py --base origin/dev --head-branch codex/issue-30-plan-approval-journey:architecture contracts ok.Commands (temporary build tree, Qt and MSVC through the wrapper that puts the Windows SDK on
PATH):Limitations. The run presents the published bytes' digest and does not materialize them as a file, so this fixture asserts the presentation rather than a filesystem write; freeing the published artifact to disk belongs to the export slice. The QML pane is exercised through the host invokables it binds to, not through a QML harness, so a QML-only regression that bypassed those invokables would not be caught here. The digest equality in
fixLifecycleStateName()and thefixPlanIsCurrent()clause inapproveActionListPlan()are defensive against a state this journey offers no path to, and are recorded as such rather than claimed as coverage. Only the Windows build was exercised; the Linux lane reports through CI.Internal logic (touched behavior-bearing code)
!fixExecutionArmed(), the refused invokables, andrecheck.available == false.Quality pass
anyor equivalent cast added only to suppress a type errorQuality summary (1-3 sentences): The diff is one fixture in an existing mapped target plus the two
changes/files, 176 insertions and no deletions. What was removed after its own failure output said so is a filesystem assertion the platform does not satisfy; what was added after independent review is the rejection step, the re-plan refusal and the explicit not-current assertion, each of which a removed guard now makes fail. What was kept on purpose is the digest comparison against the as-received input, because that is the assertion that tells a real publication apart from a no-op.Security and rollback
changes/files.Docs
docs/GOVERNED_EXECUTION.mdalready documents plan digest, previews, approval and the publish gate, and this fixture asserts the presentation against those facts without changing them.Self-review (BSP-002 §4.3)