feat: add repository-owned run delegation - #213
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.98
The patch introduces upgrade-breaking state/program migrations, an unrecoverable expiry path, and a post-target authority window. Model-level verification is recommended before merge, focused on delegation lifecycle under interruption and whether every terminal commit durably closes its grant.
Findings without inline diff anchors
[P2] Provide compatibility for existing control-program artifacts
Invariant: a runtime upgrade must either execute or explicitly migrate the prior supported program ABI. Base artifacts contain schema_version and compiler identity control-program/v1.compiler.1; the new strict decoder removes that field, keeps the replacement at revision 1, and requires a renamed compiler identity. Consequently every existing repository artifact fails before its Flow can resolve, and its old TypeScript source also uses the removed SDK shape. This incompatibility is introduced without a migration path or actual old-version fixture. Impact: repository-authored control programs stop running after upgrade. A regression test should load a genuine base-produced artifact/source under the head runtime and verify supported migration or recompilation preserves its executable semantics and fingerprint contract.
Confidence: 0.94
Location: boatstack/controlprogram/artifact.go:21-24 (RIGHT)
There was a problem hiding this comment.
Codex automated review
Verdict: patch is incorrect
Confidence: 0.98
The patch introduces concrete update/migration failures and a zero-progress continuation path after worktree authority transfer. Model-level verification is recommended before merge: verify that delegation expiry/revocation cannot strand terminal or recovery states, and that every execution_context=advance transition preserves a reachable continuation path.
Findings without inline diff anchors
[P1] Preserve or migrate the existing durable-state root
Invariant: an update must observe the exact pre-update durable state before admitting any transition. A repository initialized at the base stores state under .git/boatstack/v2 (or <state-root>/boatstack/v2); the head instead resolves .git/boatstack and sees the old state, journals, receipts, and detached bindings as absent. The official update command then deterministically refuses because the synthesized default state makes configuration stale, while manual runtime replacement exposes a fresh controller and hides any active objective or recovery transaction. This patch removes the v2 component without migration or fallback. A regression should initialize embedded and detached repositories with the base binary, including an active/pending state, then run the head updater and verify the exact revision, objective, receipts, and recovery transaction remain visible.
Confidence: 0.99
Location: boatstack/internal/softwaredelivery/plant/resolver.go:260-263 (RIGHT)
[P2] Provide compatibility for previously compiled Flow artifacts
Invariant: an unchanged runtime version must either load existing executable artifacts or migrate them before activation. An artifact produced at the base contains schema_version and compiler control-program/v1.compiler.1; after installing the head, flow check, next, and flow run decode it into this new envelope and fail on the now-unknown field before any control resolution. The patch simultaneously rejects the old program IR and changes the frontend API, but adds no migration or version boundary, making every previously compiled repository Flow unusable. A regression should compile a real fixture with the base binary/frontend and load and execute it with the head updater/runtime.
Confidence: 0.96
Location: boatstack/controlprogram/artifact.go:21-24 (RIGHT)
There was a problem hiding this comment.
Codex automated review
Verdict: patch is incorrect
Confidence: 0.98
The new delegation boundary is reachable from repository-controlled shell execution, and the advertised continuous runner deterministically reports an error after committing its target. Model-level verification is recommended before merge to check delegation settlement for zero-progress cycles and terminal replay across interruption points.
There was a problem hiding this comment.
Codex automated review
Verdict: patch is incorrect
Confidence: 0.98
Completed-delegation handling introduces a concrete resolver/apply disagreement and can permanently strand a run. Model-level verification is recommended for whether every completed delegation remains terminal under later observations or retains a legal maintenance/recovery path.
| if record.Status == "completed" && request.Operation == surfaces.OperationResolve { | ||
| // A completed delegation carries no authority, but resolving the exact | ||
| // bound run remains safe and lets restarts replay its terminal state. | ||
| return nil, nil, nil | ||
| } | ||
| if record.Status != "active" { | ||
| releaseOnError() | ||
| return nil, nil, fmt.Errorf("DELEGATION_REVOKED: run authorization is %s", record.Status) |
There was a problem hiding this comment.
[P1] Completed delegations can prescribe transitions that apply always rejects
Invariant: the same Flow state, program, and authority must not yield a prescription that the delegation boundary deterministically refuses. After a delegated run reaches its marked target, its record becomes completed; this branch permits subsequent resolves without delegated authority, but the following status check rejects every apply or recovery. For example: workspace.abandon establishes a safely-abandoned target → delegation becomes completed → a human-authorized targeted resolve prescribes the still-admissible workspace.cleanup transition → applying that exact prescription fails with DELEGATION_REVOKED. If an observed target later regresses, the same run is likewise stranded because flow authorize cannot reactivate a completed record and its deterministic run ID cannot be replaced without changing the bound inputs. The patch introduces this resolve-only exception, causing resolver/apply disagreement and zero progress. A regression test should complete a delegation, resolve an admissible human-authorized post-target transition, and verify that its exact prescription remains applicable (plus a target-regression renewal case).
Confidence: 0.99
1 participant
Summary
flow authorize,flow revoke, and resumableflow runcommandsVerification
npm run build:flow-sdkpython3 .github/scripts/run_go_tests.py(334 tests)go build ./...go vet ./...python3 -m unittest discover -s .github/tests -p 'test_*.py'(36 tests)