Skip to content

fix(combos): preserve explicit persisted cooldown clears - #8514

Merged
lidge-jun merged 3 commits into
lidge-jun:devfrom
medking82:fix/combo-persisted-clears-8117
Nov 11, 2026
Merged

lidge-jun merged 3 commits into
lidge-jun:devfrom
medking82:fix/combo-persisted-clears-8117

Conversation

@medking82

@medking82 medking82 commented Nov 11, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • Follow up on the two post-merge findings in feat(combos): persist target cooldowns across restarts #8117: clear followed by a shorter set and combo-wide clears missing peer rows.
  • Retain explicit key tombstones until the locked flush excludes the previous disk row, then overlay subsequent local cooldowns.
  • Queue null-delimited combo-prefix clears and apply them to the freshly read file under the existing config mutation lock. Failed writes retain the queue; successful writes consume it.
  • Preserve target-scoped clears, memory-only expiry/probe drops, unrelated combos, and the longer-expiry rule when no explicit clear was requested.
  • Add six native Bun regression cases and update the persistence contract. The public save/rename/delete clearing contract is unchanged.

Verification

Base: dev at 87e005f93c2ceafa6472f3d4bb56caa8b12fbfdf. Fix commit: 7b317a53b2a11ba2b264a84f90248424836369b0. Linux, pinned Bun 1.4.4.

  • RED on unchanged base source with the two new regressions: bun test tests/routing/combo-cooldown-persistence.test.ts --test-name-pattern 'a queued key clear|a combo-wide public clear': 0 pass, 2 fail. The failures show the old 24-hour row replacing the new 60-second row, and peer rows surviving a combo clear.
  • GREEN after the fix: bun test tests/routing/combo-cooldown-persistence.test.ts: 15 pass, 0 fail, 45 expectations.
  • bun run typecheck: passed across 2,397 files.
  • bun run structure:check, bun run privacy:scan, git diff --check: passed.
  • Broader focused run through bun scripts/test.ts across cooldown persistence/recording, combo management/classification, file-size ratchet and test-layout guards: 204 pass, 2 fail. Both failing tests also reproduce with the unchanged base source (59 pass, 2 fail across their two files):
    • combo management API > DELETE refresh immediately retires the final managed combo catalog row (catalog row remains present).
    • a local Codex pool cooldown uses capped target backoff and permits account recovery (502 rather than 200).
  • bun run test:changed: blocked before a completion summary because this environment refused an attempted connection to api.deepseek.com; the request payload and credential scope were not established. The run was not retried and is not counted as passing.
  • Current-head hosted checks: enforce-target, hygiene, label and resolve-pr passed. Catalog async contracts is action_required pending maintainer approval for this fork. The aggregate ci check has not completed; CodeRabbit skipped the draft.
  • Code review of the final four-file patch found no blocking regressions.
  • NOT RUN: full local suite, Windows/macOS, live providers. The local scope uses targeted regressions and changed-file coverage for this four-file state-merge fix. No full-suite pass is claimed.

Maintainer action needed: approve the fork workflow runs for 7b317a53b2a11ba2b264a84f90248424836369b0; no workflow permissions or repository settings were changed.

Maintainer verification (combo/pool bundle lane)

Head a81daf5260 (contributor branch with current dev ac41d6b7 merged; no code changes beyond the merge), macOS, Bun 1.4.4.

  • RED on current dev source: with src/combos/cooldown-store.ts and src/combos/failover.ts taken from origin/dev, the queued key and combo clear cases fail (the old 24-hour row replaces the new one). GREEN with this PR: tests/routing/combo-cooldown-persistence.test.ts 15 pass, 0 fail (three repeated runs).
  • combo-management-api, server-combo-cooldown-recording, server-combo-cooldown-fallback: pass. When these run in one process together with the persistence file, a combo cooldown is written with an absolute expiry… fails identically on unchanged dev (cross-file interaction, not this change; CI runs files isolated).
  • The two base failures listed above (DELETE refresh…, a local Codex pool cooldown…) pass on current dev here.
  • Independent review (codex / hyds/deepseek-v4.1-flash-1m): PASS, no findings.

