Develop

[qnen]

Pull Request Checklist

Pull Request Type

• Feature
• Fix
• Refactor
• Pipeline
• Tests
• Documentation

Checklist

Please check each item after it's completed.

• I have tested these changes locally.
• I have updated the documentation accordingly.
• I have added necessary comments to the code, especially in complex areas.
• I have ensured that my changes adhere to the project's coding standards.
• I have checked for any potential security issues.
• I have ensured that all tests pass.
• I have updated the version appropriately (if applicable).
• I have confirmed this code is ready for review.

Additional Notes

Obs: Please, always remember to target your PR to develop branch instead of main.

[coderabbitai]

Important

Review skipped

Review was skipped as selected files did not have any reviewable changes.

💤 Files selected but had no reviewable changes (2)

• commons/opentelemetry/otel.go
• commons/opentelemetry/otel_test.go

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: c8d011af-8ae0-4030-a7d2-2466a7de6af9

📥 Commits

Reviewing files that changed from the base of the PR and between 977411b and dfc8f4d.

📒 Files selected for processing (2)

• commons/opentelemetry/otel.go
• commons/opentelemetry/otel_test.go

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

Walkthrough

This pull request implements a comprehensive version 4 release of the lib-commons library. Changes include removal of git hooks (commit-msg, pre-commit, pre-push, pre-receive), updates to build tooling and CI configurations, and extensive library restructuring. The logging interface is redesigned from method-based (Info/Error/etc.) to structured context-aware methods (Log). New packages are introduced including assertion frameworks, backoff/retry utilities, cron expression parsing, JWT token handling, and enhanced security helpers. Existing packages are significantly refactored: MongoDB connector moves from singleton pattern to explicit Client lifecycle management; circuit breaker adds metrics and health-check integrations; context utilities add cloning and nil-safety features. Documentation is substantially reorganized with migration guides and project rules. Configuration files are updated for linting, release, and test automation.

[coderabbitai]

Actionable comments posted: 29

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

.github/workflows/go-combined-analysis.yml (1)

54-57: 🧹 Nitpick | 🔵 Trivial

Consider pinning gosec version for reproducibility.

Using securego/gosec@master can lead to inconsistent builds as the action updates. Consider pinning to a specific version tag for reproducibility, similar to how the Makefile pins GOSEC_VERSION ?= v2.22.4.

♻️ Proposed fix to pin gosec version

       - name: Gosec Scanner
