Skip to content

Production Deploy#72

Merged
NickSavino merged 21 commits into
mainfrom
development
Mar 22, 2026
Merged

Production Deploy#72
NickSavino merged 21 commits into
mainfrom
development

Conversation

@NickSavino

Copy link
Copy Markdown
Contributor

No description provided.

NickSavino and others added 21 commits February 4, 2026 03:35
Syncing main to development
Feat/middleware-wiring-webhook-setup
* added user calibration endpoints and schema"
- Added user_calibration migration
- Added sqlc queries for calibration CRUD operations
- added /api/users/me/calibration endpoints: GET, POST, DELETE
- me endpoint now fetches user from clerk id using middleware, enhancing security
- removed GET methods for user (GET /{id}, GET /)

* Fixed line endings

* Fixed time formatting in handler

* Added NOT NULL constrain to updated_at, created_at in db table

switch go dev container port to correct mapping (8080)

fixed variable typo in middleware_auth.go

---------

Co-authored-by: NicolaSavino <nick.savino@arcurve.com>
Co-authored-by: NicolaSavino <nick.savino@arcurve.com>
* Implemented workout and calibration schema with included sqlc queries. Updated makefile and seeding files

* Regenerated SQLC to match new queries and models (workout type + workout session)

* Added workout session service

* Added Workout Handler Endpoint

* Addressed PR comments
- fixed syntax error in makefile
-  Fixed migration schema
- fixed seeding script
- added /service/helps/helpers.util file
- renamed Dtos to match go standards, (Id -> ID)

* Updated API endpoints

* updated dtos to match casing convention. updated workout_session logic

* Added functionality for workout sets and reps. (services, handlers, migrations, queries)

* Addressed PR comments

* Addressed PR comments

---------

Co-authored-by: NicolaSavino <nick.savino@arcurve.com>
* fix: added clerk webhook secret

* updated CRLF line endings to LF
* fix: added clerk webhook secret

* updated CRLF line endings to LF

* fix
…pment

