fix(gateway): charge dispatched resumes and preserve cap across rollbacks - #801
Merged
Merged
Conversation
Collaborator
Author
FleetReviewConfidence: 1/5 Findings
FleetReview provenance · models: B=gpt-5.6-sol, C=claude-code-opus-5 · cost: $4.50 · duration: 14m 23s · rounds: 1 · files examined: 9 |
The v2 reader inferred "legacy repaired our counters away" from an absent session_attempts key in v1. That absence is equally true of every fresh install, so the WARNING fired on each boot of hosts that were never rolled back -- on the restart-audit channel this repo keeps grep-clean. Record whether the one-time v1->v2 migration actually carried counters (migrated_from_v1_counters in the v2 ledger) and gate the warning on that instead. Provenance survives write round-trips and cross-process reloads; an absent flag fails quiet rather than re-arming the false positive. Verified: fresh-install arm warns 0 times across 3 boots (was 3/3); control arm with real repaired-away counters still warns exactly once. New test fails against both the discrimination-deleting mutant and the previous predicate.
Collaborator
Author
FleetReviewConfidence: 2/5 Findings
FleetReview provenance · models: B=gpt-5.6-sol, C=claude-code-opus-5, D=grok-4.6, F=gpt-5.6-sol, G=grok-4.6 · cost: $6.56 · duration: 1h 02m 29s · rounds: 3 · files examined: 10 |
Kyzcreig
pushed a commit
that referenced
this pull request
Sep 22, 2026
…me cancel owns its task Two FleetReview P1s on 17888bd. P1-A slot leak on cancel. TurnAdmission.slot() is an async generator, so a raise BEFORE its first yield skips __aexit__ entirely and the pre-yield region is the ONLY release path. The old code awaited the notice task after total_acquired=True; that await sat behind ack() -> adapter.send following a 15 s wait, a wide window in which a CancelledError permanently burned one of `cap` slots. At zero, every turn blocks in acquire forever — the starvation this gate exists to prevent. Pre-yield now releases internal/total (and unwinds in_flight/_owners) on ANY BaseException and re-raises; the notice is cancelled and reaped detached via add_done_callback, never awaited inside the critical section. P1-B broken cancel contract. StartupResumePool.submit() handed back a bare Future, which marks itself done the instant it is cancelled even while the admitted resume task keeps running. gateway/run.py's shutdown path reads exactly that done/cancelled state to tell "never started -> cancel + re-mark the session" from "in progress -> leave it alone", so a running resume could be re-marked and restored a second time on top of the original turn. Admitted entries now get _AdmittedResumeHandle, whose cancel() delegates to the inner task and which stays pending until the task's done callback resolves it. Queued entries keep the plain Future contract. Verified (venv python3.11, PYTHONPATH=worktree, HERMES_HOME=/tmp): - 8-file focused set: 176 passed, 0 failed. - tests/gateway/test_resume_cap_hardening.py::test_charge_only_after_dispatch [cancel] (#801's red test on this head) green, file unedited. - Mutation-proof P1-A: delete the pre-yield except-BaseException release -> test_pre_yield_failure_releases_acquired_permits[False,True] RED (total._value 2 != 3, the burned slot). - Mutation-proof P1-B: submit() back to a bare Future -> test_admitted_resume_handle_cancel_delegates_to_its_task RED AND test_charge_only_after_dispatch[cancel] RED (assert 2 == 0). - 4 round-3 guard mutants re-run, all still RED: cap coercion, startup coercion, legacy contract, stop-guard. Class sweep (AST over every .py): exactly 2 async context managers acquire a permit before yield. The other, hermes_cli/session_db_heavy_gate.py session_db_heavy_read_slot, has zero awaits between acquire and yield, so it has no cancellation-delivery point and is not an instance of this class. ruff check, py_compile, git diff --check all clean.
github-merge-queue Bot
pushed a commit
that referenced
this pull request
Sep 22, 2026
…9-21 starvation) (#827) * fix(gateway): bound turn and boot-resume concurrency Add gateway turn admission with reserved user capacity and retain permits until executor workers actually exit. Bound startup resume execution while preserving synchronous session claims and the restore drain timeout. Verified: - 66 focused gateway tests passed via scripts/run_tests.sh - ruff passed on all changed Python files - git diff --check and py_compile passed - clean fork/main bounded-executor regression failed while the unbounded control passed * fix(gateway): schedule admitted startup resumes immediately Restore the pre-existing create_task timing for the first bounded startup resume workers so inbound restore handling observes recovery as started. Verified: - 117 focused gateway tests passed - ruff check gateway/turn_admission.py - python3 -m py_compile gateway/turn_admission.py - git diff --check * fix(gateway): harden concurrency admission boundaries Normalize runtime concurrency values before constructing semaphores, preserve direct handler callers without generation state, and update the atomic-write ratchet for renamed admitted handlers. Verified: - 184 focused gateway tests passed across 10 files - ruff check passed on changed Python files - py_compile passed on changed Python files - git diff --check passed * fix(gateway): release turn permits on pre-yield cancel; admitted resume cancel owns its task Two FleetReview P1s on 17888bd. P1-A slot leak on cancel. TurnAdmission.slot() is an async generator, so a raise BEFORE its first yield skips __aexit__ entirely and the pre-yield region is the ONLY release path. The old code awaited the notice task after total_acquired=True; that await sat behind ack() -> adapter.send following a 15 s wait, a wide window in which a CancelledError permanently burned one of `cap` slots. At zero, every turn blocks in acquire forever — the starvation this gate exists to prevent. Pre-yield now releases internal/total (and unwinds in_flight/_owners) on ANY BaseException and re-raises; the notice is cancelled and reaped detached via add_done_callback, never awaited inside the critical section. P1-B broken cancel contract. StartupResumePool.submit() handed back a bare Future, which marks itself done the instant it is cancelled even while the admitted resume task keeps running. gateway/run.py's shutdown path reads exactly that done/cancelled state to tell "never started -> cancel + re-mark the session" from "in progress -> leave it alone", so a running resume could be re-marked and restored a second time on top of the original turn. Admitted entries now get _AdmittedResumeHandle, whose cancel() delegates to the inner task and which stays pending until the task's done callback resolves it. Queued entries keep the plain Future contract. Verified (venv python3.11, PYTHONPATH=worktree, HERMES_HOME=/tmp): - 8-file focused set: 176 passed, 0 failed. - tests/gateway/test_resume_cap_hardening.py::test_charge_only_after_dispatch [cancel] (#801's red test on this head) green, file unedited. - Mutation-proof P1-A: delete the pre-yield except-BaseException release -> test_pre_yield_failure_releases_acquired_permits[False,True] RED (total._value 2 != 3, the burned slot). - Mutation-proof P1-B: submit() back to a bare Future -> test_admitted_resume_handle_cancel_delegates_to_its_task RED AND test_charge_only_after_dispatch[cancel] RED (assert 2 == 0). - 4 round-3 guard mutants re-run, all still RED: cap coercion, startup coercion, legacy contract, stop-guard. Class sweep (AST over every .py): exactly 2 async context managers acquire a permit before yield. The other, hermes_cli/session_db_heavy_gate.py session_db_heavy_read_slot, has zero awaits between acquire and yield, so it has no cancellation-delivery point and is not an instance of this class. ruff check, py_compile, git diff --check all clean. --------- Co-authored-by: Kyzcreig <9063726+Kyzcreig@users.noreply.github.com> Co-authored-by: Daedalus <daedalus@ang-ventures.local>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
adapter.handle_messagesucceeds, not when its wrapper is merely scheduled. Cancellation before dispatch and dispatch failure consume no credit.*.sessions-v2.jsonledger. Migrate v1 counters once; an existing v2 ledger is authoritative even after a legacy repair. Warn once per store instance when legacy counters are empty alongside retained v2 counters.Rollback contract (Apollo ruling)
Any legacy release may run: it never opens the v2 ledger and does not enforce the v2 cap. Rolling forward resumes from persisted v2 counters; attempts made by legacy releases are not counted. No restriction on rollback versions. Seven-day TTL refill remains as designed.
Verification so far
300 != 14400; restored GREEN.cea2ef75e0exercised byrepro_rollback_store.py: legacy verdict(True, 0); actual legacy_repairleaves v2 untouched; roll-forward verdict(False, 3).scripts/run_tests.sh tests/gateway/ -j 32, sandbox HOME + shared development interpreter, pinned baseae7db6c1c9and candidateed2a3c44dd:asyncio.to_thread, with per-store RLock serialization over read/modify/write. Concurrent 24-increment probe RED without serialization, GREEN with it. Focused cap + fsync + atomic guard suite: 76 passed.No merge or deployment performed.