-        uses: securego/gosec@master
+        uses: securego/gosec@v2.22.4
         with:
           args: ./...

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In @.github/workflows/go-combined-analysis.yml around lines 54 - 57, Replace the
floating action reference securego/gosec@master used in the "Gosec Scanner"
workflow step with a pinned tag matching the project's pinned GoSec version
(e.g., the Makefile's GOSEC_VERSION ?= v2.22.4), so update the uses value to use
that specific tag (securego/gosec@v2.22.4) or reference the GOSEC_VERSION
variable if you source it into the workflow.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@CHANGELOG.md`:
- Around line 1-3: Replace the placeholder CHANGELOG.md with versioned release
sections: add at minimum a v4.0.0 heading that lists "Breaking Changes",
"Migration Steps", and "Key Fixes/Features", then add subsequent release
headings (e.g., v4.0.1, v4.1.0) with short summaries; ensure the top of the file
keeps the existing "Changelog" heading and that each release section includes
concrete items (bullet or short-line entries) describing the behavior changes
and upgrade guidance so consumers can follow migration steps for the v4
migration.

In `@commons/assert/assert.go`:
- Around line 202-207: When logger == nil the stderr fallback currently only
prints msg and drops the assertion context/stack; update the stderr path to
mirror the structured-logger output by emitting the collapsed details string and
the stack trace as well (i.e., print details and string(stack) alongside msg),
while preserving the existing behavior when logger != nil; apply the same change
to the other fallback block that handles msg/details/stack (the second
occurrence around the Halt/goroutine flow) so both non-logger paths include
details and stack for diagnostics.
- Around line 141-155: In Asserter.NoError, replace the direct nil check (err ==
nil) with a call to isNil(err) so interface-wrapped nils are detected and
NoError returns nil; do this before any use of err so you never call err.Error()
on a typed-nil. Update the early-return condition to use isNil(err) and leave
the rest of the function (building kvWithError, using err.Error(), and calling
asserter.fail) unchanged so behavior matches NotNil and avoids panics.

In `@commons/assert/benchmark_test.go`:
- Around line 17-37: Benchmarks like BenchmarkThat_True and
BenchmarkThat_TrueWithContext repeatedly call context.Background() inside tight
loops, adding noise; hoist a single ctx := context.Background() (or separate
ctxs where different contexts are used) outside each b.N loop and pass that ctx
into asserter.That (and similarly for other benchmark helpers referenced around
lines 44-46, 65-67, 72-74, 148-156) so the timed section measures only the
assertion/predicate work rather than context allocation.

In `@commons/assert/predicates_test.go`:
- Around line 216-217: The test case entries in predicates_test.go that use
now.Add(time.Second) (the "one second future invalid" case in the test table)
are flaky in CI; update that entry to use a wider offset (e.g., time.Minute or
time.Hour) so the value remains in the future during evaluation—locate the table
of test cases in predicates_test.go and replace time.Second with a larger
duration for the future-case rows (e.g., change now.Add(time.Second) to
now.Add(time.Minute)).

In `@commons/assert/predicates.go`:
- Around line 311-335: Both predicates currently treat blank/whitespace-only
entries as valid; update TransactionHasOperations and
TransactionOperationsContain to first trim entries and drop any empty strings
before evaluating. Specifically, in TransactionHasOperations iterate the input
slice, apply strings.TrimSpace and count only non-empty tokens (return true only
if at least one non-empty token exists). In TransactionOperationsContain build
the allowedSet from trimmed non-empty allowed tokens and iterate trimmed
non-empty operations (return false if either filtered list is empty) so blank
tokens are ignored for both presence and subset checks.

In `@commons/circuitbreaker/healthchecker.go`:
- Around line 206-210: Update the OnStateChange comment on healthChecker to
state that the incoming context parameter is intentionally ignored (hence the
signature uses _ context.Context) because health-check work is scheduled to run
from context.Background() and its lifecycle is controlled by stopChan rather
than the caller's request context; reference the healthChecker.OnStateChange
method, mention context.Background() and stopChan to clarify why the request
context isn't used.

In `@commons/circuitbreaker/manager_test.go`:
- Around line 355-369: TestGetOrCreate_ReturnExistingBreaker currently only
compares cb1.State() and cb2.State(), which can be equal for two distinct
instances; change the test to prove reuse by either asserting pointer/identity
equality of cb1 and cb2 returned from Manager.GetOrCreate or by mutating shared
mutable state on cb1 (e.g., call a method that changes state such as Trip/Fail
or increment a failure counter exposed by the breaker) and then asserting the
same change is visible on cb2; update the assertions in
TestGetOrCreate_ReturnExistingBreaker to use identity comparison or the mutation
check so it verifies real reuse of the breaker instance.
- Around line 325-328: The test double mockStateChangeListener currently
discards the listener context in OnStateChange; update the mock so its callback
signature (onStateChangeFn) accepts context.Context and invoke it with the
received ctx (i.e., change mockStateChangeListener.OnStateChange to call
m.onStateChangeFn(ctx, serviceName, from, to) when non-nil). This preserves and
exercises the timeout-bound context propagation in tests (look for
mockStateChangeListener and onStateChangeFn references).

In `@commons/circuitbreaker/types.go`:
- Around line 72-94: Change the receiver of the Config validation method from a
value receiver to a pointer receiver to follow project guidelines: update the
method signature for Validate on type Config (function name Validate, receiver
Config) to use *Config instead of Config and ensure any callers still work with
the pointer receiver; no logic changes inside the method are needed.

In `@commons/constants/headers.go`:
- Around line 14-17: The DSL and FileExtension constants are misplaced in the
HTTP headers file; move the exported constants DSL and FileExtension into a new
file (e.g., file.go or dsl.go) within the same constants package, remove them
from headers.go, keep the same package name and identifiers, and update any
references across the codebase to point to the constants package as before so
compilation is unaffected.
- Around line 20-21: HeaderTraceparent and HeaderTraceparentPascal currently
have identical values; pick one of two fixes: either consolidate by removing
HeaderTraceparentPascal and replace its callers to use HeaderTraceparent (keep
the W3C doc comment on HeaderTraceparent), or make the distinction explicit by
changing HeaderTraceparentPascal to the gRPC/metadata form you intend (e.g.,
lowercase "traceparent") and update its doc comment to state "for gRPC
metadata". Do the same for the analogous pair HeaderTracestate /
HeaderTracestatePascal. Ensure all references in code are updated to the chosen
constant names so there are no duplicates or mismatched usages.

In `@commons/context_test.go`:
- Around line 164-170: Replace value equality checks with identity checks for
the injected logger: in the test that calls ContextWithLogger and
NewLoggerFromContext (and the other two assertions noted), change the
assert.Equal calls to an identity assertion (e.g., assert.Same or require.Same)
so the test verifies the exact logger instance (nop := &log.NopLogger{})
returned from NewLoggerFromContext, and update the other two logger assertions
in this file (the ones around the calls to
ContextWithLogger/NewLoggerFromContext at the other locations) the same way.

In `@commons/context.go`:
- Around line 46-57: The current interface-level nil checks can miss typed-nil
implementations; update NewLoggerFromContext (and the other guard around the
local logger variable referenced in the review) to detect typed-nil values by
doing a type switch or type assertion on CustomContextKeyValue.Logger (check
concrete types such as *GoLogger, *zap.Logger, *log.NopLogger) and ensure the
underlying pointer is non-nil before returning it; if the asserted pointer is
nil or the type is unknown, return &log.NopLogger{} instead. Ensure you apply
the same typed-nil detection logic where the review points to the other logger
check so no typed-nil loggers escape the guard.

In `@commons/cron/cron_test.go`:
- Around line 142-194: Multiple near-duplicate tests
(TestParse_InvalidExpression, TestParse_EmptyString, TestParse_TooFewFields,
TestParse_TooManyFields, TestParse_OutOfRangeValue, TestParse_InvalidStep) all
call Parse and assert ErrInvalidExpression; consolidate them into one
table-driven test (e.g., TestParse_InvalidExpressions) that defines a slice of
cases with name and expr, ranges over them with t.Run and t.Parallel per
subtest, calls Parse(tc.expr) and uses require.Error + assert.ErrorIs(err,
ErrInvalidExpression); remove the individual TestParse_* functions to avoid
duplication and keep the test suite concise.

In `@commons/crypto/crypto_test.go`:
- Line 14: Add a gitleaks/secret-scanner allowlist entry for the deterministic
test fixture constant validHexKey in crypto_test.go so the scanner ignores this
known test key; update the repository's secret-scan config (allowlist rules) to
permit patterns or specific matches for *_test.go files or the exact constant
name validHexKey so CI gitleaks no longer flags this intentional test fixture.

In `@commons/errgroup/errgroup_test.go`:
- Around line 160-163: The test currently relies on time.Sleep inside the first
group.Go goroutine to make its panic occur before the other goroutine, which is
flaky; replace the time-based ordering with deterministic channel coordination:
add an "allowPanic" (or "proceed") channel, have the panic goroutine wait on
<-allowPanic instead of time.Sleep, signal readiness via the existing started
channel, then in the test close(allowPanic) (or send to it) before starting or
unblocking the second goroutine so the panic always happens first; update the
goroutine bodies referenced (group.Go, the panic goroutine, and the other
goroutine that returns an error) to use started and allowPanic to enforce
deterministic ordering.

In `@commons/errgroup/errgroup.go`:
- Around line 69-71: The WithContext function currently calls
context.WithCancel(ctx) which will panic if ctx is nil; update WithContext to
nil-guard the input by replacing a nil ctx with context.Background() before
calling context.WithCancel. Specifically, in WithContext check if ctx == nil
then set ctx = context.Background(), then call context.WithCancel(ctx) and
return &Group{ctx: ctx, cancel: cancel}, ctx so Group and its cancel remain
correct.

In `@commons/internal/nilcheck/nilcheck_test.go`:
- Around line 21-49: The TestInterface function is not table-driven; convert it
to a table-driven test to match PROJECT_RULES.md: create a slice of test cases
with fields (name, value, wantBool) that includes all current inputs (nil,
nilPointer, nilSlice, nilMap, nilChan, nilFunc, nilIface, typedNilIface, 0, "",
sampleStruct{}, &sampleStruct{}, []string{}) and iterate them with t.Run,
asserting Interface(tc.value) equals tc.wantBool; keep t.Parallel where
appropriate and reference the existing TestInterface test and the Interface
function when implementing the new table-driven structure.

In `@commons/internal/nilcheck/nilcheck.go`:
- Around line 14-15: The switch that returns v.IsNil() for nil-capable kinds
(case reflect.Chan, reflect.Func, reflect.Interface, reflect.Map,
reflect.Pointer, reflect.Slice) must include reflect.UnsafePointer so
unsafe.Pointer(nil) is treated as nil; update the case in the nil-checking
function in commons/internal/nilcheck/nilcheck.go to include
reflect.UnsafePointer and add a unit test that asserts the function returns true
for unsafe.Pointer(nil) to prevent regressions.

In `@commons/log/log.go`:
- Line 9: Update the package comment for the Logger symbol to clarify that "v2"
refers to the internal structured-logging API generation, not the Go module
semantic version; specifically edit the comment line containing "Logger is the
package interface for v2 logging." to mention that v2 denotes the evolved
structured logging interface (see commons/log/doc.go) and is independent of the
module's v4 versioning so readers won't confuse API generation version with Go
module versioning.

In `@commons/log/sanitizer.go`:
- Around line 8-15: The SafeError function currently forwards the
caller-supplied msg into logs even when production=true; change SafeError to
avoid emitting msg text in the production branch and instead log only the
error's type/name (and any minimal context like an obfuscated field summary),
ensuring no raw caller message or sensitive payloads are included; locate
SafeError (and the other production branches that currently forward msg in the
same file) and replace direct msg forwarding with a sanitized summary (error
type and obfuscated-sensitive-fields) or a fixed production-safe message.

In `@commons/mongo/connection_string.go`:
- Around line 56-65: The current code mutates the caller's cfg.Query when adding
authSource; to fix, implement a cloneValues helper that deep-copies a url.Values
(copying slices) and use it whenever you need to modify cfg.Query: replace
direct use of cfg.Query in the branch that checks query == nil ||
!query.Has("authSource") by first cloning (if cfg.Query != nil) and then calling
Set("authSource", "admin") on the clone so the original cfg.Query remains
unchanged; reference the local variable query and the new cloneValues function
when making this change.

In `@commons/mongo/mongo_integration_test.go`:
- Around line 47-52: The cleanup closure currently calls require.NoError(t,
container.Terminate(closeCtx)) which can fail the test after it already passed;
change the closure to capture the error from container.Terminate(closeCtx) and
log it instead of asserting, e.g. call container.Terminate with the same
closeCtx/closeCancel and if err != nil use t.Logf or t.Errorf to record
"container termination failed: %v" (referencing container.Terminate, closeCtx,
closeCancel and the anonymous cleanup function) so teardown failures are
reported but do not mark the test as failed.

In `@Makefile`:
- Line 83: Add clear inline documentation and usage guidance for the
RETRY_ON_FAIL Makefile variable: update the Makefile near the RETRY_ON_FAIL
definition to include a comment describing its purpose, allowed values, and
recommended use cases (only for transient infra issues like container startup in
integration tests), and add a short note in the project README or CONTRIBUTING
that warns that enabling RETRY_ON_FAIL can mask flaky tests and should be used
sparingly with examples of acceptable use (e.g., retry count 1-3 for CI jobs
that depend on non-deterministic external services). Ensure references to
RETRY_ON_FAIL appear alongside any test or CI targets that honor it (search for
its usage in test targets) and clarify the default value and how to override it
in CI pipelines.
- Around line 77-79: The Makefile's GOLANGCI_LINT_VERSION variable is misaligned
with the CI workflow; update the GOLANGCI_LINT_VERSION definition (symbol:
GOLANGCI_LINT_VERSION) to match the version used in the workflow (v2.11.2) so
local and CI linting use the same golangci-lint release, and ensure any related
references (e.g., in targets that consume GOLANGCI_LINT_VERSION) continue to
work with the updated value.

In `@README.md`:
- Line 54: Update the README line describing the package (the paragraph that
starts with "commons/net/http/ratelimit") to hyphenate the compound adjective:
change "Redis-backed distributed rate limiting middleware" to "Redis-backed
distributed rate-limiting middleware"; keep the rest of the sentence intact (the
list of symbols like New, WithDefaultRateLimit, WithRateLimit,
WithDynamicRateLimit, MethodTierSelector,
DefaultTier/AggressiveTier/RelaxedTier,
IdentityFromIP/IdentityFromHeader/IdentityFromIPAndHeader, WithOnLimited,
RedisStorage/NewRedisStorage should remain unchanged).

In `@REVIEW.md`:
- Line 3: The report currently preserves intentionally duplicated findings;
update the generation step that produces the "Generated from 54 reviewer-agent
runs..." section to deduplicate entries by a canonical key composed of file path
+ symbol (e.g., function/class/line identifier) + root-cause, collapsing
duplicates into a single entry and adding an occurrence_count field for how many
times it appeared; locate the report assembly logic that emits the findings list
(the code that produces the "Generated from ..." block in REVIEW.md) and change
it to build a map keyed by (file, symbol, root-cause), emit one canonical entry
per key with occurrence_count, and omit the original duplicate entries.
- Around line 8-388: The review file uses non-repo-relative paths like
references/lib-commons/... which are hard to navigate; update all findings in
REVIEW.md to use repository-relative paths (e.g. commons/..., net/http/...,
opentelemetry/..., tenant-manager/...) so code-review tooling links resolve
directly. Locate and replace the repeated prefix "references/lib-commons/"
across the document (search for that exact token and the listed occurrences such
as commons/opentelemetry/otel.go, commons/net/http/withTelemetry.go,
commons/tenant-manager/middleware/tenant.go, etc.), normalize any path
formatting/commas/line-wrapped lists to valid repo-relative file paths, and
add/adjust any failing links or anchors; optionally add a small validation (CI
lint) that REVIEW.md paths are repo-relative to prevent regressions.

---

Outside diff comments:
In @.github/workflows/go-combined-analysis.yml:
- Around line 54-57: Replace the floating action reference securego/gosec@master
used in the "Gosec Scanner" workflow step with a pinned tag matching the
project's pinned GoSec version (e.g., the Makefile's GOSEC_VERSION ?= v2.22.4),
so update the uses value to use that specific tag (securego/gosec@v2.22.4) or
reference the GOSEC_VERSION variable if you source it into the workflow.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 85363ac0-bf44-4097-b65a-0b3ce94b16ce

📥 Commits

Reviewing files that changed from the base of the PR and between 5ed6219 and 977411b.

📒 Files selected for processing (300)

• .githooks/commit-msg/commit-msg
• .githooks/pre-commit/pre-commit
• .githooks/pre-push/pre-push
• .githooks/pre-receive/pre-receive
• .github/workflows/go-combined-analysis.yml
• .gitignore
• .golangci.yml
• .goreleaser.yml
• AGENTS.md
• CHANGELOG.md
• CLAUDE.md
• MIGRATION_MAP.md
• Makefile
• README.md
• REVIEW.md
• commons/app.go
• commons/app_test.go
• commons/assert/assert.go
• commons/assert/assert_extended_test.go
• commons/assert/assert_test.go
• commons/assert/benchmark_test.go
• commons/assert/doc.go
• commons/assert/predicates.go
• commons/assert/predicates_test.go
• commons/backoff/backoff.go
• commons/backoff/backoff_test.go
• commons/backoff/doc.go
• commons/circuitbreaker/config.go
• commons/circuitbreaker/doc.go
• commons/circuitbreaker/fallback_example_test.go
• commons/circuitbreaker/healthchecker.go
• commons/circuitbreaker/healthchecker_test.go
• commons/circuitbreaker/manager.go
• commons/circuitbreaker/manager_example_test.go
• commons/circuitbreaker/manager_metrics_test.go
• commons/circuitbreaker/manager_test.go
• commons/circuitbreaker/types.go
• commons/circuitbreaker/types_test.go
• commons/constants/datasource.go
• commons/constants/doc.go
• commons/constants/errors.go
• commons/constants/headers.go
• commons/constants/log.go
• commons/constants/metadata.go
• commons/constants/obfuscation.go
• commons/constants/opentelemetry.go
• commons/constants/opentelemetry_test.go
• commons/constants/pagination.go
• commons/constants/response.go
• commons/constants/transaction.go
• commons/context.go
• commons/context_clone_test.go
• commons/context_example_test.go
• commons/context_test.go
• commons/cron/cron.go
• commons/cron/cron_test.go
• commons/cron/doc.go
• commons/crypto/crypto.go
• commons/crypto/crypto_nil_test.go
• commons/crypto/crypto_test.go
• commons/crypto/doc.go
• commons/doc.go
• commons/errgroup/doc.go
• commons/errgroup/errgroup.go
• commons/errgroup/errgroup_nil_test.go
• commons/errgroup/errgroup_test.go
• commons/errors.go
• commons/errors_test.go
• commons/internal/nilcheck/nilcheck.go
• commons/internal/nilcheck/nilcheck_test.go
• commons/jwt/doc.go
• commons/jwt/jwt.go
• commons/jwt/jwt_test.go
• commons/license/doc.go
• commons/license/manager.go
• commons/license/manager_nil_test.go
• commons/license/manager_test.go
• commons/log/doc.go
• commons/log/go_logger.go
• commons/log/log.go
• commons/log/log_example_test.go
• commons/log/log_mock.go
• commons/log/log_test.go
• commons/log/nil.go
• commons/log/sanitizer.go
• commons/log/sanitizer_test.go
• commons/mongo/connection_string.go
• commons/mongo/connection_string_example_test.go
• commons/mongo/connection_string_test.go
• commons/mongo/doc.go
• commons/mongo/mongo.go
• commons/mongo/mongo_integration_test.go
• commons/mongo/mongo_test.go
• commons/net/http/context.go
• commons/net/http/context_nil_error_test.go
• commons/net/http/context_ownership.go
• commons/net/http/context_span.go
• commons/net/http/context_test.go
• commons/net/http/cursor.go
• commons/net/http/cursor_example_test.go
• commons/net/http/cursor_test.go
• commons/net/http/doc.go
• commons/net/http/error.go
• commons/net/http/error_test.go
• commons/net/http/handler.go
• commons/net/http/handler_nil_test.go
• commons/net/http/handler_test.go
• commons/net/http/health.go
• commons/net/http/health_config_test.go
• commons/net/http/health_integration_test.go
• commons/net/http/health_test.go
• commons/net/http/matcher_response.go
• commons/net/http/matcher_response_test.go
• commons/net/http/middleware_example_test.go
• commons/net/http/pagination.go
• commons/net/http/pagination_cursor_timestamp_test.go
• commons/net/http/pagination_cursor_uuid_test.go
• commons/net/http/pagination_sort.go
• commons/net/http/pagination_sort_test.go
• commons/net/http/pagination_strict_test.go
• commons/net/http/pagination_test.go
• commons/net/http/pagination_timestamp.go
• commons/net/http/proxy.go
• commons/net/http/proxy_defensive_test.go
• commons/net/http/proxy_forwarding_test.go
• commons/net/http/proxy_ssrf_test.go
• commons/net/http/proxy_test.go
• commons/net/http/proxy_transport.go
• commons/net/http/proxy_transport_test.go
• commons/net/http/proxy_validation.go
• commons/net/http/ratelimit/doc.go
• commons/net/http/ratelimit/middleware.go
• commons/net/http/ratelimit/middleware_options.go
• commons/net/http/ratelimit/middleware_test.go
• commons/net/http/ratelimit/redis_storage.go
• commons/net/http/ratelimit/redis_storage_integration_test.go
• commons/net/http/ratelimit/redis_storage_test.go
• commons/net/http/ratelimit/server_test.go
• commons/net/http/response.go
• commons/net/http/response_test.go
• commons/net/http/validation.go
• commons/net/http/validation_parse_test.go
• commons/net/http/validation_query_test.go
• commons/net/http/validation_rules_amount_test.go
• commons/net/http/validation_rules_field_test.go
• commons/net/http/validation_test.go
• commons/net/http/withBasicAuth.go
• commons/net/http/withBasicAuth_test.go
• commons/net/http/withCORS.go
• commons/net/http/withCORS_test.go
• commons/net/http/withLogging.go
• commons/net/http/withLogging_grpc_test.go
• commons/net/http/withLogging_middleware.go
• commons/net/http/withLogging_obfuscation.go
• commons/net/http/withLogging_sanitize.go
• commons/net/http/withLogging_test.go
• commons/net/http/withTelemetry.go
• commons/net/http/withTelemetry_helpers.go
• commons/net/http/withTelemetry_metrics.go
• commons/net/http/withTelemetry_route_test.go
• commons/net/http/withTelemetry_test.go
• commons/opentelemetry/README.md
• commons/opentelemetry/doc.go
• commons/opentelemetry/extract_queue_test.go
• commons/opentelemetry/inject_trace_test.go
• commons/opentelemetry/metrics/METRICS_USAGE.md
• commons/opentelemetry/metrics/account.go
• commons/opentelemetry/metrics/builders.go
• commons/opentelemetry/metrics/doc.go
• commons/opentelemetry/metrics/labels.go
• commons/opentelemetry/metrics/metrics.go
• commons/opentelemetry/metrics/operation_routes.go
• commons/opentelemetry/metrics/system.go
• commons/opentelemetry/metrics/system_test.go
• commons/opentelemetry/metrics/transaction.go
• commons/opentelemetry/metrics/transaction_routes.go
• commons/opentelemetry/metrics/v2_test.go
• commons/opentelemetry/obfuscation.go
• commons/opentelemetry/obfuscation_example_test.go
• commons/opentelemetry/obfuscation_test.go
• commons/opentelemetry/otel.go
• commons/opentelemetry/otel_example_test.go
• commons/opentelemetry/otel_test.go
• commons/opentelemetry/processor.go
• commons/opentelemetry/processor_test.go
• commons/opentelemetry/queue_trace_example_test.go
• commons/opentelemetry/queue_trace_test.go
• commons/opentelemetry/v2_test.go
• commons/os.go
• commons/os_test.go
• commons/outbox/classifier.go
• commons/outbox/config.go
• commons/outbox/config_test.go
• commons/outbox/dispatcher.go
• commons/outbox/dispatcher_test.go
• commons/outbox/doc.go
• commons/outbox/errors.go
• commons/outbox/event.go
• commons/outbox/event_test.go
• commons/outbox/handler.go
• commons/outbox/handler_test.go
• commons/outbox/metrics.go
• commons/outbox/metrics_test.go
• commons/outbox/postgres/column_resolver.go
• commons/outbox/postgres/column_resolver_test.go
• commons/outbox/postgres/db.go
• commons/outbox/postgres/db_test.go
• commons/outbox/postgres/doc.go
• commons/outbox/postgres/migrations/000001_outbox_events_schema.down.sql
• commons/outbox/postgres/migrations/000001_outbox_events_schema.up.sql
• commons/outbox/postgres/migrations/README.md
• commons/outbox/postgres/migrations/column/000001_outbox_events_column.down.sql
• commons/outbox/postgres/migrations/column/000001_outbox_events_column.up.sql
• commons/outbox/postgres/repository.go
• commons/outbox/postgres/repository_integration_test.go
• commons/outbox/postgres/repository_test.go
• commons/outbox/postgres/schema_resolver.go
• commons/outbox/postgres/schema_resolver_test.go
• commons/outbox/repository.go
• commons/outbox/sanitizer.go
• commons/outbox/sanitizer_test.go
• commons/outbox/status.go
• commons/outbox/status_test.go
• commons/outbox/tenant.go
• commons/outbox/tenant_test.go
• commons/pointers/doc.go
• commons/pointers/pointers.go
• commons/pointers/pointers_test.go
• commons/postgres/doc.go
• commons/postgres/migration_integration_test.go
• commons/postgres/pagination.go
• commons/postgres/postgres.go
• commons/postgres/postgres_integration_test.go
• commons/postgres/postgres_test.go
• commons/postgres/resilience_integration_test.go
• commons/rabbitmq/dlq.go
• commons/rabbitmq/dlq_test.go
• commons/rabbitmq/doc.go
• commons/rabbitmq/publisher.go
• commons/rabbitmq/publisher_test.go
• commons/rabbitmq/rabbitmq.go
• commons/rabbitmq/rabbitmq_integration_test.go
• commons/rabbitmq/rabbitmq_test.go
• commons/rabbitmq/trace_propagation_integration_test.go
• commons/redis/doc.go
• commons/redis/iam_example_test.go
• commons/redis/lock.go
• commons/redis/lock_integration_test.go
• commons/redis/lock_interface.go
• commons/redis/lock_test.go
• commons/redis/redis.go
• commons/redis/redis_example_test.go
• commons/redis/redis_integration_test.go
• commons/redis/redis_test.go
• commons/redis/resilience_integration_test.go
• commons/runtime/doc.go
• commons/runtime/error_reporter.go
• commons/runtime/error_reporter_test.go
• commons/runtime/example_test.go
• commons/runtime/goroutine.go
• commons/runtime/goroutine_test.go
• commons/runtime/helpers_test.go
• commons/runtime/log_mode_link_test.go
• commons/runtime/metrics.go
• commons/runtime/metrics_test.go
• commons/runtime/policy.go
• commons/runtime/policy_test.go
• commons/runtime/recover.go
• commons/runtime/recover_test.go
• commons/runtime/tracing.go
• commons/runtime/tracing_test.go
• commons/safe/doc.go
• commons/safe/math.go
• commons/safe/math_test.go
• commons/safe/regex.go
• commons/safe/regex_example_test.go
• commons/safe/regex_test.go
• commons/safe/safe_example_test.go
• commons/safe/slice.go
• commons/safe/slice_test.go
• commons/secretsmanager/m2m.go
• commons/secretsmanager/m2m_test.go
• commons/security/doc.go
• commons/security/sensitive_fields.go
• commons/security/sensitive_fields_test.go
• commons/server/doc.go
• commons/server/grpc_test.go
• commons/server/shutdown.go
• commons/server/shutdown_example_test.go
• commons/server/shutdown_integration_test.go
• commons/server/shutdown_test.go
• commons/shell/logo.txt
• commons/shell/makefile_colors.mk
• commons/shell/makefile_utils.mk
• commons/stringUtils.go
• commons/stringUtils_test.go
• commons/tenant-manager/cache/config_cache.go
• commons/tenant-manager/cache/memory.go
• commons/tenant-manager/cache/memory_test.go
• commons/tenant-manager/client/client.go

💤 Files with no reviewable changes (7)

• commons/constants/datasource.go
• commons/constants/obfuscation.go
• commons/circuitbreaker/config.go
• .githooks/pre-receive/pre-receive
• .githooks/pre-commit/pre-commit
• .githooks/commit-msg/commit-msg
• .githooks/pre-push/pre-push

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Restore versioned release notes instead of a placeholder-only changelog.

This change removes all historical/versioned entries, which is risky for a major v4 migration with breaking and behavioral changes. Please add concrete release sections (at least v4.0.0 and subsequent tags) with breaking changes, migration steps, and key fixes/features so consumers can safely upgrade.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@CHANGELOG.md` around lines 1 - 3, Replace the placeholder CHANGELOG.md with
versioned release sections: add at minimum a v4.0.0 heading that lists "Breaking
Changes", "Migration Steps", and "Key Fixes/Features", then add subsequent
release headings (e.g., v4.0.1, v4.1.0) with short summaries; ensure the top of
the file keeps the existing "Changelog" heading and that each release section
includes concrete items (bullet or short-line entries) describing the behavior
changes and upgrade guidance so consumers can follow migration steps for the v4
migration.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

cd /tmp && find . -type f -name "*.go" -path "*/commons/assert/*" 2>/dev/null | head -20

Repository: LerianStudio/lib-commons

Length of output: 50

---

🏁 Script executed:

git ls-files | grep -E "commons/assert.*\.go$"

Repository: LerianStudio/lib-commons

Length of output: 279

---

🏁 Script executed:

cat -n commons/assert/assert.go | sed -n '1,50p'

Repository: LerianStudio/lib-commons

Length of output: 1633

---

🏁 Script executed:

rg "func isNil" --type go

Repository: LerianStudio/lib-commons

Length of output: 624

---

🏁 Script executed:

cat -n commons/assert/assert.go | sed -n '141,155p'

Repository: LerianStudio/lib-commons

Length of output: 763

---

🏁 Script executed:

rg "func isNil\(v any\)" -A 10 commons/assert/assert.go

Repository: LerianStudio/lib-commons

Length of output: 314

---

🏁 Script executed:

cat -n commons/assert/assert.go | sed -n '1,100p' | tail -50

Repository: LerianStudio/lib-commons

Length of output: 1716

---

🏁 Script executed:

cat -n commons/assert/assert.go | head -300 | tail -150

Repository: LerianStudio/lib-commons

Length of output: 5654

---

🏁 Script executed:

rg "isNil\(" commons/assert/assert.go -B 2 -A 2

Repository: LerianStudio/lib-commons

Length of output: 375

---

🏁 Script executed:

cat -n commons/assert/assert.go | grep -A 15 "func (asserter \*Asserter)"

Repository: LerianStudio/lib-commons

Length of output: 4401

---

🏁 Script executed:

cat -n commons/assert/assert.go | grep -B 5 "func isNil"

Repository: LerianStudio/lib-commons

Length of output: 293

---

🏁 Script executed:

rg "NoError\(" commons/assert/assert_test.go -A 5 | head -40

Repository: LerianStudio/lib-commons

Length of output: 1492

---

Use isNil(err) instead of err == nil to handle typed-nil errors.

Line 142's direct nil check misses interface-wrapped nil pointers, allowing line 151 to call Error() on a nil receiver and panic. The isNil() function already handles this correctly (used in NotNil at line 111) and should be applied here for consistency.

Suggested fix

 func (asserter *Asserter) NoError(ctx context.Context, err error, msg string, kv ...any) error {
-	if err == nil {
+	if isNil(err) {
 		return nil
 	}
 
 	// Prepend error and error_type to key-value pairs for richer debugging

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	func (asserter *Asserter) NoError(ctx context.Context, err error, msg string, kv ...any) error {
	if err == nil {
	return nil
	}
	// Prepend error and error_type to key-value pairs for richer debugging
	// errorKVPairs: 2 pairs added (error + error_type), each pair = 2 elements
	const errorKVPairs = 4
	kvWithError := make([]any, 0, len(kv)+errorKVPairs)
	kvWithError = append(kvWithError, "error", err.Error())
	kvWithError = append(kvWithError, "error_type", fmt.Sprintf("%T", err))
	kvWithError = append(kvWithError, kv...)
	return asserter.fail(ctx, "NoError", msg, kvWithError...)
	func (asserter *Asserter) NoError(ctx context.Context, err error, msg string, kv ...any) error {
	if isNil(err) {
	return nil
	}
	// Prepend error and error_type to key-value pairs for richer debugging
	// errorKVPairs: 2 pairs added (error + error_type), each pair = 2 elements
	const errorKVPairs = 4
	kvWithError := make([]any, 0, len(kv)+errorKVPairs)
	kvWithError = append(kvWithError, "error", err.Error())
	kvWithError = append(kvWithError, "error_type", fmt.Sprintf("%T", err))
	kvWithError = append(kvWithError, kv...)
	return asserter.fail(ctx, "NoError", msg, kvWithError...)
	}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@commons/assert/assert.go` around lines 141 - 155, In Asserter.NoError,
replace the direct nil check (err == nil) with a call to isNil(err) so
interface-wrapped nils are detected and NoError returns nil; do this before any
use of err so you never call err.Error() on a typed-nil. Update the early-return
condition to use isNil(err) and leave the rest of the function (building
kvWithError, using err.Error(), and calling asserter.fail) unchanged so behavior
matches NotNil and avoids panics.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Keep the stderr fallback as informative as the structured logger path.

When logger == nil, Line 315 prints only msg, and Lines 202-207 drop the stack entirely. In goroutine + Halt flows this can be the only surviving diagnostic output, so the stderr path should emit the existing details string and stack too.

Suggested fix

-	if len(stack) > 0 && logger != nil {
-		logger.Log(context.Background(), log.LevelError, "assertion stack trace",
-			log.String("assertion_type", assertion),
-			log.String("stack_trace", string(stack)),
-		)
+	if len(stack) > 0 {
+		if logger != nil {
+			logger.Log(context.Background(), log.LevelError, "assertion stack trace",
+				log.String("assertion_type", assertion),
+				log.String("stack_trace", string(stack)),
+			)
+		} else {
+			fmt.Fprintln(os.Stderr, "ASSERTION STACK TRACE:")
+			fmt.Fprintln(os.Stderr, string(stack))
+		}
 	}
@@
 	if logger == nil {
-		// Fall back to stderr for emergency visibility
-		fmt.Fprintln(os.Stderr, "ASSERTION FAILED: "+msg)
+		formatted := "ASSERTION FAILED: " + msg
+		if details != "" {
+			formatted += "\n" + details
+		}
+		fmt.Fprintln(os.Stderr, formatted)
 
 		return
 	}

Based on learnings, assertion key-value context should stay collapsed into the opaque `details` string.

Also applies to: 312-318

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@commons/assert/assert.go` around lines 202 - 207, When logger == nil the
stderr fallback currently only prints msg and drops the assertion context/stack;
update the stderr path to mirror the structured-logger output by emitting the
collapsed details string and the stack trace as well (i.e., print details and
string(stack) alongside msg), while preserving the existing behavior when logger
!= nil; apply the same change to the other fallback block that handles
msg/details/stack (the second occurrence around the Halt/goroutine flow) so both
non-logger paths include details and stack for diagnostics.

[coderabbitai]

🧹 Nitpick | 🔵 Trivial

Hoist context.Background() out of tight benchmark loops.

This keeps measurements focused on assertion/predicate cost rather than repeated setup calls.

📉 Benchmark noise reduction pattern

 func BenchmarkThat_True(b *testing.B) {
+ ctx := context.Background()
- asserter := New(context.Background(), nil, "", "")
+ asserter := New(ctx, nil, "", "")
   for i := 0; i < b.N; i++ {
-    _ = asserter.That(context.Background(), true, "benchmark test")
+    _ = asserter.That(ctx, true, "benchmark test")
   }
 }

Apply the same pattern to the other asserter.* benchmark loops.

Also applies to: 44-46, 65-67, 72-74, 148-156

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@commons/assert/benchmark_test.go` around lines 17 - 37, Benchmarks like
BenchmarkThat_True and BenchmarkThat_TrueWithContext repeatedly call
context.Background() inside tight loops, adding noise; hoist a single ctx :=
context.Background() (or separate ctxs where different contexts are used)
outside each b.N loop and pass that ctx into asserter.That (and similarly for
other benchmark helpers referenced around lines 44-46, 65-67, 72-74, 148-156) so
the timed section measures only the assertion/predicate work rather than context
allocation.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Use a wider future offset to prevent timing flakes.

The one-second future case is brittle in parallel CI; it can become “not future” before evaluation.

🧪 Stable test tweak

- {"one second future invalid", now.Add(time.Second), false},
+ {"one minute future invalid", now.Add(time.Minute), false},

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	{"one second future invalid", now.Add(time.Second), false},
	{"one hour future invalid", now.Add(time.Hour), false},
	{"one minute future invalid", now.Add(time.Minute), false},
	{"one hour future invalid", now.Add(time.Hour), false},

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@commons/assert/predicates_test.go` around lines 216 - 217, The test case
entries in predicates_test.go that use now.Add(time.Second) (the "one second
future invalid" case in the test table) are flaky in CI; update that entry to
use a wider offset (e.g., time.Minute or time.Hour) so the value remains in the
future during evaluation—locate the table of test cases in predicates_test.go
and replace time.Second with a larger duration for the future-case rows (e.g.,
change now.Add(time.Second) to now.Add(time.Minute)).

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Version mismatch: golangci-lint in Makefile vs workflow.

GOLANGCI_LINT_VERSION is pinned to v2.1.6 here, but the workflow (.github/workflows/go-combined-analysis.yml) uses v2.11.2. This mismatch could cause different linting behavior locally vs CI.

🔧 Proposed fix to align versions

-GOLANGCI_LINT_VERSION ?= v2.1.6
+GOLANGCI_LINT_VERSION ?= v2.11.2

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	GOTESTSUM_VERSION ?= v1.12.0
	GOSEC_VERSION ?= v2.22.4
	GOLANGCI_LINT_VERSION ?= v2.1.6
	GOTESTSUM_VERSION ?= v1.12.0
	GOSEC_VERSION ?= v2.22.4
	GOLANGCI_LINT_VERSION ?= v2.11.2

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@Makefile` around lines 77 - 79, The Makefile's GOLANGCI_LINT_VERSION variable
is misaligned with the CI workflow; update the GOLANGCI_LINT_VERSION definition
(symbol: GOLANGCI_LINT_VERSION) to match the version used in the workflow
(v2.11.2) so local and CI linting use the same golangci-lint release, and ensure
any related references (e.g., in targets that consume GOLANGCI_LINT_VERSION)
continue to work with the updated value.

[coderabbitai]

🧹 Nitpick | 🔵 Trivial

Consider documenting RETRY_ON_FAIL usage carefully.

The retry mechanism can mask flaky tests. Ensure this is used intentionally (e.g., for transient container startup issues in integration tests) rather than to hide genuine test failures.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@Makefile` at line 83, Add clear inline documentation and usage guidance for
the RETRY_ON_FAIL Makefile variable: update the Makefile near the RETRY_ON_FAIL
definition to include a comment describing its purpose, allowed values, and
recommended use cases (only for transient infra issues like container startup in
integration tests), and add a short note in the project README or CONTRIBUTING
that warns that enabling RETRY_ON_FAIL can mask flaky tests and should be used
sparingly with examples of acceptable use (e.g., retry count 1-3 for CI jobs
that depend on non-deterministic external services). Ensure references to
RETRY_ON_FAIL appear alongside any test or CI targets that honor it (search for
its usage in test targets) and clarify the default value and how to override it
in CI pipelines.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Use hyphenation for the compound adjective on this line.
For readability/grammar consistency, prefer “Redis-backed distributed rate-limiting middleware”.

✏️ Suggested doc tweak

-- `commons/net/http/ratelimit`: Redis-backed distributed rate limiting middleware for Fiber — `New(conn, opts...)` returns a `*RateLimiter` (nil when disabled, nil-safe for pass-through), `WithDefaultRateLimit(conn, opts...)` as a one-liner that wires `New` + `DefaultTier` into a ready-to-use `fiber.Handler`, fixed-window counter via atomic Lua script (INCR + PEXPIRE), `WithRateLimit(tier)` for static tiers, `WithDynamicRateLimit(TierFunc)` for per-request tier selection, `MethodTierSelector` for write-vs-read split, preset tiers (`DefaultTier` / `AggressiveTier` / `RelaxedTier`) configurable via env vars, identity extractors (`IdentityFromIP` / `IdentityFromHeader` / `IdentityFromIPAndHeader` — uses `#` separator to avoid conflict with IPv6 colons), fail-open/fail-closed policy, `WithOnLimited` callback, and standard `X-RateLimit-*` / `Retry-After` headers; also exports `RedisStorage` (`NewRedisStorage`) for use with third-party Fiber middleware
+- `commons/net/http/ratelimit`: Redis-backed distributed rate-limiting middleware for Fiber — `New(conn, opts...)` returns a `*RateLimiter` (nil when disabled, nil-safe for pass-through), `WithDefaultRateLimit(conn, opts...)` as a one-liner that wires `New` + `DefaultTier` into a ready-to-use `fiber.Handler`, fixed-window counter via atomic Lua script (INCR + PEXPIRE), `WithRateLimit(tier)` for static tiers, `WithDynamicRateLimit(TierFunc)` for per-request tier selection, `MethodTierSelector` for write-vs-read split, preset tiers (`DefaultTier` / `AggressiveTier` / `RelaxedTier`) configurable via env vars, identity extractors (`IdentityFromIP` / `IdentityFromHeader` / `IdentityFromIPAndHeader` — uses `#` separator to avoid conflict with IPv6 colons), fail-open/fail-closed policy, `WithOnLimited` callback, and standard `X-RateLimit-*` / `Retry-After` headers; also exports `RedisStorage` (`NewRedisStorage`) for use with third-party Fiber middleware

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

- `commons/net/http/ratelimit`: Redis-backed distributed rate limiting middleware for Fiber — `New(conn, opts...)` returns a `*RateLimiter` (nil when disabled, nil-safe for pass-through), `WithDefaultRateLimit(conn, opts...)` as a one-liner that wires `New` + `DefaultTier` into a ready-to-use `fiber.Handler`, fixed-window counter via atomic Lua script (INCR + PEXPIRE), `WithRateLimit(tier)` for static tiers, `WithDynamicRateLimit(TierFunc)` for per-request tier selection, `MethodTierSelector` for write-vs-read split, preset tiers (`DefaultTier` / `AggressiveTier` / `RelaxedTier`) configurable via env vars, identity extractors (`IdentityFromIP` / `IdentityFromHeader` / `IdentityFromIPAndHeader` — uses `#` separator to avoid conflict with IPv6 colons), fail-open/fail-closed policy, `WithOnLimited` callback, and standard `X-RateLimit-*` / `Retry-After` headers; also exports `RedisStorage` (`NewRedisStorage`) for use with third-party Fiber middleware

- `commons/net/http/ratelimit`: Redis-backed distributed rate-limiting middleware for Fiber — `New(conn, opts...)` returns a `*RateLimiter` (nil when disabled, nil-safe for pass-through), `WithDefaultRateLimit(conn, opts...)` as a one-liner that wires `New` + `DefaultTier` into a ready-to-use `fiber.Handler`, fixed-window counter via atomic Lua script (INCR + PEXPIRE), `WithRateLimit(tier)` for static tiers, `WithDynamicRateLimit(TierFunc)` for per-request tier selection, `MethodTierSelector` for write-vs-read split, preset tiers (`DefaultTier` / `AggressiveTier` / `RelaxedTier`) configurable via env vars, identity extractors (`IdentityFromIP` / `IdentityFromHeader` / `IdentityFromIPAndHeader` — uses `#` separator to avoid conflict with IPv6 colons), fail-open/fail-closed policy, `WithOnLimited` callback, and standard `X-RateLimit-*` / `Retry-After` headers; also exports `RedisStorage` (`NewRedisStorage`) for use with third-party Fiber middleware

🧰 Tools

🪛 LanguageTool

[uncategorized] ~54-~54: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...tp/ratelimit: Redis-backed distributed rate limiting middleware for Fiber — New(conn, opts....

(EN_COMPOUND_ADJECTIVE_INTERNAL)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@README.md` at line 54, Update the README line describing the package (the
paragraph that starts with "commons/net/http/ratelimit") to hyphenate the
compound adjective: change "Redis-backed distributed rate limiting middleware"
to "Redis-backed distributed rate-limiting middleware"; keep the rest of the
sentence intact (the list of symbols like New, WithDefaultRateLimit,
WithRateLimit, WithDynamicRateLimit, MethodTierSelector,
DefaultTier/AggressiveTier/RelaxedTier,
IdentityFromIP/IdentityFromHeader/IdentityFromIPAndHeader, WithOnLimited,
RedisStorage/NewRedisStorage should remain unchanged).

[coderabbitai]

🛠️ Refactor suggestion | 🟠 Major

Make this report actionable instead of intentionally duplicative.

Keeping duplicate findings “intentionally preserved” creates triage noise and slows remediation. Please deduplicate by canonical key (file + symbol + root-cause) and keep one entry with occurrence count.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@REVIEW.md` at line 3, The report currently preserves intentionally duplicated
findings; update the generation step that produces the "Generated from 54
reviewer-agent runs..." section to deduplicate entries by a canonical key
composed of file path + symbol (e.g., function/class/line identifier) +
root-cause, collapsing duplicates into a single entry and adding an
occurrence_count field for how many times it appeared; locate the report
assembly logic that emits the findings list (the code that produces the
"Generated from ..." block in REVIEW.md) and change it to build a map keyed by
(file, symbol, root-cause), emit one canonical entry per key with
occurrence_count, and omit the original duplicate entries.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Use repository-relative file paths for all findings.

Most entries reference references/lib-commons/..., which is hard to navigate in this repo and breaks direct linking from code-review tooling. Normalize paths to actual repo-relative locations (for example, commons/...) so each item is immediately traceable and fixable.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@REVIEW.md` around lines 8 - 388, The review file uses non-repo-relative paths
like references/lib-commons/... which are hard to navigate; update all findings
in REVIEW.md to use repository-relative paths (e.g. commons/..., net/http/...,
opentelemetry/..., tenant-manager/...) so code-review tooling links resolve
directly. Locate and replace the repeated prefix "references/lib-commons/"
across the document (search for that exact token and the listed occurrences such
as commons/opentelemetry/otel.go, commons/net/http/withTelemetry.go,
commons/tenant-manager/middleware/tenant.go, etc.), normalize any path
formatting/commas/line-wrapped lists to valid repo-relative file paths, and
add/adjust any failing links or anchors; optionally add a small validation (CI
lint) that REVIEW.md paths are repo-relative to prevent regressions.

Dismiss Review