Make Flow control law repository-owned - #212
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.99
The patch makes repository-owned targets unusable through required core transitions, permits caller-selected trusted reducer semantics, and breaks all stated mixed-version schemas without migration. Model-level verification is recommended before merge: after fixing target/class projection, verify that every accepted repository target retains prerequisite and recovery progress across core, runtime, and extension transitions.
Findings without inline diff anchors
[P1] Carry the target-to-trusted-class binding into the kernel
Invariant: a repository target must have one kernel-owned trusted class used consistently by core selection, admission, and reducers. The adapter remaps only runtime transitions. Witness 1: a Flow entry targeting published-pr produces TargetID=published-pr → core objective.bind still supports only the five legacy IDs → resolution refuses, so the new Flow cannot bind its objective. Witness 2: a standard RPC can instead submit TargetID=open-or-updated-pr, TrustedClass=approved-plan → validation accepts it → plan.approve commits TERMINAL although the open-PR contract is false → normal resolution has no progress transition from TERMINAL. The patch introduces both failures by discarding the computed target/class mapping and trusting request-carried TrustedClass. Carry and fingerprint this mapping in the compiled program, project core transitions through it, and reject mismatched classes before admission. Regression tests should resolve objective.bind for published-pr, and reject the mismatched RPC objective before any state effect.
Confidence: 0.99
Location: boatstack/flow/softwaredelivery/definition.go:88-93 (RIGHT)
There was a problem hiding this comment.
Codex automated review
Verdict: patch is incorrect
Confidence: 0.99
The patch introduces blocking target/class composition failures and breaks upgrades from the base durable schemas. Model-level verification is recommended after correction: verify every repository target retains prerequisite core transitions and that no strengthened target can produce terminal durable state while its runtime contract remains false.
Findings without inline diff anchors
[P1] Reject strengthened targets that contradict terminal postconditions
Invariant: a committed terminal transition must satisfy the compiled runtime terminal contract. Replacing exact contract matching with conditionsStrengthen accepts, for example, target open-or-updated-pr with the trusted conditions plus terminal=nonterminal. After publication.observe sees an open PR, the reducer commits terminal=established and phase TERMINAL; the strengthened contract is false and no normal progress transition is selectable, yielding a durable zero-progress state. The prior exact-match check rejected this program. Add a negative compilation test for this contradictory strengthening, or an execution test proving no terminal fact is committed unless the complete strengthened contract holds.
Confidence: 0.96
Location: boatstack/flow/softwaredelivery/definition.go:167-169 (RIGHT)
| } | ||
| if !g.Kind.Valid() { | ||
| return fmt.Errorf("objective: invalid kind %q", g.Kind) | ||
| if !g.TargetID.Valid() || (g.TrustedClass != "" && !g.TrustedClass.Valid()) { |
There was a problem hiding this comment.
[P1] Validate trusted classes against the compiled target mapping
Invariant: callers may request a target but may not self-assign its kernel-trusted semantic class. Objective.Validate accepts any syntactically valid TrustedClass without checking the compiled target-to-class mapping. For example, an RPC request can bind target open-or-updated-pr with trusted class approved-plan; plan.approve then establishes durable terminal state while the open-PR contract remains unsatisfied, and subsequent resolution is stuck in terminal phase. This field and syntax-only validation are introduced here. Reject mismatched target/class pairs before objective.bind, with a regression test asserting zero state changes for such an RPC request.
Confidence: 0.98
| ) | ||
|
|
||
| const StateSchemaVersion = 3 | ||
| const StateSchemaVersion = 4 |
There was a problem hiding this comment.
[P1] Provide an upgrade path for existing durable state
Invariant: activating an admitted runtime update must leave the repository readable by that runtime. Existing repositories contain schema-3 state with objective.kind; the head runtime requires schema 4 and strictly decodes unknown fields, so after activation observation fails before any recovery or update transition can run. The same patch also strictly bumps journal and receipt schemas, preventing historical committed records from being scanned. No migration or old-schema reader is added; the new test merely asserts rejection. Add a fixture produced by the base revision, update it with the head runtime, and verify state, journals, objective identity, and receipts remain usable.
Confidence: 0.99
There was a problem hiding this comment.
Codex automated review
Verdict: patch is incorrect
Confidence: 0.98
Repository-owned target identities are not translated across the extension composition boundary, causing concrete compilation failures and unreachable extension transitions. Model-level verification is recommended before merge: verify that every extension constraint and transition remains reachable for each repository target mapped to its trusted class, including multiple targets sharing one class.
| compiledExtensions = append(compiledExtensions, compiledExtension{manifest: manifest, identity: identity, runtime: runtime}) | ||
| for _, constraint := range manifest.ObjectiveConstraints { | ||
| extensionConditions[constraint.ObjectiveKind] = append(extensionConditions[constraint.ObjectiveKind], constraint.Conditions...) | ||
| extensionConditions[constraint.TargetID] = append(extensionConditions[constraint.TargetID], constraint.Conditions...) |
There was a problem hiding this comment.
[P1] Map extension objectives through repository target identities
Invariant: extensions targeting a trusted objective class must compose with repository-owned targets mapped to that class. A Flow target published-pr mapped to open-or-updated-pr plus releasenote.Definition() records extension conditions under open-or-updated-pr, while the Flow contract is keyed by published-pr; compilation then fails with extension constrains unsupported objective, so every command for that repository is blocked. This patch introduced the mismatch by preserving repository target IDs while leaving extension constraints and transition filters keyed directly by trusted-class IDs. The smallest regression test should compile product-delivery-a.flow.ts with the release-note extension, assert its obligation is attached to published-pr, and verify the extension transition supports an objective with target published-pr and trusted class open-or-updated-pr; all targets sharing that trusted class should behave likewise.
Confidence: 0.99
1 participant
Summary
Verification
npm run test:flow-sdkcd boatstack && go build ./... && go vet ./...python3 .github/scripts/run_go_tests.py(324 tests)python3 -m unittest discover -s .github/tests -p 'test_*.py'(36 tests, 1 expected failure)\n-python3 .github/scripts/release_notes.py validate --repo .\n-git diff --check