fix(context): revalidate compression state under session lock - #64511
fix(context): revalidate compression state under session lock#64511ljy-2000 wants to merge 1 commit into
Conversation
|
Thanks for the focused concurrency hardening. The premise still holds on current main: The proposed refreshes and parent-ownership check directly cover those verified windows, preserve forced Automated hermes-sweeper review. |
|
Salvaged in #67363 with your commit cherry-picked to preserve authorship — thanks @ljy-2000! The revalidation logic and rotated-parent ownership check land as-is (verified with real-SessionDB probes including two-thread races and the legacy no-lock-API path). Two follow-up commits on top close the review findings: (1) the durable-guard refresh now also runs at the should_compress() pre-gates via _automatic_compression_blocked (check-refresh-recheck, hot path pays no DB reads) so a cleared durable row can unblock a prebound agent whose stale fallback streak has no expiry timer; (2) a refresh finding no durable cooldown row no longer clears a live local cooldown whose DB persist failed. #67363 has auto-merge armed and will land once CI completes. |
Follow-up on the salvaged #64511 commit: - _automatic_compression_blocked() now refreshes durable guard state (cooldown + fallback streak) when — and only when — the in-memory snapshot says blocked, then re-evaluates. The should_compress() pre-gates (preflight/turn paths) consult this before ever reaching compress_context, and a stale fallback streak has no expiry timer, so without a gate-level refresh a cleared durable row could never unblock a prebound agent. The unblocked hot path pays no DB reads. - A refresh that finds no durable cooldown row no longer clears a live local cooldown whose DB persist FAILED (_cooldown_persist_failed): an empty row is not evidence another agent cleared it, and honouring it would reopen the #11529 thrash window. A successful durable round-trip (record or read) makes the DB authoritative again. - Guard tests for both directions (red on the pre-fix code), including a hot-path test asserting the unblocked gate never touches the DB.
Follow-up on the salvaged NousResearch#64511 commit: - _automatic_compression_blocked() now refreshes durable guard state (cooldown + fallback streak) when — and only when — the in-memory snapshot says blocked, then re-evaluates. The should_compress() pre-gates (preflight/turn paths) consult this before ever reaching compress_context, and a stale fallback streak has no expiry timer, so without a gate-level refresh a cleared durable row could never unblock a prebound agent. The unblocked hot path pays no DB reads. - A refresh that finds no durable cooldown row no longer clears a live local cooldown whose DB persist FAILED (_cooldown_persist_failed): an empty row is not evidence another agent cleared it, and honouring it would reopen the NousResearch#11529 thrash window. A successful durable round-trip (record or read) makes the DB authoritative again. - Guard tests for both directions (red on the pre-fix code), including a hot-path test asserting the unblocked gate never touches the DB.
Summary
Revalidates durable compaction guard state while automatic compression is serialized, and prevents a delayed stale agent from rotating an already-compressed parent session a second time.
This is a concurrency hardening follow-up to #63965 / #63008. It does not reopen or claim to address the broader R1-R6 issue scope.
Root cause
The persisted fallback breaker and provider cooldown were loaded when a compressor was bound, then checked before the per-session compression lock. Another agent could update or clear those durable guards before this agent reached
compress(), leaving the decision based on stale in-memory state.The compression lock also serialized overlapping work but did not prove that the old session was still a live rotation owner. A delayed contender could acquire the parent lock after the winner released it, then rotate the already-ended parent into a second child.
Changes
end_reason='compression'/compresspolicy bypass for breaker/cooldown while keeping session ownership mandatoryNo config, schema, provider, tool, or plugin API changes are included.
Validation
610 passedacross compressor, rotation, concurrent fork, host-contract, Codex app-server, gateway synchronization, command, and state suitestest_concurrent_compression_does_not_fork_sessionregression passespy_compilepassedgit diff --checkpassed