Skip to content

fix(proxy): retry detached API key release - #1545

Open
mastertyko wants to merge 4 commits into
Soju06:mainfrom
mastertyko:fix/retry-detached-api-key-release
Open

fix(proxy): retry detached API key release#1545
mastertyko wants to merge 4 commits into
Soju06:mainfrom
mastertyko:fix/retry-detached-api-key-release

Conversation

@mastertyko

@mastertyko mastertyko commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Summary

A detached stream API-key settlement could fail, enqueue its reservation-release fallback, and then lose that cleanup if the fallback also hit a transient persistence error. The persistence drain could consequently report success while the reservation continued consuming quota until stale recovery.

This change keeps the tracked fallback alive and retries the idempotent release with capped exponential backoff. A shared four-attempt gate bounds aggregate repository pressure, and drain_persistence_tasks() cannot report completion while that settlement/release chain remains unfinished.

Type of change

  • fix: — bug fix (no behavior change beyond the bug)

Linked issue: None — no matching issue was found.

OpenSpec

  • This PR includes / updates an OpenSpec change

Change directory: openspec/changes/retry-detached-api-key-release/

This does not change any upstream request or response wire format.

Changes

  • Retry transient persistence failures only inside tracked detached-settlement release fallbacks.
  • Bound retry-enabled fallback repository attempts to four per proxy service instance while queued fallbacks remain tracked and drain-visible.
  • Preserve idempotent reservation settlement, cancellation ownership, detached response latency, and truthful persistence-drain results.
  • Add deterministic coverage for double-transient recovery, aggregate retry fan-out, and the synchronous one-shot backstop.

Related work

Test plan

  • Regression failed deterministically twice on baseline 3fe0d6f286019a0505783d803db9a1d8cdf6b307 and passes with this fix.
  • Detached-persistence integration suite: 6 passed.
  • Settlement and cancellation unit slice: 9 passed.
  • API-key reservation and SQLite settlement slice: 4 passed.
  • Changed-file Ruff, formatting, and type checks passed.
  • Proxy-architecture checks passed.
  • Strict scoped OpenSpec validation and all 48 main-spec validations passed.
  • The committed diff was re-reviewed clean after each local review repair.
  • Full local CI was intentionally not run; GitHub required CI remains the authoritative integration gate.

Screenshots / output

Not applicable — no dashboard, rendered UI, media, request schema, or response schema changes.

Checklist

  • Title is in Conventional Commits format.
  • Related work was checked; no matching issue exists.
  • Added regression coverage for the affected persistence paths.
  • Ran the relevant focused local test, lint, type, architecture, and OpenSpec checks.
  • Simplicity gates reviewed; no setting, setup step, README section, default, or dashboard navigation item changes.
  • CHANGELOG.md is not edited.

@mastertyko

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c77b1feb95

ℹ️ 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".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread app/modules/proxy/_service/api_key_usage.py
@mastertyko

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Bravo.

Reviewed commit: 11d9d4e1af

ℹ️ 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".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@Komzpa Komzpa added 🤖 codex: ok [@codex review] says no issues found. and removed 🤖 codex: ok [@codex review] says no issues found. labels Jul 30, 2026
@Komzpa Komzpa added 🤖 codex: ok [@codex review] says no issues found. and removed 🤖 codex: ok [@codex review] says no issues found. labels Jul 31, 2026
@Soju06

Soju06 commented Aug 4, 2026

Copy link
Copy Markdown
Owner

Triage review passed (verified the tracked-retry semantics and the one-shot inline backstop) and this was queued for merge today, but #1558 landed first and touches the same settlement region, so the branch flipped to conflicting. One rebase over current main and this merges immediately — the review stands, no re-review needed unless conflict resolution changes the retry/semaphore logic.

@mastertyko

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. 🚀

Reviewed commit: 7469430cba

ℹ️ 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".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@Komzpa Komzpa added the 🤖 codex: ok [@codex review] says no issues found. label Aug 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🤖 codex: ok [@codex review] says no issues found.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants