[init] add modules#1
Conversation
📝 WalkthroughWalkthroughRepository module renamed to github.com/bit-issues/backend; Telegram/bot subsystem removed; database support added (Goose migrations, Bun ORM, MySQL driver) with Fx wiring; configuration switched from Telegram to Database settings; a CI benchmark job disabled; a repo-root Changes
Sequence Diagram(s)sequenceDiagram
participant App as App (main)
participant Fx as Fx container
participant DBModule as db.Module
participant Goose as Goose (goosefx)
participant Bun as Bun ORM / sqlfx
rect rgba(0,128,0,0.5)
App->>Fx: Start application (internal.Run)
Fx->>DBModule: Construct db.Module()
end
rect rgba(0,0,255,0.5)
DBModule->>Goose: Provide migration storage (migrations.FS) & dialect
DBModule->>Bun: Provide mysqldialect and driver registration
end
rect rgba(128,0,128,0.5)
Fx->>Goose: Run migrations on startup (goosefx)
Fx->>Bun: Initialize DB connection pool (sqlfx/bunfx)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
main.go (1)
8-8: Consider updating the contact URL in Swagger annotations.The contact URL still references
https://github.com/capcom6while the module has been renamed togithub.com/bit-issues/backend. Update this if the project contact has changed.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@main.go` at line 8, Update the Swagger contact URL annotation (the commented line starting with "@contact.url") to point to the current project/contact location instead of "https://github.com/capcom6"; replace that value with the new URL "https://github.com/bit-issues/backend" (or the correct contact URL) in the annotation so the Swagger docs reflect the renamed module.internal/db/models.go (1)
5-8: Consider adding automatic timestamp management.The
TimedModelrequires manual population ofCreatedAtandUpdatedAtbefore insert/update. You could leverage Bun's hooks or model interface to auto-populate these fields.Example with Bun hooks
package db import ( "context" "time" "github.com/uptrace/bun" ) type TimedModel struct { CreatedAt time.Time `bun:",nullzero,notnull" json:"created_at"` UpdatedAt time.Time `bun:",nullzero,notnull" json:"updated_at"` } var _ bun.BeforeAppendModelHook = (*TimedModel)(nil) func (m *TimedModel) BeforeAppendModel(ctx context.Context, query bun.Query) error { now := time.Now().UTC() switch query.(type) { case *bun.InsertQuery: if m.CreatedAt.IsZero() { m.CreatedAt = now } m.UpdatedAt = now case *bun.UpdateQuery: m.UpdatedAt = now } return nil }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/db/models.go` around lines 5 - 8, TimedModel currently requires manual setting of CreatedAt/UpdatedAt; implement Bun's hook by making TimedModel satisfy bun.BeforeAppendModelHook (e.g., add var _ bun.BeforeAppendModelHook = (*TimedModel)(nil)) and implement BeforeAppendModel(ctx context.Context, query bun.Query) to set now := time.Now().UTC(), set CreatedAt if zero and always update UpdatedAt on InsertQuery, and set UpdatedAt on UpdateQuery so timestamps are auto-managed for Insert/Update operations.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@dev-docs`:
- Line 1: The symlink named "dev-docs" points to "../dev-docs" but the target is
missing; either remove this broken symlink or recreate/relocate the target
directory at "../dev-docs" so the link resolves correctly—update the symlink
target or delete the link depending on whether the documentation directory
should exist, and confirm via ls -l that "dev-docs" is no longer broken.
In `@internal/db/errors.go`:
- Around line 5-11: IsUniqueViolation currently looks for SQLite text ("UNIQUE
constraint failed"); update it to detect MySQL duplicate-key errors by checking
for a mysql.MySQLError with Number == 1062 (using
github.com/go-sql-driver/mysql) and fall back to matching common text like
"Duplicate entry" for safety; ensure nil checks remain and avoid importing mysql
unconditionally if build constraints require it (i.e., update
internal/db/errors.go to type-assert err to *mysql.MySQLError in
IsUniqueViolation and return true for Number == 1062, otherwise check
strings.Contains(err.Error(), "Duplicate entry")).
In `@internal/db/migrations/20260204013005_initial.sql`:
- Around line 1-9: The migration file for version 20260204013005 currently only
runs SELECTs and will be marked applied by Goose, preventing future DDL from
taking effect; either replace the placeholder statements between the goose
markers (-- +goose Up / StatementBegin ... StatementEnd) with the actual initial
schema DDL (tables, indexes, constraints) so the Up section creates the schema
and the Down section drops it, or delete this migration entirely and create new
proper migrations later; locate the sections labeled "-- +goose Up" and "--
+goose Down" in the migration for version 20260204013005 and update or remove
them accordingly.
- Line 5: Remove the stray malformed separator `---` on line 5 which is not
valid Goose or SQL syntax; open the migration `20260204013005_initial.sql`,
delete the `---` line so the file only contains proper SQL and Goose directives
(use `-- +goose` for Goose directives and `--` for comments) ensuring no other
non-SQL tokens remain.
---
Nitpick comments:
In `@internal/db/models.go`:
- Around line 5-8: TimedModel currently requires manual setting of
CreatedAt/UpdatedAt; implement Bun's hook by making TimedModel satisfy
bun.BeforeAppendModelHook (e.g., add var _ bun.BeforeAppendModelHook =
(*TimedModel)(nil)) and implement BeforeAppendModel(ctx context.Context, query
bun.Query) to set now := time.Now().UTC(), set CreatedAt if zero and always
update UpdatedAt on InsertQuery, and set UpdatedAt on UpdateQuery so timestamps
are auto-managed for Insert/Update operations.
In `@main.go`:
- Line 8: Update the Swagger contact URL annotation (the commented line starting
with "@contact.url") to point to the current project/contact location instead of
"https://github.com/capcom6"; replace that value with the new URL
"https://github.com/bit-issues/backend" (or the correct contact URL) in the
annotation so the Swagger docs reflect the renamed module.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6cd10a41-05de-4a8d-83e2-802327152258
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (15)
dev-docsgo.modinternal/app.gointernal/bot/handler/handler.gointernal/bot/handlers/start/handler.gointernal/bot/module.gointernal/config/config.gointernal/config/module.gointernal/db/errors.gointernal/db/migrations/20260204013005_initial.sqlinternal/db/migrations/embed.gointernal/db/models.gointernal/db/module.gointernal/server/module.gomain.go
💤 Files with no reviewable changes (3)
- internal/bot/handler/handler.go
- internal/bot/module.go
- internal/bot/handlers/start/handler.go
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/workflows/go.yml (1)
77-80: Consider documenting why the benchmark job is disabled or removing it.The
if: falsepattern leaves an entire job that will never run. Static analysis (actionlint) also flags this as a constant false condition.If this is temporary during the module initialization phase, consider adding a comment explaining when it should be re-enabled. If the job is no longer needed, removing it entirely would reduce workflow maintenance burden.
💡 Option 1: Add a comment explaining the disable
benchmark: name: Benchmark runs-on: ubuntu-latest - if: false + if: false # TODO: Re-enable once benchmark baseline is established for new modules permissions:💡 Option 2: Remove the job entirely if not needed
If benchmarks are not planned for this project, consider removing the entire
benchmarkjob (lines 77-132) to keep the workflow file clean.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/go.yml around lines 77 - 80, The benchmark job currently has a constant disable via "if: false" which actionlint flags; either remove the entire "benchmark" job block (job name: benchmark) if it's not needed, or keep it but replace the unconditional "if: false" with a short inline comment explaining why it's disabled and when to re-enable it (e.g., during module initialization or when benchmarks are added) so reviewers and linters understand the intent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.github/workflows/go.yml:
- Around line 77-80: The benchmark job currently has a constant disable via "if:
false" which actionlint flags; either remove the entire "benchmark" job block
(job name: benchmark) if it's not needed, or keep it but replace the
unconditional "if: false" with a short inline comment explaining why it's
disabled and when to re-enable it (e.g., during module initialization or when
benchmarks are added) so reviewers and linters understand the intent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 92c3ee63-5320-49b0-9483-660bbedaeb90
📒 Files selected for processing (2)
.github/workflows/go.ymlinternal/db/errors.go
🚧 Files skipped from review as they are similar to previous changes (1)
- internal/db/errors.go
🤖 Pull request artifacts
|
b2adcd7 to
9d28083
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/config/config.go (1)
45-59:⚠️ Potential issue | 🟠 MajorDo not ship hardcoded DB credentials in defaults.
Line 58 embeds username/password in source and Line 45 suppresses
gosecfor the whole function. This weakens security posture and makes accidental insecure deploys more likely. Prefer an empty default URL and fail fast when unset.Suggested change
func Default() Config { - //nolint:gosec // default values return Config{ @@ Database: databaseConfig{ - URL: "mariadb://bit-issues:bit-issues@127.0.0.1:3306/bit-issues?charset=utf8mb4&parseTime=True&loc=UTC", + URL: "", ConnMaxIdleTime: 0, ConnMaxLifetime: 0, MaxOpenConns: 0, MaxIdleConns: 0, }, @@ func New() (Config, error) { cfg := Default() @@ if err := config.Load(&cfg, options...); err != nil { return Config{}, fmt.Errorf("failed to load config: %w", err) } + if cfg.Database.URL == "" { + return Config{}, fmt.Errorf("database.url is required") + } return cfg, nil }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/config/config.go` around lines 45 - 59, The default config currently embeds hardcoded DB credentials in the returned Config (databaseConfig.URL) and suppresses gosec; remove the hardcoded connection string by setting databaseConfig.URL to an empty string, drop or narrow the //nolint:gosec annotation, and add a fail-fast validation (e.g., in the config constructor/Validate function that returns Config) that errors if databaseConfig.URL is empty so the app refuses to start without an explicit, secure DB URL; refer to the Config and databaseConfig types and the URL field when making these changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/go.yml:
- Line 80: The benchmark job is permanently disabled by the literal "if: false";
remove this dead conditional and either delete the job if you intend to remove
benchmarking permanently, or replace "if: false" with a controllable gate such
as a repository variable (e.g., use an env like RUN_BENCH and set "if:
env.RUN_BENCH == 'true'") or a workflow_dispatch input (check
github.event.inputs.run_bench) so the benchmark job can be enabled
intentionally; locate the benchmark job entry that contains "if: false" and
apply one of these replacements.
In `@go.mod`:
- Line 6: These dependencies (bunfx, goosefx, sqlfx, validatorfx) are pre-1.0
and risky for production; either replace them with stable alternatives or pin
them to exact, reviewed commits and record the rationale: update go.mod to use
specific pseudo-versions or commit SHAs (use `go get module@<commit>` to pin) or
add a temporary `replace` directive for each module to a vetted commit, and add
a short note in the repo README or a TODO in the codebase documenting why the
pre-1.0 pins are used and when to revisit; reference the module names bunfx,
goosefx, sqlfx, and validatorfx when making these edits.
---
Outside diff comments:
In `@internal/config/config.go`:
- Around line 45-59: The default config currently embeds hardcoded DB
credentials in the returned Config (databaseConfig.URL) and suppresses gosec;
remove the hardcoded connection string by setting databaseConfig.URL to an empty
string, drop or narrow the //nolint:gosec annotation, and add a fail-fast
validation (e.g., in the config constructor/Validate function that returns
Config) that errors if databaseConfig.URL is empty so the app refuses to start
without an explicit, secure DB URL; refer to the Config and databaseConfig types
and the URL field when making these changes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b394dfb6-93b6-4781-8247-fe485942f698
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (16)
.github/workflows/go.ymldev-docsgo.modinternal/app.gointernal/bot/handler/handler.gointernal/bot/handlers/start/handler.gointernal/bot/module.gointernal/config/config.gointernal/config/module.gointernal/db/errors.gointernal/db/migrations/20260204013005_initial.sqlinternal/db/migrations/embed.gointernal/db/models.gointernal/db/module.gointernal/server/module.gomain.go
💤 Files with no reviewable changes (3)
- internal/bot/module.go
- internal/bot/handlers/start/handler.go
- internal/bot/handler/handler.go
✅ Files skipped from review due to trivial changes (7)
- dev-docs
- internal/db/migrations/20260204013005_initial.sql
- main.go
- internal/db/models.go
- internal/db/migrations/embed.go
- internal/db/errors.go
- internal/server/module.go
🚧 Files skipped from review as they are similar to previous changes (3)
- internal/db/module.go
- internal/config/module.go
- internal/app.go
Summary by CodeRabbit