Resolve foreground-work inputs from committed producer results - #233
Conversation
![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.98
The change introduces two unsupported durable-schema boundaries and a zero-progress path after program reconciliation. Model-level verification is recommended before merge, particularly to check whether missing current-program producer receipts can block consumers and whether every dependency retains a route to refresh its producer after update or recovery.
|
Review cycle 1 settled at commit 5eb85a2. The zero-progress path is removed by resolving committed Work inputs only after transition selection. The schema-1 package and schema-3 foreground-record requests remain intentionally unsupported under the pre-alpha breaking-change boundary. Verification passed: Flow SDK, docs, 63 repository Python tests, 599 isolated Go tests, go test -race, go vet, go build, release-note preflight, and git diff --check. |
There was a problem hiding this comment.
Codex automated review
Verdict: patch is incorrect
Confidence: 0.98
The change introduces a durable-history compatibility break, accepts unverified producer provenance at the kernel surface, and admits a concrete resolver/materializer zero-progress cycle. Model-level verification is recommended before merge, focused on whether every accepted foreground-work dependency retains a reachable committed producer across initially satisfied targets, repeated transitions, program updates, and execution-context advances.
|
Review cycle 2 settled at 1a4b040.
Local verification passed: 32 Flow SDK tests, documentation checks, 63 Python tests, 601 isolated Go tests, go test -race ./..., go vet ./..., go build ./..., release-note validation/preflight, and git diff --check. |
There was a problem hiding this comment.
Codex automated review
Verdict: patch is incorrect
Confidence: 0.99
The change introduces durable-version incompatibility and two concrete zero-progress paths in foreground-work dependency resolution. Model-level verification is recommended before merge, particularly to check that every missing-producer state has an admissible prerequisite transition and cannot be shadowed by normal selection.
| ReceiptID: committed.Receipt.ID, TransitionID: committed.Receipt.TransitionID, WorkID: committed.Work.ContractID, OutputID: committed.Output.ID, | ||
| ResultFingerprint: committed.Work.ResultFingerprint, ContractFingerprint: committed.Work.ContractFingerprint, OutputSHA256: committed.Output.SHA256, | ||
| } | ||
| values[foregroundWorkInputKey(work.ID, input.ID)] = protocol.WorkInputValue{Value: committed.Output.Content, Fingerprint: committed.Output.SHA256, WorkOutput: &provenance} |
There was a problem hiding this comment.
[P2] Valid empty producer outputs cannot be consumed
Invariant: every required output accepted and committed under a work contract must be usable by a declared downstream work input. The foreground-work verifier permits a required text output to be a zero-byte UTF-8 file and commits its SHA-256 and empty content. This new construction then places that empty content in WorkInputValue, whose validator requires Value != ""; the consumer request is therefore rejected on every retry even though the exact producer receipt exists. The patch introduces the failure by reusing a locator-oriented input value type for output content without reconciling its non-empty constraint. The observable result is a permanently blocked consumer. A regression test should commit an empty required text output and resolve a consumer bound to it, expecting the binding to validate successfully (or the producer completion to reject emptiness consistently).
Confidence: 0.99
|
P1 convergence pass settled at 0b19c60.
The P2 empty-output observation is recorded but not absorbed after the two requested full review cycles. |
There was a problem hiding this comment.
Codex automated review
Verdict: patch is incorrect
Confidence: 0.98
The patch introduces an upgrade-breaking durable-history incompatibility and admits a concrete zero-progress dependency route. Model-level verification is recommended before merge, particularly to check whether every new producer redirect remains source-admissible across phases and whether any other dependency state can cycle without a committed producer fact.
|
P1 convergence repair is published at 88a3ff4.
Local verification passed: 32 Flow SDK tests, documentation checks, 63 Python tests, 602 isolated Go tests, go test -race ./..., go vet ./..., go build ./..., release-note preflight, and git diff --check. |
There was a problem hiding this comment.
Codex automated review
Verdict: patch is incorrect
Confidence: 0.97
The patch permits stale work-output reuse across objective-evidence changes and accepts a concrete producer/consumer relation that cannot progress for some objectives. Model-level verification is recommended before merge. Questions: Can any other trusted objective projection leave a required producer unreachable? Can objective rebinding or alternate target-establishing transitions make a previously committed producer result stale?
| func sameObjectiveIdentity(left, right model.Objective) bool { | ||
| return left.ID == right.ID && left.TargetID == right.TargetID && left.TrustedClass == right.TrustedClass && left.DeliveryID == right.DeliveryID && left.FrontierIsStop == right.FrontierIsStop |
There was a problem hiding this comment.
[P1] Bind committed work outputs to the complete objective
Invariant: committed work output must remain bound to the exact objective evidence under which it was admitted. Minimal failure: objective O with EvidenceFingerprint A → producer commits output → objective.bind changes only EvidenceFingerprint to B → consumer resolution accepts the A output because sameObjectiveIdentity ignores that field → downstream work is admitted under B using stale evidence. This comparison is introduced by the patch and is used by both helper resolution and controller verification. The impact is cross-revision reuse of foreground-work facts after objective evidence changes. Propagate EvidenceFingerprint through boundFlowObjective and add a selector test where admission and selector differ only in EvidenceFingerprint; resolution must reject it.
Confidence: 0.98
| for _, consumer := range transitionsByWork[contract.ID] { | ||
| if producer.ID == consumer.ID || producer.Priority >= consumer.Priority || !predicateImplies(consumer.Guard, producer.Target) { | ||
| return invalid(field, "work output is not guaranteed before the consuming foreground work") |
There was a problem hiding this comment.
[P1] Reject work dependencies whose producer cannot serve the consumer objective
Invariant: every objective that can select a work-output consumer must also admit its required producer transition. The new check proves only predicate and priority ordering. A valid program can attach producer work to plan.abandon (supported only for safely-abandoned) and dependent consumer work to publication.observe (supported for published-pr); compilation and runtime-manifest construction accept both. During a published-pr run, publication.observe is selected → missing output redirects to plan.abandon → targeted resolution refuses plan.abandon for that objective → unchanged state selects publication.observe again, producing a permanent zero-progress path. This is introduced by the new dependency validation and prerequisite redirection. After trusted TargetIDs are projected, require the producer's target set to cover every consumer target set. A regression test can extend the existing abandonment-entry fixture with this dependency and require RuntimeManifest to reject it.
Confidence: 0.96
1 participant
Summary
Verification
npm run test:flow-sdknpm run docs:checkpython3 -m unittest discover -s .github/tests -p 'test_*.py'python3 .github/scripts/run_go_tests.pygo test -race ./...,go vet ./..., andgo build ./...fromboatstack/git diff --checkapproved-plan; two downstream Work contracts sealed identical committed producer provenance while producing independent results