Skip to content

fix(compression): recover abandoned locks and suppress no-op retries - #65775

Closed
the3asic wants to merge 3 commits into
NousResearch:mainfrom
the3asic:pr/compression-lock-recovery
Closed

fix(compression): recover abandoned locks and suppress no-op retries#65775
the3asic wants to merge 3 commits into
NousResearch:mainfrom
the3asic:pr/compression-lock-recovery

Conversation

@the3asic

Copy link
Copy Markdown
Contributor

What changed

  • Reclaim a compression lease immediately when its structured pid= holder is
    proven dead on POSIX, instead of waiting for the full TTL.
  • Keep Windows on TTL-only recovery because Python implements
    os.kill(pid, 0) destructively there.
  • Stop the pre-API pressure loop from calling _compress_context three times
    when compression returns an unchanged message count.
  • Add regression coverage for dead/live/legacy holders, Windows behavior, and
    no-op compression retry suppression.

Root cause

When a gateway process was killed during compression, its SQLite lease remained
valid for 300 seconds. The replacement gateway repeatedly saw the abandoned
lock and retried compaction. A second issue in the pre-API loop ignored the
documented no-op return contract and emitted three identical compact attempts in
the same turn.

Validation

  • 88 focused compression/compaction tests passed on current upstream main
  • ruff passed for all changed files
  • Fable Max review: no blockers; PR-ready

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state labels Jul 16, 2026

@tonydwb tonydwb left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review Summary

Verdict: Approved (read-only token — formal approval requires write access)

Fixes compression recovery for abandoned locks and suppresses no-op retries.

  • Addresses a race condition where compression locks could be abandoned
  • Suppresses retry attempts that would be no-ops (nothing to compress)
  • Well-scoped changes with clear documentation
  • No security concerns

Reviewed by Hermes Agent

@teknium1 teknium1 added sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform area/compression Context compression and continuation sessions labels Jul 18, 2026
teknium1 added a commit that referenced this pull request Jul 23, 2026
…add same-pid self-reclaim guard

Hardening on top of the salvaged dead-PID lease reclamation from PR #65775
(@the3asic):

- Probe via psutil.pid_exists (hard dependency; CONTRIBUTING.md critical
  rule #1) with the contributor's os.kill(pid, 0) POSIX probe retained
  only as a scaffold-phase fallback when psutil is missing.
- Same-process holders (pid == os.getpid()) are never probed and never
  self-reclaimed — another thread's live lease is owned by the lease
  refresher/release path.
- Any probe doubt (exceptions, permission errors) conservatively keeps
  the lease until normal TTL expiry; Windows stays TTL-only.
- Tests: psutil-first dead-pid reclaim (probe call pinned), os.kill
  fallback path, probe-doubt keeps lease, same-pid no self-reclaim,
  legacy holder + Windows paths assert NO probe via either API.
teknium1 added a commit that referenced this pull request Jul 23, 2026
…add same-pid self-reclaim guard

Hardening on top of the salvaged dead-PID lease reclamation from PR #65775
(@the3asic):

- Probe via psutil.pid_exists (hard dependency; CONTRIBUTING.md critical
  rule #1) with the contributor's os.kill(pid, 0) POSIX probe retained
  only as a scaffold-phase fallback when psutil is missing.
- Same-process holders (pid == os.getpid()) are never probed and never
  self-reclaimed — another thread's live lease is owned by the lease
  refresher/release path.
- Any probe doubt (exceptions, permission errors) conservatively keeps
  the lease until normal TTL expiry; Windows stays TTL-only.
- Tests: psutil-first dead-pid reclaim (probe call pinned), os.kill
  fallback path, probe-doubt keeps lease, same-pid no self-reclaim,
  legacy holder + Windows paths assert NO probe via either API.
@teknium1

Copy link
Copy Markdown
Contributor

The dead-PID lease reclamation half of this draft landed via salvage PR #70247 with your commits cherry-picked and authorship preserved — thanks @the3asic! Hardening on top: psutil.pid_exists as the probe (per CONTRIBUTING's os.kill(pid,0) ban; it's already a hard dep) and a same-pid self-reclaim guard. The attempts=3 loop hunk was dropped as discussed — it would clamp the user-configurable compression.max_attempts that landed via #69315. Closing the draft as landed.

@teknium1 teknium1 closed this Jul 24, 2026
randlee pushed a commit to randlee/hermes-agent that referenced this pull request Aug 11, 2026
…add same-pid self-reclaim guard

Hardening on top of the salvaged dead-PID lease reclamation from PR NousResearch#65775
(@the3asic):

- Probe via psutil.pid_exists (hard dependency; CONTRIBUTING.md critical
  rule #1) with the contributor's os.kill(pid, 0) POSIX probe retained
  only as a scaffold-phase fallback when psutil is missing.
- Same-process holders (pid == os.getpid()) are never probed and never
  self-reclaimed — another thread's live lease is owned by the lease
  refresher/release path.
- Any probe doubt (exceptions, permission errors) conservatively keeps
  the lease until normal TTL expiry; Windows stays TTL-only.
- Tests: psutil-first dead-pid reclaim (probe call pinned), os.kill
  fallback path, probe-doubt keeps lease, same-pid no self-reclaim,
  legacy holder + Windows paths assert NO probe via either API.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/compression Context compression and continuation sessions comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P2 Medium — degraded but workaround exists sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants