Skip to content

Controls cache invalidation or persistance on errors#4945

Open
sergeibbb wants to merge 1 commit intomainfrom
4944-cache-auth-errors
Open

Controls cache invalidation or persistance on errors#4945
sergeibbb wants to merge 1 commit intomainfrom
4944-cache-auth-errors

Conversation

@sergeibbb
Copy link
Member

@sergeibbb sergeibbb commented Jan 30, 2026

Description

Solves #4942

Controls cache invalidation or persistence on errors.

Be careful, because it makes #4943 more noticeable.

Checklist

  • I have followed the guidelines in the Contributing document
  • My changes follow the coding style of this project
  • My changes build without any errors or warnings
  • My changes have been formatted and linted
  • My changes include any required corresponding changes to the documentation (including CHANGELOG.md and README.md)
  • My changes have been rebased and squashed to the minimal number (typically 1) of relevant commits
  • My changes have a descriptive commit message with a short title, including a Fixes $XXX - or Closes #XXX - prefix to auto-close the issue that your PR addresses

@sergeibbb sergeibbb force-pushed the 4944-cache-auth-errors branch 2 times, most recently from a635ec7 to 811c35b Compare January 30, 2026 16:16
@sergeibbb sergeibbb force-pushed the 4944-cache-auth-errors branch from 811c35b to 81c5010 Compare January 30, 2026 16:17
@sergeibbb sergeibbb marked this pull request as ready for review January 30, 2026 16:24
@augmentcode
Copy link

augmentcode bot commented Jan 30, 2026

🤖 Augment PR Summary

Summary: This PR refines integration caching behavior to better control whether cached entries are invalidated or retained when requests fail.

Changes:

  • Extends cache callbacks to receive a CacheController, allowing callers to explicitly invalidate entries after async work completes
  • Adds an expireOnError option to CacheProvider to choose whether failed promises remove cached entries
  • Updates IntegrationBase.getCurrentAccount to invalidate cache on cancellations and non-auth errors, but to rethrow provider errors for callers to surface
  • Adjusts PR/issue cache wrapping to propagate the new controller through wrappers
  • Updates the changelog noting improved display of revoked-credential auth errors in Launchpad

Technical Notes: Failed-promise caching behavior is now configurable via expireOnError and explicit invalidation.

🤖 Was this summary useful? React with 👍 or 👎

Copy link

@augmentcode augmentcode bot left a comment

Choose a reason for hiding this comment

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

Review completed. 1 suggestions posted.

Fix All in Augment

Comment augment review to trigger a new review at any time.

},
);
void value.then(v => this.set(cache, key, v, etag, expiresAt, expireOnError));
if (expireOnError !== false) {
Copy link

Choose a reason for hiding this comment

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

When expireOnError === false, a rejected Promise remains cached, and since promise entries never get an expiresAt, this can effectively persist indefinitely and also overwrite any previously cached successful value. Consider whether call sites relying on this need a bounded retry/un-stick mechanism (e.g., explicit invalidation on credential change) so auth failures don’t become permanent.

Other Locations
  • src/plus/integrations/models/integration.ts:630

Fix This in Augment

🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant