Open
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughBumped a client-go dependency; removed a custom Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
go.mod (1)
7-7: Pseudo-version dependency — consider pinning to a tagged release before merging.
v1.10.1-0.20260217004820-83d276545735is a pseudo-version pointing to an unreleased commit. This is fine during development, but ideally this should be updated to a proper tagged release (e.g.,v1.10.1) before shipping, to ensure reproducibility and clearer dependency tracking.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@go.mod` at line 7, The go.mod entry for module github.com/android-sms-gateway/client-go currently uses a pseudo-version (v1.10.1-0.20260217004820-83d276545735); replace that pseudo-version with a proper tagged release (for example v1.10.1 or the latest stable tag) in go.mod and then refresh the module metadata (update go.sum / tidy modules) so the project references a reproducible, tagged dependency.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@go.mod`:
- Line 7: The go.mod entry for module github.com/android-sms-gateway/client-go
currently uses a pseudo-version (v1.10.1-0.20260217004820-83d276545735); replace
that pseudo-version with a proper tagged release (for example v1.10.1 or the
latest stable tag) in go.mod and then refresh the module metadata (update go.sum
/ tidy modules) so the project references a reproducible, tagged dependency.
🤖 Pull request artifacts
|
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
.golangci.yml (1)
400-402:allow-unused: truedisables detection of stalenolintdirectives.Setting this to
truemeans dead//nolint:...annotations will silently accumulate as code changes. Since the PR also adds several newnolintdirectives (e.g., ininternal/sms-gateway/modules/metrics/module.go,internal/version/version.go), consider keeping thisfalselong-term and only using it during the version-bump stabilization window, to preserve the hygiene guarantee.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.golangci.yml around lines 400 - 402, The config currently sets allow-unused: true which disables detection of stale //nolint directives; revert this to allow-unused: false (or remove the change) so the linter will warn about dead nolint annotations, and if you need to relax it temporarily, document the version-bump window and flip it back afterwards—also review the newly added nolint instances in internal/sms-gateway/modules/metrics/module.go and internal/version/version.go and remove or justify any unnecessary //nolint entries before re-enabling the stricter check.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.golangci.yml:
- Around line 346-351: The global suppression of gosec rule G117 (and G704) in
the gosec block removes an important security guardrail; instead of excluding
G117 project-wide, revert the global excludes and apply targeted suppressions
only where intentional JSON marshaling of secrets occurs (use inline nolint
comments or file-specific golangci.yml overrides) so that functions/types
handling secrets are explicitly annotated; update the gosec configuration to
remove G117 from the top-level excludes and add guidance in code comments where
you keep G704/G117 suppressed locally for known false positives, referencing the
gosec rule IDs G117 and G704 and the gosec section in .golangci.yml to locate
the change.
- Around line 256-259: The .golangci.yml contains duplicate exhaustruct
exclusion patterns—remove the redundant entries for
"^github.com/gofiber/.+Config$", "^gopkg.in/telebot.v4.LongPoller$",
"^gopkg.in/telebot.v4.ReplyMarkup$", and "^gopkg.in/telebot.v4.Settings$" so
each pattern appears only once; locate the duplicate block that repeats these
patterns and delete the repeated lines, leaving the original exclusion entries
intact to avoid noise.
---
Nitpick comments:
In @.golangci.yml:
- Around line 400-402: The config currently sets allow-unused: true which
disables detection of stale //nolint directives; revert this to allow-unused:
false (or remove the change) so the linter will warn about dead nolint
annotations, and if you need to relax it temporarily, document the version-bump
window and flip it back afterwards—also review the newly added nolint instances
in internal/sms-gateway/modules/metrics/module.go and
internal/version/version.go and remove or justify any unnecessary //nolint
entries before re-enabling the stricter check.
There was a problem hiding this comment.
🧹 Nitpick comments (2)
.golangci.yml (2)
396-398:allow-unused: truesilently permits stale//nolintdirectives and contradicts the inline comment.The comment directly above this setting reads "Disable to ensure that all nolint directives actually have an effect", which describes the behaviour of
false. Flipping totruedisables that check, allowing dead suppressions to accumulate undetected.If the intent is to accommodate the newly added
//nolint:revivedirectives in other files, the safer approach is to keepallow-unused: falseand only place those directives on lines wherereviveactually fires — rather than globally relaxing the enforcement.♻️ Proposed fix
- # Disable to ensure that all nolint directives actually have an effect. - # Default: false - allow-unused: true + # Disable to ensure that all nolint directives actually have an effect. + # Default: false + allow-unused: false🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.golangci.yml around lines 396 - 398, The config currently sets allow-unused: true which silently permits stale //nolint directives; change allow-unused back to false in the .golangci.yml (the allow-unused setting) and remove the global relaxation, then instead add targeted //nolint:revive comments only on the specific lines/functions where revive actually reports issues (use the revive linter output to find these locations) so unused nolint directives are not allowed to accumulate.
262-262: Consider narrowing the two broad exhaustruct exclusion patterns for consistency.
- Line 262:
^github.com/go-telegram/bot/models.+$matches every type in themodelssubpackage. The sibling pattern on line 261 (^github.com/go-telegram/bot.+Params$) uses a suffix anchor; a narrowed pattern (e.g.…models.+(Config|Params|Options)$) would align better with the library's builder-style type naming.- Line 268:
^github.com/revrost/go-openrouter.+$excludes the entirego-openrouterpackage unconditionally, unlike most other patterns in this list that use specific suffix anchors (Config$,Params$,Claims$,Opts$, etc.). Consider narrowing to common suffixes (e.g.…(Request|Config|Options|Params)$).Both patterns are valid for external libraries where exhaustruct is intentionally permissive, but narrowing them would improve consistency with the rest of the configuration.
♻️ Proposed narrowing
- - ^github.com/go-telegram/bot/models.+$ + - ^github.com/go-telegram/bot/models.+(Config|Params|Options)$ ... - - ^github.com/revrost/go-openrouter.+$ + - ^github.com/revrost/go-openrouter.+(Request|Config|Options|Params)$🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.golangci.yml at line 262, The exclusion regexes in .golangci.yml are too broad: replace the pattern ^github.com/go-telegram/bot/models.+$ with a narrowed pattern that only targets builder-style types (for example limit to suffixes like (Config|Params|Options)$) and similarly change ^github.com/revrost/go-openrouter.+$ to only match common type suffixes (for example (Request|Config|Options|Params|Opts)$); update the two entries so they use these more specific suffix-anchored regexes to match only intended types (referencing the existing patterns ^github.com/go-telegram/bot/models.+$ and ^github.com/revrost/go-openrouter.+$ when making the replacements).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In @.golangci.yml:
- Around line 342-347: The global suppression of gosec rule G117 in the
gosec.excludes list removes an important guardrail; edit the gosec.excludes
block (the gosec config under the gosec key) to remove G117 from the global
excludes and instead apply targeted, inline suppressions for specific
files/structs that are false-positives; keep G704 in the global excludes only if
you have validated its false-positive rate, otherwise restrict it similarly by
directory or inline comments. Locate the gosec.excludes entry (the current list
containing G117 and G704) and remove G117 from that global list, then add
//nolint:gosec or equivalent file-level annotations next to the exact
structs/functions that are known false positives, or scope disables to specific
packages in the linter config.
---
Nitpick comments:
In @.golangci.yml:
- Around line 396-398: The config currently sets allow-unused: true which
silently permits stale //nolint directives; change allow-unused back to false in
the .golangci.yml (the allow-unused setting) and remove the global relaxation,
then instead add targeted //nolint:revive comments only on the specific
lines/functions where revive actually reports issues (use the revive linter
output to find these locations) so unused nolint directives are not allowed to
accumulate.
- Line 262: The exclusion regexes in .golangci.yml are too broad: replace the
pattern ^github.com/go-telegram/bot/models.+$ with a narrowed pattern that only
targets builder-style types (for example limit to suffixes like
(Config|Params|Options)$) and similarly change
^github.com/revrost/go-openrouter.+$ to only match common type suffixes (for
example (Request|Config|Options|Params|Opts)$); update the two entries so they
use these more specific suffix-anchored regexes to match only intended types
(referencing the existing patterns ^github.com/go-telegram/bot/models.+$ and
^github.com/revrost/go-openrouter.+$ when making the replacements).
capcom6
force-pushed
the
settings/add-gateway-settings-support
branch
from
cf0d0cd to
1d4ef31
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
.golangci.yml (2)
396-398:allow-unused: truesilences deadnolintdirectives project-wide.With
require-explanation: trueandrequire-specific: truestill enforced, the quality bar for new directives is maintained, but stale directives on removed code won't be caught. This is likely intentional to accommodate the new file-level//nolint:reviveannotations that may not fire in all lint configurations, but it's worth acknowledging the trade-off.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.golangci.yml around lines 396 - 398, The setting allow-unused: true in the golangci-lint config mutes detection of dead //nolint directives project-wide; to avoid stale nolint comments slipping through, change allow-unused to false (or document and scope the exception) so golangci-lint will flag unused nolint directives; ensure the repo still preserves required behavior by keeping require-explanation: true and require-specific: true, and if you need to exempt file-level //nolint:revive usages, do so explicitly (e.g., per-file policy) rather than leaving allow-unused enabled globally.
260-262: Remove unusedexhaustructpackage exclusions from.golangci.yml.The following packages have exclusion rules in
.golangci.yml(lines 260–262, 266, 268, 277) but are not present ingo.mod:
github.com/go-telegram-bot-api/telegram-bot-apigithub.com/go-telegram/botgithub.com/mymmrac/telegogithub.com/revrost/go-openroutergithub.com/ThreeDotsLabs/watermillThese appear to be inherited from a template config. Since the project never imports these libraries, their exclusions add unnecessary noise and should be removed.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.golangci.yml around lines 260 - 262, Remove the unused exhaustruct exclusion entries from .golangci.yml: delete the regex rules referencing ^github.com/go-telegram-bot-api/telegram-bot-api/.+Config$, ^github.com/go-telegram/bot.+Params$, ^github.com/go-telegram/bot/models.+$ and the other exclusions for github.com/mymmrac/telego, github.com/revrost/go-openrouter and github.com/ThreeDotsLabs/watermill; confirm these packages are not present in go.mod and then run golangci-lint to ensure no remaining false-positive exhaustruct exclusions remain.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@go.mod`:
- Line 7: The go.mod currently pins github.com/android-sms-gateway/client-go to
a pseudo-version (v1.10.1-0.20260218044357-fb0b167fbe22); replace this with a
real released tag or coordinate a release: either update go.mod to the proper
tagged release (e.g., github.com/android-sms-gateway/client-go v1.10.1) after
that tag is published, or revert to the last stable published version (e.g.,
v1.10.0) until v1.10.1 is published; ensure the dependency line containing
"github.com/android-sms-gateway/client-go v1.10.1-0.20260218044357-fb0b167fbe22"
is updated and run go mod tidy to refresh the module graph.
In `@internal/config/config.go`:
- Around line 89-91: The JWT TTL field currently uses time.Duration which the
YAML unmarshaler can't parse from human-readable strings like "24h"; restore the
previous custom Duration type (e.g., type Duration struct { time.Duration }) and
implement UnmarshalYAML on it to accept string values (using time.ParseDuration)
and numeric node kinds (treat as nanoseconds), then change the JWT config struct
field TTL from time.Duration to this Duration type (references: the JWT struct
fields Secret, TTL, Issuer in internal/config/config.go and the Duration type
and its UnmarshalYAML method); ensure any code that reads TTL converts Duration
back to time.Duration where needed.
---
Duplicate comments:
In @.golangci.yml:
- Around line 342-347: The global gosec exclusion of G117 in .golangci.yml is
too broad; remove G117 from the top-level gosec.excludes and instead add a
scoped suppression using golangci-lint's "exclude-rules" (or gosec's
file/path-specific config) targeting only the offending files/types once you've
identified them, leaving G704 unchanged if it truly needs global exclusion;
update gosec.excludes to only include genuinely global rules (e.g., G704) and
create one or more exclude-rules entries that reference the specific file path
regexes and rule: G117 to narrow the scope.
---
Nitpick comments:
In @.golangci.yml:
- Around line 396-398: The setting allow-unused: true in the golangci-lint
config mutes detection of dead //nolint directives project-wide; to avoid stale
nolint comments slipping through, change allow-unused to false (or document and
scope the exception) so golangci-lint will flag unused nolint directives; ensure
the repo still preserves required behavior by keeping require-explanation: true
and require-specific: true, and if you need to exempt file-level //nolint:revive
usages, do so explicitly (e.g., per-file policy) rather than leaving
allow-unused enabled globally.
- Around line 260-262: Remove the unused exhaustruct exclusion entries from
.golangci.yml: delete the regex rules referencing
^github.com/go-telegram-bot-api/telegram-bot-api/.+Config$,
^github.com/go-telegram/bot.+Params$, ^github.com/go-telegram/bot/models.+$ and
the other exclusions for github.com/mymmrac/telego,
github.com/revrost/go-openrouter and github.com/ThreeDotsLabs/watermill; confirm
these packages are not present in go.mod and then run golangci-lint to ensure no
remaining false-positive exhaustruct exclusions remain.
capcom6
force-pushed
the
settings/add-gateway-settings-support
branch
from
1d4ef31 to
0915454
Compare
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary by CodeRabbit
New Features
Behavioral Changes
Chores