Co-authored-by: medking82 j.g.wong@hotmail.com

Checklist

  • Scope stays focused and avoids unrelated cleanup.
  • Docs or release notes were updated when needed.
  • Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults. No credential, auth, workflow, dependency or release behavior changed.

Review readiness checklist

This PR stays in draft until every box below is ticked. Tick all four boxes once the requirements are met:

  • Required local validation passed; commands, results, and any full-suite exception are documented.
  • I pushed my PR to a recent dev commit (at most 10 behind; a maintainer may still ask for the exact tip before merge).
  • I resolved all correct Codex and CodeRabbit findings.
  • My PR is ready for review.

Keep queued key tombstones through subsequent sets and apply combo-prefix
clears to the fresh file under the mutation lock. Add regressions for
shorter replacements, peer rows, retry retention, and clear ordering.

Follow-up to lidge-jun#8117.
@coderabbitai

coderabbitai Bot commented Nov 11, 2026 •

Copy link
Copy Markdown
Contributor

Explain Change →

Pre-merge checks: Passed 1 · Inconclusive 1

Failed Failed checks: 1 inconclusive

Check name Status Explanation Resolution
Title check Pending This assessment has not completed. The check will be retried on the next pre-merge check run.
Passed Passed checks: 1 passed

Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

@github-actions

Copy link
Copy Markdown
Contributor

✅ Deterministic PR hygiene checks passed.

@github-actions github-actions Bot added the bug Something isn't working label Nov 11, 2026
@github-actions

github-actions Bot commented Nov 11, 2026 •

Copy link
Copy Markdown
Contributor

✅ READY

  • all PR quality gates passed; the review readiness checklist is complete.

Review readiness checklist

  • ✅ Required local validation passed; commands, results, and any full-suite exception are documented.
  • ✅ I pushed my PR to a recent dev commit (at most 10 behind; a maintainer may still ask for the exact tip before merge).
  • ✅ I resolved all correct Codex and CodeRabbit findings.
  • ✅ My PR is ready for review.

✅ 4/4 boxes ticked.

This pull request is already Ready for Review.
The review-ready label marks this PR as ready; review automation runs independently.
Maintainers notified: @lidge-jun @Ingwannu

Hygiene

✅ Deterministic PR hygiene checks passed.

@lidge-jun
lidge-jun marked this pull request as ready for review November 11, 2026 09:44
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 11, 2026 •

Copy link
Copy Markdown
Codex Review Summary: ✅ Completed

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-11T09:49:32.158623Z a81daf5 Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@lidge-jun

Copy link
Copy Markdown
Owner

Maintainer judgment (combo/pool bundle lane), head a81daf5260.

  • Value: closes the two post-merge findings on feat(combos): persist target cooldowns across restarts #8117. An explicit cooldown clear followed by a shorter cooldown no longer resurrects the older, longer row on disk, and clearing a combo now also removes rows a peer process wrote that this process never loaded.
  • UX change scope: none visible. Combo save, rename and delete clear cooldowns as documented; only the persisted file now matches.
  • Existing-user regression evidence: the longer-expiry merge for rows that were not cleared is unchanged and still tested. Combo management, cooldown recording and fallback suites pass.
  • Windows impact: same locked read-modify-write and atomic rename as before; no new filesystem primitive. Busy-lock retry keeps pending clears (tested).
  • Verification: RED on current dev source, GREEN 15/15 (three runs); typecheck clean on this head; CI on this head green except CodeRabbit pending. Independent review on codex / hyds/deepseek-v4.1-flash-1m: PASS, no findings.
  • NOT RUN: macOS/Windows lanes (batch dev dispatch policy); full local suite.

@lidge-jun
lidge-jun merged commit 5cc64ae into lidge-jun:dev Nov 11, 2026
9 of 11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working review-ready

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants