Add governed foreground work to repository flows - #221
Conversation
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Codex automated review
Verdict: patch is incorrect
Confidence: 0.97
The patch introduces an upgrade blocker, a false planning-package approval path, and a compiled foreground-work dead end. Model-level verification is recommended before merge. Questions: can any new work status or planning-package state become permanently blocking, and can the new planning transition priorities shadow an otherwise required recovery or prerequisite transition?
There was a problem hiding this comment.
Codex automated review
Verdict: patch is incorrect
Confidence: 0.98
Foreground-work freshness can admit stale outputs, and the provider-authority migration strands an existing correction transition plus a recovery route. Model-level verification is recommended for work invalidation/retry cycles and provider-authorized recovery reachability.
There was a problem hiding this comment.
Codex automated review
Verdict: patch is incorrect
Confidence: 0.94
The new package state aliases conventional PlanValid while using a separate lock, creating both a deterministic resolver/apply disagreement and a same-base concurrency integrity failure. Model-level verification is recommended before merge, particularly to check whether any other conventional plan transition can match package-backed states or shadow required package prerequisites.
There was a problem hiding this comment.
Codex automated review
Verdict: patch is incorrect
Confidence: 0.98
The patch introduces a concrete recovery blocker and accepts planning-work contracts that necessarily enter zero-progress refusal loops. Model-level verification is recommended before merge, particularly for reachability of every provider-backed recovery and satisfiability of every work-enabled transition contract.
There was a problem hiding this comment.
Codex automated review
Verdict: patch is incorrect
Confidence: 0.98
The patch breaks upgrades from the stated base schema and introduces concrete control-bundle identity, replay, and foreground-work progress failures. Model-level verification is recommended before merge, particularly for control-bundle target mismatch recovery and progress from every newly introduced foreground-work state.
Findings without inline diff anchors
[P2] Project control-bundle mutations before validating an idempotent replay
Invariant: retrying a committed idempotency key must return the winning receipt without repeating effects. For installation/runtime transitions, the supplied bundle is projected from source B0 to target B1. After commit, a retry either carries the original unprojected target or rebuilds its source from the now-current B1. This early check compares that request bundle directly with receipt facts B0→B1, deterministically rejecting the already-committed operation as a different bundle. The new bundle comparison introduces the false failure; callers that lost the first response cannot recover the committed receipt. Add a regression that commits installation.update, rebuilds the request from the resulting runtime pin, retries with the returned key, and expects Replayed=true with zero additional effects.
Confidence: 0.98
Location: boatstack/internal/softwaredelivery/engine/engine.go:330-337 (RIGHT)
|
Resolved the unanchored idempotent-replay finding in commit 9fe4b50. Replay now accepts either the original admitted source or the already-committed target as the current bundle source, while still requiring the exact committed target and rejecting unrelated drift. TestMaintenanceReplayAcceptsCurrentCommittedControlBundleTarget covers rebuilt-current success and unrelated-bundle refusal; TestConcurrentApplyConsumesOneRevisionExactlyOnce confirms the winning receipt replays without another durable commit. |
There was a problem hiding this comment.
Codex automated review
Verdict: patch is incorrect
Confidence: 0.93
The patch introduces an actionable provider-authority bypass and an incomplete admission-to-receipt relation for foreground work. Model-level verification is recommended before merge, particularly to check whether new control-bundle preflights can block workspace cleanup/recovery or create zero-progress retries.
There was a problem hiding this comment.
Codex automated review
Verdict: patch is incorrect
Confidence: 0.97
The patch introduces concrete failures in explicit apply, idempotent recovery, valid optional-work completion, and approval attribution. Model-level verification is recommended before merge, particularly for whether new control-bundle refusal points retain recovery paths and whether foreground-work candidate states can enter zero-progress cycles.
Findings without inline diff anchors
[P2] Preserve control-bundle identity in CLI prescriptions
Invariant: an exact prescription returned by resolution must remain valid when reconstructed by the supported apply CLI. Sequence: resolve a bundle-bound transition such as installation.update → receive a prescription whose ID includes ControlBundleFingerprint → invoke boatstack apply with the returned fields → this literal omits that fingerprint → Prescription.Validate recomputes a different ID and refuses before apply. The patch added bundle identity to prescriptions without adding it to CLI reconstruction. The observable impact is that explicit resolve/apply workflows cannot execute bundle-bound transitions, although automatic one-process continuations work. A regression test should reconstruct a resolved bundle-bound prescription through buildRequest and assert that validation and apply succeed.
Confidence: 0.99
Location: boatstack/cmd/boatstack-helper/main.go:629-634 (RIGHT)
[P2] Project control bundles before validating idempotent replays
Invariant: replaying the identical admitted request after a committed transition must return its winning receipt without another effect. Sequence: resolve and commit installation.initialize or installation.update → admission projects the request's target bundle by adding the new runtime pin → receipt records that projected target → resend the identical request after losing the response → the early replay path compares the receipt against the original unprojected target and returns “different repository control-bundle target.” This patch introduced the projection and comparison but performs the first replay check before ProjectControlBundle. The caller observes failure despite a successful durable commit. A regression test should commit an installation transition, resend the same ApplyRequest and idempotency key, and assert Replayed=true with zero additional effects.
Confidence: 0.98
Location: boatstack/internal/softwaredelivery/engine/engine.go:330-337 (RIGHT)
| func (e WorkEvidence) Validate() error { | ||
| if e.SchemaVersion != WorkEvidenceSchemaVersion || e.RequestID == "" || !validSHA256(e.RequestFingerprint) || | ||
| !validSHA256(e.ResultFingerprint) || e.ContractID == "" || !validSHA256(e.ContractFingerprint) || e.TransitionID == "" || | ||
| !validSHA256(e.ProgramFingerprint) || !validSHA256(e.ContextFingerprint) || e.StateRevision == 0 || e.RepositoryID == "" || e.WorktreeID == "" || len(e.Outputs) == 0 { | ||
| return fmt.Errorf("foreground work evidence has incomplete identity") |
There was a problem hiding this comment.
[P2] Allow completed work with no optional outputs
Invariant: a valid foreground-work contract must be completable when every declared output is optional and none is produced. Sequence: compile a contract containing only required:false outputs → verifyOutputs correctly returns an empty list → SealWorkEvidence calls this validation → len(e.Outputs)==0 deterministically refuses completion → the transition remains blocked without any state progress. The patch permits contracts with no required outputs but newly requires every evidence result to contain an output. The smallest regression test should complete an all-optional contract with an empty staging directory and verify that valid, current evidence is produced.
Confidence: 0.99
| actor := authorityActor(admission) | ||
| artifact := approvalArtifact{SchemaVersion: 1, DeliveryID: deliveryID, PlanFingerprint: manifest.PlanFingerprint, PackageFingerprint: manifest.Fingerprint, Actor: actor, AdmissionID: admission.ID, ApprovedAt: admission.IssuedAt.UTC()} |
There was a problem hiding this comment.
[P2] Attribute package approval to the approving authority
Invariant: a durable approval artifact's actor must identify the authority that satisfied the approval rule. Sequence: an autonomy delegation from Alice remains in the authority bundle → human-only approval policy is satisfied by Bob's additional human receipt → authorityActor selects the first canonical receipt, which can be Alice's delegation-* receipt → approval.json records Alice as the approver. This new planning-package path derives Actor from arbitrary bundled authority rather than the human receipt required by policy. The observable audit fact is false even though admission itself was legal. A regression test should approve with differently named autonomy and human subjects under human-only policy and assert that the artifact names the human subject.
Confidence: 0.9
1 participant
Summary
Repository Flows can now require bounded human or agent work without granting that work effect authority. The runtime stages declared outputs, validates exact evidence, resumes the same run, and still admits repository mutations only through trusted transitions.
Validation
go test ./...go vet ./...go build ./...npm run test:flow-sdknpm run docs:checkpython3 -m unittest discover -s .github/tests -p 'test_*.py' -vpython3 .github/scripts/release_notes.py check-policy --repo . --base origin/main --head HEADgit diff --check