# Conflicts:
#	scripts/ci/deploy_via_ssm.sh
* Prod fix (#67)

* Small changes to dev tooling (docker/make/compose

* Server: minor variable rename

* Auth middleware + clerk webhook handler

* Added WithTransaction to store

* Removed debug logging message exposing userid

* Removed docker-compose changes not relevant to PR

* Renamed users.go to users.handler.go in handlers

* Added documentation explaining dev bypass

* Feat/calibration-endpoints (#57)

* added user calibration endpoints and schema"
- Added user_calibration migration
- Added sqlc queries for calibration CRUD operations
- added /api/users/me/calibration endpoints: GET, POST, DELETE
- me endpoint now fetches user from clerk id using middleware, enhancing security
- removed GET methods for user (GET /{id}, GET /)

* Fixed line endings

* Fixed time formatting in handler

* Added NOT NULL constrain to updated_at, created_at in db table

switch go dev container port to correct mapping (8080)

fixed variable typo in middleware_auth.go

---------

Co-authored-by: NicolaSavino <nick.savino@arcurve.com>

* Fixed syntax error on migration (#58)

Co-authored-by: NicolaSavino <nick.savino@arcurve.com>

* Feat/workouts api (#59)

* Implemented workout and calibration schema with included sqlc queries. Updated makefile and seeding files

* Regenerated SQLC to match new queries and models (workout type + workout session)

* Added workout session service

* Added Workout Handler Endpoint

* Addressed PR comments
- fixed syntax error in makefile
-  Fixed migration schema
- fixed seeding script
- added /service/helps/helpers.util file
- renamed Dtos to match go standards, (Id -> ID)

* Updated API endpoints

* updated dtos to match casing convention. updated workout_session logic

* Added functionality for workout sets and reps. (services, handlers, migrations, queries)

* Addressed PR comments

* Addressed PR comments

---------

Co-authored-by: NicolaSavino <nick.savino@arcurve.com>

* fix: added clerk webhook secret (#62)

* Fix/add clerkwebhook secret (#64)

* fix: added clerk webhook secret

* updated CRLF line endings to LF

* Fix/add clerkwebhook secret (#66)

* fix: added clerk webhook secret

* updated CRLF line endings to LF

* fix

---------

Co-authored-by: NicolaSavino <nick.savino@arcurve.com>

* fix: wipe data on seed

---------

Co-authored-by: Nicola Savino <77707655+NickSavino@users.noreply.github.com>
Co-authored-by: NicolaSavino <nick.savino@arcurve.com>
- updated workout session history endpoint
- added reset_workflow script along with github action
- added ingestion functionality for webhook
@qodo-code-review

Copy link
Copy Markdown

Review Summary by Qodo

Add user deletion webhook and refactor workout session history query

✨ Enhancement 🐞 Bug fix

Grey Divider

Walkthroughs

Description
• Added user deletion webhook handler for Clerk integration
• Refactored workout session history query to include aggregated sets/reps counts
• Removed calibration angle fields from workout session updates
• Added database reset workflow and improved deployment scripts
• Fixed line endings and added database seeding truncation
Diagram
flowchart LR
  A["Clerk Webhook Events"] -->|user.created| B["HandleCreateUserFromClerk"]
  A -->|user.deleted| C["HandleDeleteUserFromClerk"]
  B --> D["Store User in DB"]
  C --> E["Delete User by ClerkID"]
  F["GetUserWorkoutSessionHistory"] -->|Enhanced Query| G["Aggregate Sets/Reps"]
  G --> H["WorkoutSessionHistoryItemDTO"]
  I["CI/CD Pipeline"] -->|New Workflow| J["Reset Database Script"]
Loading

Grey Divider

File Changes

1. internal/core/service/user.service.go ✨ Enhancement +57/-48

Added user deletion handler for Clerk webhook

internal/core/service/user.service.go


2. internal/core/service/workout/workout_session.service.go ✨ Enhancement +38/-2

Refactored history query with aggregated metrics

internal/core/service/workout/workout_session.service.go


3. internal/db/users.sql.go ✨ Enhancement +9/-0

Added DeleteUserByClerkID database query

internal/db/users.sql.go


View more (9)
4. internal/db/workout_session.sql.go ✨ Enhancement +33/-33

Updated history query with sets/reps aggregation

internal/db/workout_session.sql.go


5. internal/http/handlers/auth.handler.go ✨ Enhancement +7/-1

Added user.deleted webhook event handler

internal/http/handlers/auth.handler.go


6. scripts/ci/deploy_via_ssm.sh Formatting +37/-37

Fixed line endings for shell script

scripts/ci/deploy_via_ssm.sh


7. scripts/ci/reset_database.sh ✨ Enhancement +28/-0

New database reset deployment script

scripts/ci/reset_database.sh


8. .gitattributes ⚙️ Configuration changes +1/-0

Added line ending normalization for shell scripts

.gitattributes


9. .github/workflows/reset-database.yml ✨ Enhancement +37/-0

New manual database reset workflow

.github/workflows/reset-database.yml


10. db/queries/users.sql ✨ Enhancement +3/-0

Added DeleteUserByClerkID SQL query

db/queries/users.sql


11. db/queries/workout_session.sql ✨ Enhancement +16/-8

Enhanced history query with aggregated metrics

db/queries/workout_session.sql


12. db/seeds/seed.sql ✨ Enhancement +6/-0

Added table truncation before seeding

db/seeds/seed.sql


Grey Divider

Qodo Logo

@NickSavino
NickSavino merged commit f16e297 into main Mar 22, 2026
@qodo-code-review

qodo-code-review Bot commented Mar 22, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (3) 📘 Rule violations (0) 📎 Requirement gaps (0) 📐 Spec deviations (0)

Grey Divider


Action required

1. Unprotected prod DB reset 🐞 Bug ⛨ Security
Description
A new workflow_dispatch job can execute goose reset + goose up against the production database
URL, which will irreversibly destroy all prod data. The only guard is a string input and being on
main, with no environment protection/approval gate or stronger access controls.
Code

.github/workflows/reset-database.yml[R11-37]

+jobs:
+  reset-database:
+    if: ${{ inputs.confirm_reset == 'RESET' && github.ref == 'refs/heads/main'}}
+    runs-on: ubuntu-latest
+
+    permissions:
+      id-token: write
+      contents: read
+    
+    steps:
+      - name: Checkout repository
+        uses: actions/checkout@v4
+
+      - name: Configure AWS credentials with OIDC
+        uses: aws-actions/configure-aws-credentials@v4
+        with:
+          role-to-assume: ${{ secrets.AWS_DEPLOY_ROLE_ARN }}
+          aws-region: ${{ secrets.AWS_REGION }}
+      
+      - name: Run database reset
+        env:
+          AWS_REGION: ${{ secrets.AWS_REGION }}
+          ECR_REPO: ${{ secrets.ECR_REPO }}
+          EC2_INSTANCE_ID: ${{ secrets.EC2_INSTANCE_ID }}
+          GITHUB_SHA: ${{ github.sha }}
+        run: |
+          bash ./scripts/ci/reset_database.sh
Evidence
The workflow is manually triggerable and runs scripts/ci/reset_database.sh, which explicitly
fetches the production DB URL from SSM (/cgc-2026-prod/...) and runs goose ... reset, which is a
destructive operation.

.github/workflows/reset-database.yml[1-37]
scripts/ci/reset_database.sh[12-24]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The new GitHub Action can hard-reset the production database with minimal safeguards (typed string + main branch). This is effectively a production “big red button” that can be triggered via `workflow_dispatch`, causing irreversible data loss.

### Issue Context
- Workflow runs on `workflow_dispatch` and calls `scripts/ci/reset_database.sh`.
- `reset_database.sh` retrieves `/cgc-2026-prod/api/database_url` and runs `goose reset`.

### Fix approach
- Add GitHub **environment protection** (e.g., `environment: production`) with required reviewers/approvals.
- Add **concurrency** to prevent parallel destructive runs.
- Add stricter checks (e.g., allowlist actors, require a second explicit confirmation input like the full DB name/account, and/or require a manual approval step).
- Consider making the script require an explicit `TARGET_ENV=prod` (and hard-fail otherwise) so accidental runs are harder.

### Fix Focus Areas
- .github/workflows/reset-database.yml[1-37]
- scripts/ci/reset_database.sh[1-28]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

2. Webhook delete can no-op 🐞 Bug ⛯ Reliability
Description
HandleDeleteUserFromClerk deletes by clerk_id without validating that an ID was actually
provided, and the DB delete helper doesn’t verify that any row was deleted. This can return success
(and log "Deleted user") even when the user was not removed, leaving stale users/data.
Code

internal/core/service/user.service.go[R50-57]

+func HandleDeleteUserFromClerk(ctx context.Context, store *data.Store, data json.RawMessage) error {
+	var userData types.ClerkUserData
+	if err := json.Unmarshal(data, &userData); err != nil {
+		return fmt.Errorf("failed to parse user data: %w", err)
+	}
+
+	return store.Queries.DeleteUserByClerkID(ctx, userData.ID)
+}
Evidence
The webhook handler calls HandleDeleteUserFromClerk, which unmarshals JSON into ClerkUserData
(missing fields remain zero values) and passes userData.ID directly to DeleteUserByClerkID.
DeleteUserByClerkID uses Exec and returns only err, so a 0-row delete is treated as success,
and the handler logs success.

internal/core/service/user.service.go[50-57]
internal/db/users.sql.go[59-66]
internal/http/handlers/auth.handler.go[88-94]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The Clerk `user.deleted` path can silently succeed even if no DB row was deleted because the code does not validate a non-empty Clerk user ID and does not check rows affected from the DELETE.

### Issue Context
- `HandleDeleteUserFromClerk` unmarshals into a struct where missing JSON fields become empty strings.
- `DeleteUserByClerkID` ignores `RowsAffected`, so `DELETE ... WHERE clerk_id = &#x27;&#x27;` or a non-existent id returns `nil` error.

### Fix approach
- In `HandleDeleteUserFromClerk`, validate `userData.ID != &quot;&quot;` and return an error if empty.
- In `DeleteUserByClerkID`, capture the command tag and return a not-found error when `RowsAffected() == 0`.
- Optionally adjust the handler to treat not-found as 200 (idempotent) but log it explicitly; or treat as 404/500 depending on desired semantics.

### Fix Focus Areas
- internal/core/service/user.service.go[50-57]
- internal/db/users.sql.go[59-66]
- internal/http/handlers/auth.handler.go[88-94]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Advisory comments

3. Misleading seed run comment 🐞 Bug ⚙ Maintainability
Description
db/seeds/seed.sql instructs running db/seeds/seed.dev.sql, but the repo’s db-seed Makefile
target actually executes db/seeds/seed.sql. This mismatch can cause developers to run the
wrong/non-existent seed script and increases the chance of operational mistakes.
Code

db/seeds/seed.sql[R2-9]

 * Seed data for development environment
 * Run with: psql -d your_database_name -f db/seeds/seed.dev.sql
 */
+TRUNCATE TABLE 
+    workout_session,
+    workout_type,
+    users
+RESTART IDENTITY CASCADE;
Evidence
The seed file header comment points to seed.dev.sql, while the documented/automated seeding
entrypoint in the Makefile runs db/seeds/seed.sql.

db/seeds/seed.sql[1-4]
Makefile[88-92]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
Seed instructions in `db/seeds/seed.sql` reference a different filename than the actual seeding command used by the repository (`make db-seed`).

### Issue Context
- The Makefile `db-seed` target runs `psql ... -f db/seeds/seed.sql`.
- The seed file comment says to run `db/seeds/seed.dev.sql`.

### Fix approach
- Update the comment to reference `db/seeds/seed.sql`, or rename/add the intended `seed.dev.sql` and update the Makefile accordingly.

### Fix Focus Areas
- db/seeds/seed.sql[1-9]
- Makefile[88-92]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

ⓘ The new review experience is currently in Beta. Learn more

Grey Divider

Qodo Logo

Comment on lines +11 to +37
jobs:
reset-database:
if: ${{ inputs.confirm_reset == 'RESET' && github.ref == 'refs/heads/main'}}
runs-on: ubuntu-latest

permissions:
id-token: write
contents: read

steps:
- name: Checkout repository
uses: actions/checkout@v4

- name: Configure AWS credentials with OIDC
uses: aws-actions/configure-aws-credentials@v4
with:
role-to-assume: ${{ secrets.AWS_DEPLOY_ROLE_ARN }}
aws-region: ${{ secrets.AWS_REGION }}

- name: Run database reset
env:
AWS_REGION: ${{ secrets.AWS_REGION }}
ECR_REPO: ${{ secrets.ECR_REPO }}
EC2_INSTANCE_ID: ${{ secrets.EC2_INSTANCE_ID }}
GITHUB_SHA: ${{ github.sha }}
run: |
bash ./scripts/ci/reset_database.sh No newline at end of file

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

1. Unprotected prod db reset 🐞 Bug ⛨ Security

A new workflow_dispatch job can execute goose reset + goose up against the production database
URL, which will irreversibly destroy all prod data. The only guard is a string input and being on
main, with no environment protection/approval gate or stronger access controls.
Agent Prompt
### Issue description
The new GitHub Action can hard-reset the production database with minimal safeguards (typed string + main branch). This is effectively a production “big red button” that can be triggered via `workflow_dispatch`, causing irreversible data loss.

### Issue Context
- Workflow runs on `workflow_dispatch` and calls `scripts/ci/reset_database.sh`.
- `reset_database.sh` retrieves `/cgc-2026-prod/api/database_url` and runs `goose reset`.

### Fix approach
- Add GitHub **environment protection** (e.g., `environment: production`) with required reviewers/approvals.
- Add **concurrency** to prevent parallel destructive runs.
- Add stricter checks (e.g., allowlist actors, require a second explicit confirmation input like the full DB name/account, and/or require a manual approval step).
- Consider making the script require an explicit `TARGET_ENV=prod` (and hard-fail otherwise) so accidental runs are harder.

### Fix Focus Areas
- .github/workflows/reset-database.yml[1-37]
- scripts/ci/reset_database.sh[1-28]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants