Add named human identity roles to Control Programs - #227
Conversation
There was a problem hiding this comment.
Codex automated review
Verdict: patch is incorrect
Confidence: 0.98
The new prior-role rule creates a concrete liveness failure in the documented fresh-install path. Model-level verification is recommended after correction, specifically to confirm every newly reachable roleless admitted-program state has a path to a role-bound Flow.
|
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. |
|
Review cycle 1 complete at
Verified with the focused identity/runtime tests, all 550 isolated Go tests, |
There was a problem hiding this comment.
Codex automated review
Verdict: patch is incorrect
Confidence: 0.96
The patch introduces two concrete authority/recovery failures and a role-preservation TOCTOU. Model-level verification is recommended before merge, particularly to check whether any other newly reachable configuration-drift state becomes blocking and whether any other pre-admission identity receipt can become effective after program reconciliation.
Findings without inline diff anchors
[P1] Reject delegation authorization before the selected role is admitted
Invariant: a candidate program may request a role, but that role cannot authorize product delegation until the candidate fingerprint and role are durably admitted. Starting from admitted program P0/role R0 with another configured role R1, commit candidate P1 selecting R1 and invoke flow authorize directly: delegation projection reaches this line and constructs an R1-bound request even though state still admits R0. The command can store an active P1 record; after R0 approves installation.reconcile-update, the record exactly matches P1 and supplies delegated authority without a fresh post-admission authorization. This is introduced by selecting the candidate program's role here. The impact is retroactive reuse of pre-admission authority. A regression should attempt direct authorization before reconciliation, admit P1, and verify that product resolution still requires a fresh R1 delegation.
Confidence: 0.93
Location: boatstack/cmd/boatstack-helper/flow_runtime.go:268-280 (RIGHT)
|
Review cycle 2 complete at
Bounded Locus analysis treated stale recovery as blocking before the repair and nonblocking afterward, and identified the split-read swap as an uncontrollable transition into the forbidden missing-role state; the single-snapshot design has no such violation. Repository verification passed all 552 isolated Go tests, |
There was a problem hiding this comment.
Codex automated review
Verdict: patch is incorrect
Confidence: 0.98
The role binding itself is generally fail-closed, but the patch breaks base-to-head migration for durable state, configuration, delegation, and pending input artifacts. Model-level verification is recommended after fixing these paths: verify every role/configuration/program drift state retains an ordered recovery transition, while obsolete authority facts remain unusable without poisoning fresh progress.
Findings without inline diff anchors
[P1] Preserve readability of the immediately preceding durable state schema
Invariant: a new runtime must read durable state written by the base runtime before performing any effects. A base installation writes schema 6; head changes the current schema to 7 but still recognizes only schema 4 as its predecessor. Thus base state → head command/update → DecodeState rejects schema 6 → state remains unchanged and every retry fails. This patch introduces the incompatibility and makes existing repositories unable to update or recover. A regression test should encode state using the actual base revision, then decode and transition it with head while preserving rollback bytes until commit.
Confidence: 0.99
Location: boatstack/internal/softwaredelivery/durable/state.go:17-18 (RIGHT)
[P1] Provide an update path from schema-4 project configuration
Invariant: the supported updater must either accept or transactionally migrate configuration written by the base runtime. A base repository has schema-4 configuration; head's update first resolves current human identity, whose strict validation now requires schema 5, so install.sh update → identity binding → configuration rejection → no installation transition or launcher update. Repeating cannot progress. The patch changes the accepted schema without adding migration to the production update path. A regression test should initialize with the actual base binary and run the head updater successfully through configuration and runtime migration.
Confidence: 0.98
Location: boatstack/internal/softwaredelivery/protocol/config.go:160-161 (RIGHT)
[P2] Allow prior delegation records to be safely superseded
Invariant: invalidated historical authority must not be reused, but it must not prevent fresh authorization. A base run with a schema-3 delegation record → head reconstructs a role-bound request → Load rejects the existing record solely because head requires schema 4 → bind/authorize/revoke all return the same error → the run cannot acquire fresh authority without deleting runtime state manually. The schema bump introduces this zero-progress path. A regression test should retain a base schema-3 record, verify it grants no head authority, then authorize a fresh schema-4 request through the governed command path.
Confidence: 0.94
Location: boatstack/internal/softwaredelivery/delegation/record.go:21-22 (RIGHT)
[P2] Do not let old input requests poison new request generation
Invariant: a stale input request must be rejected as authority while allowing a fresh request for the current context. A base run suspended with a schema-3 request → head resolves the same transition → LatestRequest validates the old file before checking whether it matches the new role-bound context → validation requires schema 4 and aborts → no new request is emitted and every retry repeats the failure. The revision bump introduces this blocking path. A regression test should create a pending request with the base binary, then verify head ignores it as authority and emits a new schema-4 request.
Confidence: 0.95
Location: boatstack/invocation/invocation.go:20-24 (RIGHT)
Summary
Breaking changes
This intentionally bumps the affected project, Control Program, artifact, compiler, delegation, surface, durable-state, and invocation schemas. No compatibility identity reader is included.
Verification
npm run test:flow-sdknpm run docs:checkpython3 .github/scripts/run_go_tests.py— 549 testsgo test -race ./...,go vet ./...,go build ./...fromboatstack/git diff --check origin/main...HEADConsumer proof status
Actual Cursor Computer Use reached the role-bound delegation boundary and executed the configured
gh api user --jq .loginidentity descriptor twice. GitHub returned HTTP 503, and the agent correctly stopped and requested an explicit fallback without inferring an actor. The consumer run is paused and is not part of this PR.