You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite. Learn more
PR Review: fix(api-peer): refresh runner config list reads
Summary
This is a minimal one-line fix in engine/packages/api-peer/src/runner_configs.rs. The list endpoint for runner configs previously passed bypass_cache: false to pegboard::ops::runner_config::get::Input, meaning reads went through a 5-second TTL cache. The fix changes this to bypass_cache: true, ensuring the list endpoint always reads directly from the store (UniversalDB) rather than serving potentially stale cached data.
What the Change Does
The runner_configs::list handler has two code paths:
Named lookup (when runner_name/runner_names query params are provided) — calls pegboard::ops::runner_config::get. This was previously cached with a 5-second TTL; now it bypasses the cache.
Paginated list (no specific names provided) — calls pegboard::ops::runner_config::list, which does not use the cache at all. This path is unaffected by the PR.
The fix targets path (1) only.
Correctness
The fix is correct and matches the established pattern in the codebase. All other callers that need fresh reads already use bypass_cache: true:
pegboard/src/ops/runner_config/upsert.rs — fresh read before write.
pegboard/src/workflows/runner_pool_metadata_poller.rs — reads fresh config to drive metadata polling.
api-public/src/runner_configs/refresh_metadata.rs — reads fresh config to drive metadata refresh.
The only non-critical callers that tolerate stale data (and correctly use bypass_cache: false) are internal workflow scheduling reads in runner_pool.rs and actor/runtime.rs, where a brief stale view is acceptable for scheduling decisions.
The API list endpoint is a user-facing read operation where freshness is expected. Returning stale data (up to 5 seconds old) after a recent upsert would be a correctness issue, particularly for tools or UIs that poll the list immediately after a configuration change.
Code Quality
No concerns. The change is minimal, targeted, and consistent with surrounding codebase conventions.
Potential Bugs / Gaps
The fix only applies to the named-lookup path. The paginated list path calls pegboard::ops::runner_config::list, which reads directly from UniversalDB without a cache layer and was already always-fresh. There is no inconsistency introduced — both paths now return fresh data.
Performance Considerations
Bypassing the cache increases load on UniversalDB for the named-lookup path. This is the correct tradeoff for a user-facing API endpoint where stale data is worse than the marginal extra latency. The bypass_cache: false path with a 5-second TTL makes sense for high-frequency internal scheduling calls (like runner_pool.rs), not for infrequent API list reads triggered by human operators or CI pipelines.
Security
No security concerns. The change does not affect authorization checks, trust boundaries, or data exposure. It only affects caching behavior of an already-authorized read.
Test Coverage
The existing tests cover the cache-bypass semantics for refresh_metadata but do not directly cover the list endpoint's named-lookup path. A targeted test asserting that an immediate GET /runner-configs?runner_name=X after PUT /runner-configs/X returns the updated config without a 5-second wait would prevent regression. Minor gap, not a blocker.
Verdict
The fix is correct, minimal, and consistent with how bypass_cache: true is used everywhere else that needs authoritative reads. Ready to merge.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Please include a summary of the changes and the related issue. Please also include relevant motivation and context.
Type of change
How Has This Been Tested?
Please describe the tests that you ran to verify your changes.
Checklist: