Skip to content

Persist provider profile dispatch leases - #528

Merged
andrei-hasna merged 7 commits into
mainfrom
fix/a59d3086-provider-profile-leases
Aug 9, 2026
Merged

Persist provider profile dispatch leases#528
andrei-hasna merged 7 commits into
mainfrom
fix/a59d3086-provider-profile-leases

Conversation

@andrei-hasna

@andrei-hasna andrei-hasna commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

Summary

  • replace the process-local fixed-duration provider-profile lease map with a durable SQLite-backed, owner-bound lease contract in codex-state
  • select from fresh provider-backed usage data, exclude unavailable/exhausted/leased identities, and fail closed when no healthy unused identity exists
  • carry lease ownership, safe profile name, opaque account identity hash, expiry, heartbeat renewal, explicit release, and stale recovery through schedule and mailbox dispatch
  • recheck the durable lease and fresh provider health at execution start; abort the exact active turn if lease renewal is lost
  • preserve mailbox dispatch through the existing usage-profile broker

Focused regressions

  • concurrent aliases cannot lease one provider identity
  • foreign owners cannot renew, validate, or release a lease
  • heartbeat, release, replacement, and stale/crash recovery
  • fresh provider health exclusions and fail-closed behavior
  • schedule execution-start recheck and lease-heartbeat cadence
  • mailbox Delivered propagation, execution-start recheck, renewal, abort, and release
  • all synthetic test identities avoid email addresses and credential values

Validation

  • cargo fmt -- --check --config imports_granularity=Item — exit 0
  • git diff --check and git diff --cached --check — exit 0
  • six named production source-shape sites closed
  • staged shield review — no security issues found before each commit
  • exact commit-range gitleaks scans — exit 0 with the expected commit count and no leaks found
  • local heavy Rust and Bazel builds intentionally not run
  • Blacksmith Testbox run 31332675354 on the prior head found two candidate-owned SQLx executor errors; fixed in d9dda9ec78838c72f8d18ac751916c0356befdfe
  • Blacksmith Testbox run 31333052869 then reached codex-app-server and found test-only Debug plus partial-move compile errors; fixed in 8d0132414feb15317d577fe0fc475c79f359acdc
  • replacement Blacksmith Testbox run 31333760091: success on prior head 8d0132414feb15317d577fe0fc475c79f359acdc, command cd codex-rs && just test
  • required native rust-ci run 31333735933: failed argument-comment lint at three candidate-owned test call sites (usage_profile_heartbeat_interval_secs, account_id, and current_profile None arguments); BuildBuddy invocation f011b233-9989-4acd-bb6a-266443a98c76
  • those three lint-only findings are fixed in 3d8ad7878024fad4c618e48344a0ee60c68c4381 using comments only; behavior and assertions are unchanged
  • prior-head native rust-ci run 31334782921: Linux and Windows argument-comment lint jobs both succeeded
  • prior-head Bazel run 31334782901: Linux Clippy found three R5 production findings (unnecessary_map_or and expect_used in thread_mailbox_dispatcher_runtime.rs, map_identity in usage_profile_broker.rs); BuildBuddy invocation 90baaf56-1dbf-4df9-b13e-8ca9a57b6284
  • those three Clippy findings are fixed narrowly in 4d7fd30bb5f6ea14262cbf412952d6535191c80c: exact is_none_or equivalence, ownership-safe fail-closed let-else, and identity-map removal
  • prior-head Blacksmith full gate 31335319686: success on 3d8ad7878024fad4c618e48344a0ee60c68c4381, command cd codex-rs && just test
  • our pre-push staged Shield result was No security issues found in staged changes.; the exact 3d8ad7878024fad4c618e48344a0ee60c68c4381..4d7fd30bb5f6ea14262cbf412952d6535191c80c Gitleaks range was non-vacuous (1 commits scanned.) and returned no leaks found
  • after our single replacement push, Trebius externally fast-forwarded the branch with a lease-loss queued-work repair and its rustfmt-only follow-up; the supplemental exact range 4d7fd30bb5f6ea14262cbf412952d6535191c80c..bae61b1418b9069145b82303c9f0c1d5929266f9 is non-vacuous (2 commits scanned.) and Gitleaks returned no leaks found
  • intermediate exact-head CI 31337030236 failed only Rust formatting in codex-rs/core/src/tasks/mod.rs; Trebius repaired that defect in bae61b1418b9069145b82303c9f0c1d5929266f9
  • exact current-head ci 31337152058, blob-size 31337152020, Codespell 31337152025, cargo-deny 31337152039, and CLA 31337150815 are successful
  • exact current-head Bazel 31337152017, rust-ci 31337152047, sdk 31337152051, and codewith-cli 31337152066 remain in progress
  • parent-head Blacksmith full gate 31336630533 remains in progress on 4d7fd30bb5f6ea14262cbf412952d6535191c80c; it will not be superseded, and an exact-current-head full Testbox is still required afterward because the external commit changed shared core
  • the compile/gate repairs occurred before Erdos's initial review and therefore do not consume bounded post-NO_GO remediation cycles; Erdos has issued no verdict
  • Trebius posted review comments and a review-fix outside the fixed-reviewer sequence; those comments do not replace Erdos and do not consume an Erdos remediation cycle

Candidate

  • head: bae61b1418b9069145b82303c9f0c1d5929266f9
  • tree: 67aaad84e6fe0b50b582d46aeb7a44893dd7dc4e
  • Todos task: a59d3086-a01e-4152-b315-aee3ca2e470f

Residual risk

  • P2: released and expired lease rows are retained for provenance without age-based pruning, producing one metadata-only row per claim.

Executor handoff

  • PR fix(monitors): fence persistent command authorization #527 overlaps this PR in codex-rs/core/src/codex_thread.rs and codex-rs/state/src/lib.rs and was merged externally at head d32cdc9990df9764e20c20315dda6face8749dad / merge 6730226823f72b43331b0b8b0e8b750706b03610 on 2026-08-09T21:11:19Z; the R5 executor did not merge it.
  • Because main moved through the overlapping PR, resolve live origin/main from the branch ref and recompute the actual merge tree before Erdos review; do not reuse the prior 11a0f23d tree or a PR baseRefOid snapshot.
  • Do not merge without Erdos GO; no reviewer verdict exists yet.

Production Package Arrivals was not enabled or modified by this worker. This PR must not be merged by this worker.

@andrei-hasna

Copy link
Copy Markdown
Contributor Author

[REVIEW] NO_GO — #528 @ 8d01324 — lens: correctness+security+gates, reviewer Trebius (1 of 1)

Reviewed the exact candidate against the supplied freshly fetched base 4be2515b784b7568e4a588f110badc02048e2d87. I read the complete diff for all 11 changed files and the surrounding mailbox dispatch, schedule dispatch, turn lifecycle, pending-work restart, and state-runtime code.

Commands and declared gates:

  • git log --oneline origin/main..HEAD — exit 0; three candidate commits read.
  • git diff origin/main...HEAD --stat — exit 0; 11 files, 1,726 insertions and 317 deletions.
  • bun install — exit 0; setup only, 521 packages installed; pass/fail count not applicable. The generated untracked bun.lock was removed so the review worktree remains clean.
  • package.json declares no typecheck script.
  • package.json declares no test script, so this repository declares no test gate. I did not invent a replacement command.
  • gh pr view 528 --repo hasna/codewith --json statusCheckRollup with transcript-safe nested-field projection — exit 0; 8 success, 0 failure, 3 skipped, 15 in progress.

Forge checks read by name and conclusion:

  • Bazel test on ubuntu-24.04 for x86_64-unknown-linux-gnu — IN_PROGRESS
  • Build Codewith Linux CLI — IN_PROGRESS
  • cla — SUCCESS
  • Check for spelling errors — SUCCESS
  • Blob size policy — SUCCESS
  • cargo-deny — SUCCESS
  • build-test — SUCCESS
  • Detect changed areas — SUCCESS
  • python-sdk — SUCCESS
  • Bazel test on ubuntu-24.04 for x86_64-unknown-linux-musl — IN_PROGRESS
  • Bazel test on windows-latest for x86_64-pc-windows-gnullvm shard 1/4 — IN_PROGRESS
  • Format / etc — IN_PROGRESS
  • sdks — IN_PROGRESS
  • Bazel test on windows-latest for x86_64-pc-windows-gnullvm shard 2/4 — IN_PROGRESS
  • Bazel test on windows-latest for x86_64-pc-windows-gnullvm shard 3/4 — IN_PROGRESS
  • Bazel test on windows-latest for x86_64-pc-windows-gnullvm shard 4/4 — IN_PROGRESS
  • Bazel test on windows-latest for x86_64-pc-windows-gnullvm (native main) — SKIPPED
  • cargo shear — SUCCESS
  • Bazel clippy on ubuntu-24.04 for x86_64-unknown-linux-gnu — IN_PROGRESS
  • Argument comment lint package — SKIPPED
  • Bazel clippy on windows-latest for x86_64-pc-windows-gnullvm — IN_PROGRESS
  • Verify release build on ubuntu-24.04 for x86_64-unknown-linux-gnu — IN_PROGRESS
  • Argument comment lint - Linux — IN_PROGRESS
  • Verify release build on windows-latest for x86_64-pc-windows-gnullvm — IN_PROGRESS
  • Argument comment lint - Windows — IN_PROGRESS
  • [code]smith — SKIPPED

Blocking P0/P1 findings:

  • P1 — queued mailbox turns can continue on a provider account after the durable profile lease is released. thread_mailbox_dispatcher_runtime.rs:463-546 acquires/carries a usage-profile lease only on the not-loaded resume path; a trigger-turn message delivered to an already active target carries no lease. When a leased mailbox turn reaches a terminal event, thread_lifecycle.rs:342-433 removes and releases its lease. Independently, core turn completion checks queued trigger-turn mail and calls maybe_start_turn_for_pending_work() at core/src/tasks/mod.rs:911-912. That successor turn is started directly, has no UsageProfileTurnLease registration, and has no lease heartbeat. This is reachable whenever another trigger-turn message arrives while the first leased mailbox turn is running. After the first terminal event releases the identity lease, another schedule or mailbox dispatcher can reserve that same provider account while the unleased successor is still running, defeating the PR's cross-lane account-exclusivity guarantee. The same bypass already exists for an initially loaded idle mailbox target, because the active-peer path never invokes the broker. The fix needs the loaded-target and automatic successor-turn paths to acquire, bind, heartbeat, and release a durable identity lease before model work starts, with a regression covering two queued trigger turns and a competing alias claim.

Non-blocking follow-ups: none.

The forge checks were not yet terminal when read. This NO_GO is based on the concrete P1 above, not on an invented local gate or an in-progress check.

@andrei-hasna

Copy link
Copy Markdown
Contributor Author

[REVIEW] NO_GO — #528 @ 4d7fd30 — lens: correctness+security+gates, reviewer Trebius (1 of 1)

Reviewed

  • Exact candidate: HEAD 4d7fd30 against fetched origin/main 6730226.
  • Read the full diff and surrounding lease, scheduler, mailbox-dispatch, thread-lifecycle, abort, migration, and state-runtime code across all 11 changed files.
  • package.json declares no typecheck script.
  • package.json declares no test gate. No undeclared local Rust command is treated as a repository gate.

