Skip to content

fix(handlers): bound the invalid shared values warning set (#711) - #714

Merged
omattsson merged 1 commit into
mainfrom
fix/711-bound-shared-values-warnings
Oct 26, 2026
Merged

omattsson merged 1 commit into
mainfrom
fix/711-bound-shared-values-warnings

Conversation

@omattsson

Copy link
Copy Markdown
Owner

Fixes #711.

warnedInvalidSharedValues (internal/api/handlers/shared_values_loader.go) was a package-level sync.Map. Every new version of an invalid shared values entry added a key that stayed for the life of the process.

Change: it is now an internal/cache TTLCache with a one-hour TTL (invalidSharedValuesWarnTTL). A reported entry version is forgotten after an hour, so the set stays bounded.

Behaviour change: a broken entry was logged once per process. It is now logged again each hour while it stays broken.

The cache check and the store are not one atomic step. Two values builds at the same moment can both log the same version, which is harmless for a warning and noted in a code comment.

Tests

  • TestWarnInvalidSharedValues uses a fresh cache instead of a fresh sync.Map.
  • New TestWarnInvalidSharedValues_ReportedAgainAfterTTL (50 ms TTL): a version is logged once within the TTL, forgotten after it, and logged again.

Checks

  • The warning tests pass with -race -count=10.
  • go test ./... -short -race and go vet pass.
  • make lint: 0 errors.
  • make gosec: 0 issues.

🤖 Generated with Claude Code

warnedInvalidSharedValues was a package-level sync.Map: every new version
of an invalid shared values entry added a key for the life of the process.
It is now an internal/cache TTLCache with a one-hour TTL
(invalidSharedValuesWarnTTL): a reported version is forgotten after an hour,
so the set stays bounded and a broken entry is reported again each hour
while it stays broken.

The Get and the Set are not one atomic step, so two concurrent values
builds can both log the same version; that is harmless for a warning.

A new test checks that a version is forgotten and logged again after the
TTL.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@omattsson omattsson added the tech-debt Technical debt and code quality improvements label Oct 25, 2026
@coderabbitai

coderabbitai Bot commented Oct 25, 2026

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 1ce2c40a-dd71-4bbd-9c4c-87a115cf61a9
  • 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

Comment @coderabbitai help to get the list of available commands.

@omattsson
omattsson merged commit 3a53938 into main Oct 26, 2026
8 checks passed
@omattsson
omattsson deleted the fix/711-bound-shared-values-warnings branch October 26, 2026 06:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

tech-debt Technical debt and code quality improvements

Projects

None yet

Development

Successfully merging this pull request may close these issues.

handlers: bound the warnedInvalidSharedValues set

1 participant