Make fresh Flow runs self-initializing - #223
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.92
The fresh-run path itself composes correctly, but changing checksum-bound generated skills without a compatible migration path makes existing delegated Flow repositories fail validation before they can update. Model-level verification is not required before addressing this concrete migration defect.
| if entry.Delegation != nil { | ||
| startCommand = fmt.Sprintf("boatstack flow run --repo . --flow %s --entry %s --repository-authority --host %s --format json", compiled.Document.Program.ID, entry.ID, host) | ||
| } |
There was a problem hiding this comment.
[P1] Provide an upgrade path for existing generated Flow artifacts
Invariant: a candidate runtime must accept or atomically migrate artifacts produced by the admitted runtime before requiring candidate-generated projections. A repository with a delegated Flow compiled at the base revision has control-program.compiler.4 artifacts and skill digests containing the old boatstack next command. Running the head candidate regenerates different skill bytes here while retaining the same compiler version, so CheckArtifact returns CONTROL_PROGRAM_STALE before resolution or installation.update; recompiling with head first conversely makes the active base runtime reject the repository. This patch therefore blocks the governed runtime-update path for existing delegated Flow repositories. Add a regression test that creates the fixture with the actual base generator, then verifies the head runtime can perform the update/migration without an unmanaged incompatible intermediate state.
Confidence: 0.94
There was a problem hiding this comment.
Codex automated review
Verdict: patch is incorrect
Confidence: 0.99
The new prerequisite ordering has two concrete liveness failures: the normal generated command cannot authorize initialization, and an authority-bearing invocation installs an uncommitted bundle that later fails workspace admission. Model-level verification is recommended before merge, particularly to check that every new pre-delegation prerequisite has an actionable authority path and that installation effects compose with revision-bound workspace transitions.
|
Review cycle 1 is settled at d58ee1c. Both P1 threads were fixed, answered individually, and resolved. The canonical local Boatstack review is READY; affected Go suites and the exact fake-provider product-delivery Flow pass. This head is now frozen for the next independent review cycle. |
There was a problem hiding this comment.
Codex automated review
Verdict: patch is incorrect
Confidence: 0.98
The new revision guarantee is bypassable through the public kernel/SDK path and remains vulnerable to a helper-to-effect TOCTOU race. This is a concrete boundary defect, so model-level verification is not required before fixing it; afterward, model-level verification is not specifically recommended.
There was a problem hiding this comment.
Codex automated review
Verdict: patch is incorrect
Confidence: 0.97
The new commit boundary can shadow mandatory recovery, rejects valid idempotent replays, and remains vulnerable to revision drift between its final check and effect execution. Model-level verification is recommended before merge, particularly to ask whether any other pending-journal or maintenance states are shadowed by the new pre-resolution suspension.
| if options.transitionID == "" || !installationAuthority { | ||
| acceptedBundleRevision, err = requireCommittedAcceptedFlowBundle(ctx, repository, options, bundle.Source, bundleFingerprint) | ||
| if err != nil { | ||
| return commandOptions{}, err |
There was a problem hiding this comment.
[P1] Allow interrupted installation to reach recovery before enforcing commit
Invariant: a pending transaction must reach its prescribed recovery transition without requiring the transaction's partial effects to be committed. If installation.initialize installs its target pin, generated bundle, and final state mutation but the process stops before journal commit, the state fingerprint matches the dirty target bundle while a pending journal requires recovery. Restarting the Flow enters this unconditional guard, VerifyControlBundleRevision fails, and returns CONTROL_BUNDLE_COMMIT_REQUIRED before the controller can observe the journal and select recovery.resume or recovery.rollback; every retry repeats the suspension. This patch introduces that pre-resolution guard, forcing users to commit unverified, rollbackable effects or leaving recovery blocked. A regression test should interrupt initialization after PreparedEffect.Execute but before journal commit, restart the same Flow, and assert that recovery is selected without a commit suspension and rollback can restore the bundle.
Confidence: 0.98
| if request.ControlBundleRevision != "" { | ||
| layout, _, layoutErr := k.resolver.ResolveLayout(ctx, invocation) | ||
| if layoutErr != nil { | ||
| response.Error = layoutErr.Error() | ||
| return response, layoutErr | ||
| } | ||
| if revisionErr := boatstackruntime.VerifyCurrentControlBundleRevision(ctx, layout.RepositoryRoot, request.ControlBundleRevision, request.ControlBundle.Source); revisionErr != nil { | ||
| response.Error = revisionErr.Error() | ||
| return response, revisionErr |
There was a problem hiding this comment.
[P2] Check idempotency receipts before rejecting revision drift
Invariant: replaying an already committed idempotent request must return the committed fact without re-admitting current repository authority. Apply a revision-bound transition at commit A, advance HEAD to commit B while retaining identical bundle files, then resend the exact request with its idempotency key and ControlBundleRevision=A. This new controller-level check rejects the request before Engine.Apply can find and replay the receipt, producing a false failure despite no effect being required. The observable result is an ambiguous failed retry of a successfully committed transition. A regression test should commit a transition, create an empty Git commit, resend the identical request, and assert Replayed=true, the same receipt, and zero new effects.
Confidence: 0.96
| // Prepare is the final side-effect-free plant preflight. Run it after the | ||
| // locked observation but before receipt binding or transaction state so a | ||
| // changed external boundary leaves no managed mutation to recover. | ||
| prepared, err := e.effects.Prepare(ctx, admission, transition) | ||
| if err != nil { | ||
| return result, err | ||
| } |
There was a problem hiding this comment.
[P1] Close the race between final revision verification and effect execution
Invariant: if the admitted control-bundle revision changes before the first transition effect, the transition must perform zero effects. The added revision check runs during Prepare, but after it succeeds the engine writes journal state and later calls PreparedEffect.Execute; neither the repository ref nor Git writers are locked across that interval. A concurrent commit/reset can therefore move HEAD from admitted revision A to B after preparation, after which a state-only or program effect executes and can commit a receipt whose admission binds A even though the effect ran at B. This patch introduces the exact-revision claim without making its final check atomic with execution. A regression test should wrap a prepared effect so that it moves HEAD immediately before delegating Execute, then assert refusal with no journal, state mutation, command, or receipt.
Confidence: 0.93
1 participant
Problem
A fresh repository Flow requested product delegation before installation had established the exact control bundle. Generated skills also entered through a path that expected initialization to have happened separately.
Change
Verification
python3 .github/scripts/run_go_tests.py— 489 tests passedgo test ./cmd/boatstack-helper ./flow/softwaredeliverygo build ./...go vet ./...python3 -m unittest discover -s .github/tests -p 'test_*.py' -v— 54 tests passed, one expected failureRelease note:
release-notes/2026-08-16-canonical-flow-initialization.md