Commands and gates

  • bun install — exit 0 — setup only; 521 packages installed; no pass/fail test count.
  • gh pr view 528 --repo hasna/codewith --json statusCheckRollup with a value-safe projection — exit 0.
  • Bazel test on ubuntu-24.04 for x86_64-unknown-linux-gnu — IN_PROGRESS, no conclusion.
  • Build Codewith Linux CLI — IN_PROGRESS, no conclusion.
  • cla — SUCCESS.
  • Check for spelling errors — SUCCESS.
  • Blob size policy — SUCCESS.
  • cargo-deny — SUCCESS.
  • build-test — SUCCESS.
  • Detect changed areas — SUCCESS.
  • python-sdk — SUCCESS.
  • Bazel test on ubuntu-24.04 for x86_64-unknown-linux-musl — IN_PROGRESS, no conclusion.
  • sdks — IN_PROGRESS, no conclusion.
  • Bazel test on windows-latest for x86_64-pc-windows-gnullvm shard 1/4 — SUCCESS.
  • Format / etc — SUCCESS.
  • Bazel test on windows-latest for x86_64-pc-windows-gnullvm shard 2/4 — SUCCESS.
  • Bazel test on windows-latest for x86_64-pc-windows-gnullvm shard 3/4 — SUCCESS.
  • Bazel test on windows-latest for x86_64-pc-windows-gnullvm shard 4/4 — SUCCESS.
  • Bazel test on windows-latest for x86_64-pc-windows-gnullvm (native main) — SKIPPED.
  • cargo shear — SUCCESS.
  • Bazel clippy on ubuntu-24.04 for x86_64-unknown-linux-gnu — IN_PROGRESS, no conclusion.
  • Argument comment lint package — SKIPPED.
  • Bazel clippy on windows-latest for x86_64-pc-windows-gnullvm — SUCCESS.
  • Verify release build on ubuntu-24.04 for x86_64-unknown-linux-gnu — IN_PROGRESS, no conclusion.
  • Argument comment lint - Linux — SUCCESS.
  • Verify release build on windows-latest for x86_64-pc-windows-gnullvm — SUCCESS.
  • Argument comment lint - Windows — SUCCESS.
  • Bazel test on windows-latest for x86_64-pc-windows-gnullvm — SUCCESS.
  • CI results (required) — SUCCESS.
  • [code]smith — SKIPPED.

Blocking P0/P1 findings

  • P1 lease-bypass after heartbeat loss. The new mailbox and schedule heartbeat failure paths call abort_turn_if_active(..., TurnAbortReason::Interrupted) at thread_mailbox_dispatcher_runtime.rs:942/955 and thread_schedule_runtime.rs:751. The existing exact-turn abort implementation in core/src/tasks/mod.rs:669-670 immediately calls maybe_start_turn_for_pending_work() for Interrupted. If another trigger-turn mailbox item is pending when the durable provider-account lease expires, is stolen after expiry, or cannot be renewed, the abort therefore starts a fresh turn under the same resumed thread/profile with a new generated turn ID. That new turn is not registered in usage_profile_leases_by_turn_id and does not acquire or recheck a replacement lease, so it can overlap the new lease owner on the same provider account. This defeats the identity-exclusivity property on a reachable path.
    Remedy: add/use a lease-loss abort path that terminates the exact turn without the generic Interrupted auto-resume, and cover the heartbeat-loss-plus-pending-mail path with a regression proving no unleased replacement turn starts.

Non-blocking follow-ups

  • The PR changes 2,042 lines (1,725 additions, 317 deletions), above the repository review-size guidance. Future work should split durable state primitives from scheduler/mailbox integration where dependencies permit.
  • Released/expired usage_profile_leases rows have no pruning path. This is bounded operational hardening rather than a current P0/P1 gate, but a retention policy should be added before high-frequency dispatch history materially grows the state database.

@andrei-hasna

Copy link
Copy Markdown
Contributor Author

[REVIEW-FIX] #528 @ 86e6858 — reviewer Trebius

Fixed the current-head P1 from the prior NO_GO:

  • Added an exact-turn abort variant that leaves pending mailbox work queued.
  • Routed mailbox and schedule usage-profile lease-loss heartbeats through that variant, so lease loss cannot start a replacement turn before a new durable lease is acquired.
  • Added focused coverage proving an interrupted lease-loss abort leaves the active turn empty and the trigger-turn mailbox item queued.

Evidence:

  • git diff --check — exit 0.
  • gitleaks staged scan before commit — exit 0, no leaks found.
  • gitleaks origin/main..HEAD range scan before push — exit 0, 6 commits scanned, no leaks found.
  • Commit 86e6858 carries Agent: Trebius.
  • The repository formatter could not run on station02: just fmt exited 127 because just is absent; the underlying formatting script exited 1 because just, uv, and cargo are absent. These are not declared repository test gates; the forge formatting/build checks on this head remain authoritative.

@andrei-hasna

Copy link
Copy Markdown
Contributor Author

[REVIEW] GO — #528 @ bae61b1 — lens: correctness+security+gates, reviewer Trebius (1 of 1)

Review basis

  • git log --oneline origin/main..HEAD: exit 0. Read the seven-commit range from the freshly fetched base 6730226823f72b43331b0b8b0e8b750706b03610 through this head.
  • git diff origin/main...HEAD --stat: exit 0. Read the full diff of every changed file and surrounding source.
  • bun install: exit 0; 521 packages installed. Setup only, not a test gate.
  • package.json declares no typecheck script.
  • package.json declares no test script, so this repository declares no package test gate.

Focused remediation review

  • The prior P1 was reproduced on the original head: lease-heartbeat failure aborted an interrupted turn through a path that immediately restarted pending mailbox work, allowing work to resume without the lost usage-profile lease.
  • The current head adds a no-restart abort disposition for lease-loss paths in mailbox and schedule runtimes, leaves pending work queued, and adds lease_loss_abort_keeps_pending_mailbox_work_queued regression coverage.
  • The affected correctness lane and its direct regression are green in the forge. No blocking P0/P1 findings remain.

Lane-chosen commands, not repository gates

  • git diff --check: exit 0; 0 whitespace errors.
  • staged gitleaks scan before each commit: exit 0; 0 findings.
  • gitleaks scan over origin/main..HEAD: exit 0; 7 commits scanned, 0 findings.
  • just fmt: exit 127 because just is not installed on this reviewer machine. The underlying formatter invocation exited 1 because its local formatter dependencies were absent. These are not declared repository gates; the authoritative Format / etc forge check passed after applying its exact rustfmt output.

Authoritative forge checks

  • gh pr checks 528 --repo hasna/codewith --json name,state,bucket: exit 0; 25 passed, 0 failed, 3 skipped.
  • CI results (required): SUCCESS
  • Bazel test on windows-latest for x86_64-pc-windows-gnullvm: SUCCESS
  • Bazel test on ubuntu-24.04 for x86_64-unknown-linux-gnu: SUCCESS
  • Verify release build on ubuntu-24.04 for x86_64-unknown-linux-gnu: SUCCESS
  • Bazel test on windows-latest for x86_64-pc-windows-gnullvm shard 2/4: SUCCESS
  • Verify release build on windows-latest for x86_64-pc-windows-gnullvm: SUCCESS
  • Bazel clippy on windows-latest for x86_64-pc-windows-gnullvm: SUCCESS
  • Bazel clippy on ubuntu-24.04 for x86_64-unknown-linux-gnu: SUCCESS
  • Bazel test on windows-latest for x86_64-pc-windows-gnullvm shard 4/4: SUCCESS
  • Bazel test on ubuntu-24.04 for x86_64-unknown-linux-musl: SUCCESS
  • Bazel test on windows-latest for x86_64-pc-windows-gnullvm shard 1/4: SUCCESS
  • Bazel test on windows-latest for x86_64-pc-windows-gnullvm shard 3/4: SUCCESS
  • Bazel test on windows-latest for x86_64-pc-windows-gnullvm (native main): SKIPPED
  • Format / etc: SUCCESS
  • Argument comment lint - Windows: SUCCESS
  • cargo shear: SUCCESS
  • Argument comment lint - Linux: SUCCESS
  • Argument comment lint package: SKIPPED
  • build-test: SUCCESS
  • Blob size policy: SUCCESS
  • cargo-deny: SUCCESS
  • Build Codewith Linux CLI: SUCCESS
  • Detect changed areas: SUCCESS
  • sdks: SUCCESS
  • python-sdk: SUCCESS
  • Check for spelling errors: SUCCESS
  • cla: SUCCESS
  • [code]smith: SKIPPED

Blocking P0/P1 findings

  • None on the current head.

Non-blocking follow-ups

  • The new usage-profile lease state module is large and may merit later decomposition; this is a maintainability follow-up, not a correctness gate.
  • Released lease-row pruning can be considered later if operational growth warrants it; no current required scale path makes it blocking.

@andrei-hasna
andrei-hasna merged commit 202ebca into main Aug 9, 2026
29 checks passed
@andrei-hasna
andrei-hasna deleted the fix/a59d3086-provider-profile-leases branch August 9, 2026 22:07
@github-actions github-actions Bot locked and limited conversation to collaborators Aug 9, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant