Skip to content

fix(CI): major cleanup#3750

Merged
imeyer merged 4 commits intomainfrom
push-kzutyuvtvvkl
Aug 7, 2025
Merged

fix(CI): major cleanup#3750
imeyer merged 4 commits intomainfrom
push-kzutyuvtvvkl

Conversation

@imeyer
Copy link
Contributor

@imeyer imeyer commented Aug 7, 2025

What does this PR do?

Fixes # (issue)

If there is not an issue for this, please create one first. This is used to tracking purposes and also helps use understand why this PR exists

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • Chore (refactoring code, technical debt, workflow improvements)
  • Enhancement (small improvements)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How should this be tested?

  • Test A
  • Test B

Checklist

Required

  • Filled out the "How to test" section in this PR
  • Read Contributing Guide
  • Self-reviewed my own code
  • Commented on my code in hard-to-understand areas
  • Ran pnpm build
  • Ran pnpm fmt
  • Checked for warnings, there are none
  • Removed all console.logs
  • Merged the latest changes from main onto my branch with git pull origin main
  • My changes don't cause any responsiveness issues

Appreciated

  • If a UI change was made: Added a screen recording or screenshots to this PR
  • Updated the Unkey Docs if changes were necessary

Summary by CodeRabbit

  • Chores

    • Removed several integration and unit tests related to rate limiting, cluster membership, and key management.
    • Updated workflow configurations to eliminate jobs and filters related to agent testing.
    • Deleted specific workflow files and job references for agent local testing.
    • Updated deployment configurations with improved MySQL health checks and service dependencies.
  • Refactor

    • Enhanced Makefiles to automatically detect CPU cores and adjust parallelism for tests.

@changeset-bot
Copy link

changeset-bot bot commented Aug 7, 2025

⚠️ No Changeset found

Latest commit: 4d089cc

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@vercel
Copy link

vercel bot commented Aug 7, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

2 Skipped Deployments
Name Status Preview Comments Updated (UTC)
dashboard ⬜️ Ignored (Inspect) Visit Preview Aug 7, 2025 5:06pm
engineering ⬜️ Ignored (Inspect) Visit Preview Aug 7, 2025 5:06pm

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Aug 7, 2025

📝 Walkthrough

Walkthrough

This change removes all integration and cluster-level tests for the agent component, deletes the agent-specific test workflows from GitHub Actions, and updates Makefiles to dynamically set parallelism based on CPU cores. It eliminates the "agent" change detection filter and all references to the test_agent_local job across workflow files.

Changes

Cohort / File(s) Change Summary
GitHub Actions Workflows Cleanup
.github/workflows/deploy.yaml, .github/workflows/pr.yaml, .github/workflows/job_detect_changes.yaml, .github/workflows/job_test_agent_local.yaml, .github/workflows/test_agent_local.yaml
Removed the test_agent_local job and its workflow file; deleted all references to the job in deployment and PR workflows; eliminated the "agent" change detection filter and output; deleted both the original and referenced test agent workflow files.
Agent Integration & Cluster Tests Removal
apps/agent/integration/cluster/docker/ratelimits_test.go, apps/agent/integration/identities/identities_ratelimits_accuracy_test.go, apps/agent/integration/identities/ratelimits_with_cost_load_test.go, apps/agent/integration/identities/token_ratelimits_test.go, apps/agent/integration/identities/update_identity_with_many_keys_test.go, apps/agent/integration/keys/ratelimits_test.go, apps/agent/integration/keys/update_ratelimits_test.go
Deleted all integration tests related to rate limiting, identity management, key updates, and rate limit accuracy for the agent.
Agent Cluster, Membership, and Ring Unit Tests Removal
apps/agent/pkg/cluster/cluster_test.go, apps/agent/pkg/membership/membership_test.go, apps/agent/pkg/ring/ring_test.go
Deleted all unit and integration tests for cluster membership, consistent hashing ring, and cluster propagation/lookup logic.
Agent Ratelimit Service Tests Removal
apps/agent/services/ratelimit/ratelimit_mitigation_test.go, apps/agent/services/ratelimit/ratelimit_replication_test.go, apps/agent/services/ratelimit/ratelimit_test.go
Deleted all tests for the ratelimit service, including mitigation, replication, and accuracy tests for clustered environments.
Makefile Parallelism Update
apps/agent/Makefile, go/Makefile
Enhanced Makefiles to detect CPU cores and set GOMAXPROCS and parallel test execution accordingly; suppressed command output for some targets; added --force-recreate to docker compose run in the Go Makefile.
Deployment SQL and Docker Compose Updates
deployment/04-seed-workspace.sql, deployment/docker-compose.yaml
Fixed trailing spaces and corrected table name in SQL inserts; updated MySQL service in Docker Compose to add --skip-log-bin option and a healthcheck; changed service dependency to wait for MySQL health before starting.

Sequence Diagram(s)

No sequence diagram generated, as the changes are limited to test removals, workflow deletions, and Makefile updates.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~35 minutes

Possibly related PRs

  • fix: optimize workflow tests #3743: Also modifies and deletes the .github/workflows/job_test_agent_local.yaml workflow, directly overlapping with this PR's workflow removals.
  • chore: CI improvements part 1. #3176: Introduces the "agent" change detection output and conditional test job, which this PR now removes, indicating a reversal or cleanup of those changes.
  • fix: Make CI actually CI  #3468: Modifies the conditions and triggers for the test_agent_local job in .github/workflows/pr.yaml, which this PR also removes.

Suggested reviewers

  • mcstepp
  • MichaelUnkey
  • ogzhanolguncu

Note

⚡️ Unit Test Generation is now available in beta!

Learn more here, or try it out under "Finishing Touches" below.

✨ Finishing Touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch push-kzutyuvtvvkl

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai generate unit tests to generate unit tests for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@github-actions
Copy link
Contributor

github-actions bot commented Aug 7, 2025

Thank you for following the naming conventions for pull request titles! 🙏

@vercel vercel bot temporarily deployed to Preview – engineering August 7, 2025 14:51 Inactive
@vercel vercel bot temporarily deployed to Preview – dashboard August 7, 2025 14:51 Inactive
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 4

♻️ Duplicate comments (1)
go/Makefile (1)

3-15: Same Bash-specific arithmetic issue as in agent Makefile

$(shell echo $$(($(GOMAXPROCS_VAL) > 1 ? $(GOMAXPROCS_VAL)/2 : 1))) depends on Bash’s ternary operator. Replace with a POSIX-portable construct to avoid failures on dash-based runners (e.g., Debian, Ubuntu).

See earlier diff suggestion in apps/agent/Makefile.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6312937 and cc39f15.

📒 Files selected for processing (20)
  • .github/workflows/deploy.yaml (1 hunks)
  • .github/workflows/job_detect_changes.yaml (0 hunks)
  • .github/workflows/job_test_agent_local.yaml (0 hunks)
  • .github/workflows/pr.yaml (0 hunks)
  • .github/workflows/test_agent_local.yaml (0 hunks)
  • apps/agent/Makefile (2 hunks)
  • apps/agent/integration/cluster/docker/ratelimits_test.go (0 hunks)
  • apps/agent/integration/identities/identities_ratelimits_accuracy_test.go (0 hunks)
  • apps/agent/integration/identities/ratelimits_with_cost_load_test.go (0 hunks)
  • apps/agent/integration/identities/token_ratelimits_test.go (0 hunks)
  • apps/agent/integration/identities/update_identity_with_many_keys_test.go (0 hunks)
  • apps/agent/integration/keys/ratelimits_test.go (0 hunks)
  • apps/agent/integration/keys/update_ratelimits_test.go (0 hunks)
  • apps/agent/pkg/cluster/cluster_test.go (0 hunks)
  • apps/agent/pkg/membership/membership_test.go (0 hunks)
  • apps/agent/pkg/ring/ring_test.go (0 hunks)
  • apps/agent/services/ratelimit/ratelimit_mitigation_test.go (0 hunks)
  • apps/agent/services/ratelimit/ratelimit_replication_test.go (0 hunks)
  • apps/agent/services/ratelimit/ratelimit_test.go (0 hunks)
  • go/Makefile (3 hunks)
💤 Files with no reviewable changes (17)
  • .github/workflows/job_test_agent_local.yaml
  • .github/workflows/pr.yaml
  • apps/agent/services/ratelimit/ratelimit_mitigation_test.go
  • apps/agent/integration/keys/update_ratelimits_test.go
  • apps/agent/pkg/ring/ring_test.go
  • apps/agent/integration/keys/ratelimits_test.go
  • .github/workflows/job_detect_changes.yaml
  • apps/agent/pkg/membership/membership_test.go
  • apps/agent/integration/identities/update_identity_with_many_keys_test.go
  • apps/agent/integration/cluster/docker/ratelimits_test.go
  • apps/agent/services/ratelimit/ratelimit_test.go
  • apps/agent/integration/identities/token_ratelimits_test.go
  • apps/agent/pkg/cluster/cluster_test.go
  • .github/workflows/test_agent_local.yaml
  • apps/agent/integration/identities/identities_ratelimits_accuracy_test.go
  • apps/agent/services/ratelimit/ratelimit_replication_test.go
  • apps/agent/integration/identities/ratelimits_with_cost_load_test.go
🧰 Additional context used
🧠 Learnings (12)
📚 Learning: applies to go/deploy/deploy/{assetmanagerd,billaged,builderd,metald}/**/makefile : use `make build` ...
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-21T18:05:58.236Z
Learning: Applies to go/deploy/deploy/{assetmanagerd,billaged,builderd,metald}/**/Makefile : Use `make build` to test that the binary builds.

Applied to files:

  • apps/agent/Makefile
  • go/Makefile
📚 Learning: applies to go/deploy/**/makefile : global makefile should be `makefile` at the repository root....
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-21T18:05:58.236Z
Learning: Applies to go/deploy/**/Makefile : Global makefile should be `Makefile` at the repository root.

Applied to files:

  • apps/agent/Makefile
  • go/Makefile
📚 Learning: applies to go/deploy/deploy/*/makefile : service-level makefile should be `/makefile`....
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-21T18:05:58.236Z
Learning: Applies to go/deploy/deploy/*/Makefile : Service-level makefile should be `<service>/Makefile`.

Applied to files:

  • apps/agent/Makefile
  • go/Makefile
📚 Learning: applies to go/deploy/deploy/{assetmanagerd,billaged,builderd,metald}/**/*.go : when a service's `*.g...
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-21T18:05:58.236Z
Learning: Applies to go/deploy/deploy/{assetmanagerd,billaged,builderd,metald}/**/*.go : When a service's `*.go` code changes significantly, increase the patch-level version number.

Applied to files:

  • apps/agent/Makefile
  • go/Makefile
📚 Learning: applies to go/deploy/deploy/{assetmanagerd,billaged,builderd,metald}/**/makefile : never use `go bui...
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-21T18:05:58.236Z
Learning: Applies to go/deploy/deploy/{assetmanagerd,billaged,builderd,metald}/**/Makefile : Never use `go build` for any of the `assetmanagerd`, `billaged`, `builderd`, `metald` binaries.

Applied to files:

  • apps/agent/Makefile
  • go/Makefile
📚 Learning: applies to go/deploy/deploy/{assetmanagerd,billaged,builderd,metald}/**/makefile : use `make install...
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-21T18:05:58.236Z
Learning: Applies to go/deploy/deploy/{assetmanagerd,billaged,builderd,metald}/**/Makefile : Use `make install` to build and install the binary with systemd unit from `$SERVICE/contrib/systemd`.

Applied to files:

  • apps/agent/Makefile
  • go/Makefile
📚 Learning: applies to **/*_test.go : organize go integration tests with real dependencies...
Learnt from: CR
PR: unkeyed/unkey#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-04T07:44:39.438Z
Learning: Applies to **/*_test.go : Organize Go integration tests with real dependencies

Applied to files:

  • apps/agent/Makefile
  • go/Makefile
📚 Learning: applies to **/*.go : use aidev-* comments for complex/important code in go services...
Learnt from: CR
PR: unkeyed/unkey#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-04T07:44:39.438Z
Learning: Applies to **/*.go : Use AIDEV-* comments for complex/important code in Go services

Applied to files:

  • apps/agent/Makefile
  • go/Makefile
📚 Learning: applies to **/*_test.go : use table-driven tests in go...
Learnt from: CR
PR: unkeyed/unkey#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-04T07:44:39.438Z
Learning: Applies to **/*_test.go : Use table-driven tests in Go

Applied to files:

  • apps/agent/Makefile
  • go/Makefile
📚 Learning: applies to **/*.go : follow comprehensive documentation guidelines for go code as described in go/go...
Learnt from: CR
PR: unkeyed/unkey#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-04T07:44:39.438Z
Learning: Applies to **/*.go : Follow comprehensive documentation guidelines for Go code as described in go/GO_DOCUMENTATION_GUIDELINES.md

Applied to files:

  • apps/agent/Makefile
  • go/Makefile
📚 Learning: applies to **/*.go : prefer interfaces for testability in go code...
Learnt from: CR
PR: unkeyed/unkey#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-04T07:44:39.438Z
Learning: Applies to **/*.go : Prefer interfaces for testability in Go code

Applied to files:

  • apps/agent/Makefile
📚 Learning: applies to **/doc.go : use doc.go files for go package documentation...
Learnt from: CR
PR: unkeyed/unkey#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-04T07:44:39.438Z
Learning: Applies to **/doc.go : Use doc.go files for Go package documentation

Applied to files:

  • apps/agent/Makefile
🪛 actionlint (1.7.7)
.github/workflows/deploy.yaml

15-15: could not read reusable workflow file for "./.github/workflows/job_test_agent_local.yaml": open /home/jailuser/git/.github/workflows/job_test_agent_local.yaml: no such file or directory

(workflow-call)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
  • GitHub Check: Test API / API Test Local
  • GitHub Check: Test Go API Local / Test
  • GitHub Check: Build / Build
  • GitHub Check: Test Packages / Test
  • GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (2)
apps/agent/Makefile (1)

23-23: tparse hard-dependency may break fresh CI runners

The test target pipes go test JSON into tparse, but the Makefile never installs it. Add an explicit go install github.com/mfridman/tparse@latest step (e.g., in install:) or gate the pipe with command -v tparse.

go/Makefile (1)

31-31: --force-recreate in migration loop may litter exited containers

Each docker compose run --force-recreate --rm clickhouse_migrator creates a new one-off container; if the migration keeps failing for the first N-1 attempts, you will spawn and then remove N containers. With --rm it’s generally fine, but double-check your CI logs/quota for noise. If you only need to restart the same container, consider docker compose up --abort-on-container-exit.

Copy link
Collaborator

chronark commented Aug 7, 2025

:bugeyes:

Copy link
Collaborator

@chronark chronark left a comment

Choose a reason for hiding this comment

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

lgtm, that one rabbit comment says ci might be broken, we'll see

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

♻️ Duplicate comments (1)
go/Makefile (1)

47-52: Previous 15 min global timeout discussion – just noting duplication
The hard-coded -timeout=15m has already been debated and accepted as “fine” for this repo (see prior thread). No further action.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between cc39f15 and 849250b.

📒 Files selected for processing (3)
  • .github/workflows/deploy.yaml (0 hunks)
  • apps/agent/Makefile (2 hunks)
  • go/Makefile (2 hunks)
💤 Files with no reviewable changes (1)
  • .github/workflows/deploy.yaml
🧰 Additional context used
🧠 Learnings (21)
📚 Learning: applies to go/deploy/**/makefile : global makefile should be `makefile` at the repository root....
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-21T18:05:58.236Z
Learning: Applies to go/deploy/**/Makefile : Global makefile should be `Makefile` at the repository root.

Applied to files:

  • go/Makefile
  • apps/agent/Makefile
📚 Learning: applies to go/deploy/deploy/{assetmanagerd,billaged,builderd,metald}/**/*.go : when a service's `*.g...
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-21T18:05:58.236Z
Learning: Applies to go/deploy/deploy/{assetmanagerd,billaged,builderd,metald}/**/*.go : When a service's `*.go` code changes significantly, increase the patch-level version number.

Applied to files:

  • go/Makefile
  • apps/agent/Makefile
📚 Learning: applies to go/deploy/deploy/{assetmanagerd,billaged,builderd,metald}/**/makefile : use `make build` ...
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-21T18:05:58.236Z
Learning: Applies to go/deploy/deploy/{assetmanagerd,billaged,builderd,metald}/**/Makefile : Use `make build` to test that the binary builds.

Applied to files:

  • go/Makefile
  • apps/agent/Makefile
📚 Learning: applies to go/deploy/deploy/*/makefile : service-level makefile should be `/makefile`....
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-21T18:05:58.236Z
Learning: Applies to go/deploy/deploy/*/Makefile : Service-level makefile should be `<service>/Makefile`.

Applied to files:

  • go/Makefile
  • apps/agent/Makefile
📚 Learning: applies to go/deploy/deploy/{assetmanagerd,billaged,builderd,metald}/**/makefile : never use `go bui...
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-21T18:05:58.236Z
Learning: Applies to go/deploy/deploy/{assetmanagerd,billaged,builderd,metald}/**/Makefile : Never use `go build` for any of the `assetmanagerd`, `billaged`, `builderd`, `metald` binaries.

Applied to files:

  • go/Makefile
  • apps/agent/Makefile
📚 Learning: applies to **/*_test.go : use table-driven tests in go...
Learnt from: CR
PR: unkeyed/unkey#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-04T07:44:39.438Z
Learning: Applies to **/*_test.go : Use table-driven tests in Go

Applied to files:

  • go/Makefile
  • apps/agent/Makefile
📚 Learning: applies to **/*.go : follow comprehensive documentation guidelines for go code as described in go/go...
Learnt from: CR
PR: unkeyed/unkey#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-04T07:44:39.438Z
Learning: Applies to **/*.go : Follow comprehensive documentation guidelines for Go code as described in go/GO_DOCUMENTATION_GUIDELINES.md

Applied to files:

  • go/Makefile
  • apps/agent/Makefile
📚 Learning: applies to **/*.go : use aidev-* comments for complex/important code in go services...
Learnt from: CR
PR: unkeyed/unkey#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-04T07:44:39.438Z
Learning: Applies to **/*.go : Use AIDEV-* comments for complex/important code in Go services

Applied to files:

  • go/Makefile
  • apps/agent/Makefile
📚 Learning: applies to **/*_test.go : organize go integration tests with real dependencies...
Learnt from: CR
PR: unkeyed/unkey#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-04T07:44:39.438Z
Learning: Applies to **/*_test.go : Organize Go integration tests with real dependencies

Applied to files:

  • go/Makefile
  • apps/agent/Makefile
📚 Learning: applies to go/deploy/**/*.{go,js,ts,tsx,py,sh,md,txt,json,yaml,yml,ini,env,conf,html,css,scss,xml,c,...
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-21T18:05:58.236Z
Learning: Applies to go/deploy/**/*.{go,js,ts,tsx,py,sh,md,txt,json,yaml,yml,ini,env,conf,html,css,scss,xml,c,h,cpp,java,rb,rs,php,pl,sql} : Use `AIDEV-NOTE:`, `AIDEV-TODO:`, `AIDEV-BUSINESS_RULE:`, or `AIDEV-QUESTION:` (all-caps prefix) as anchor comments aimed at AI and developers.

Applied to files:

  • go/Makefile
  • apps/agent/Makefile
📚 Learning: applies to **/*.go : prefer interfaces for testability in go code...
Learnt from: CR
PR: unkeyed/unkey#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-04T07:44:39.438Z
Learning: Applies to **/*.go : Prefer interfaces for testability in Go code

Applied to files:

  • go/Makefile
  • apps/agent/Makefile
📚 Learning: applies to go/deploy/**/*.{go,js,ts,tsx,py,sh,md,txt,json,yaml,yml,ini,env,conf,html,css,scss,xml,c,...
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-21T18:05:58.236Z
Learning: Applies to go/deploy/**/*.{go,js,ts,tsx,py,sh,md,txt,json,yaml,yml,ini,env,conf,html,css,scss,xml,c,h,cpp,java,rb,rs,php,pl,sql} : Make sure to add relevant anchor comments whenever a file or piece of code is too complex, very important, confusing, or could have a bug.

Applied to files:

  • go/Makefile
  • apps/agent/Makefile
📚 Learning: applies to go/deploy/**/*.{go,js,ts,tsx,py,sh,md,txt,json,yaml,yml,ini,env,conf,html,css,scss,xml,c,...
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-21T18:05:58.236Z
Learning: Applies to go/deploy/**/*.{go,js,ts,tsx,py,sh,md,txt,json,yaml,yml,ini,env,conf,html,css,scss,xml,c,h,cpp,java,rb,rs,php,pl,sql} : Do not remove `AIDEV-*`s without explicit human instruction.

Applied to files:

  • go/Makefile
  • apps/agent/Makefile
📚 Learning: applies to **/*_test.go : organize go tests by http status codes...
Learnt from: CR
PR: unkeyed/unkey#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-04T07:44:39.438Z
Learning: Applies to **/*_test.go : Organize Go tests by HTTP status codes

Applied to files:

  • go/Makefile
📚 Learning: in the go/cmd/cli/commands/deploy/ cli codebase, ogzhanolguncu prefers to allow deployment to contin...
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#3564
File: go/cmd/cli/commands/deploy/deploy.go:153-158
Timestamp: 2025-07-16T09:18:45.379Z
Learning: In the go/cmd/cli/commands/deploy/ CLI codebase, ogzhanolguncu prefers to allow deployment to continue even when Docker push fails (around lines 153-158 in deploy.go) because the team is working locally and needs this behavior for local development workflows where registry access might not be available.

Applied to files:

  • go/Makefile
  • apps/agent/Makefile
📚 Learning: in go/cmd/cli/commands/deploy/deploy.go, the debug_delay constant is temporary code that ogzhanolgun...
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#3564
File: go/cmd/cli/commands/deploy/deploy.go:14-14
Timestamp: 2025-07-15T14:47:43.718Z
Learning: In go/cmd/cli/commands/deploy/deploy.go, the DEBUG_DELAY constant is temporary code that ogzhanolguncu plans to remove in a future PR, rather than making it configurable.

Applied to files:

  • go/Makefile
📚 Learning: applies to go/deploy/**/*.{go,js,ts,tsx,py,sh,md,txt,json,yaml,yml,ini,env,conf,html,css,scss,xml,c,...
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-21T18:05:58.236Z
Learning: Applies to go/deploy/**/*.{go,js,ts,tsx,py,sh,md,txt,json,yaml,yml,ini,env,conf,html,css,scss,xml,c,h,cpp,java,rb,rs,php,pl,sql} : Update relevant anchors when modifying associated code.

Applied to files:

  • go/Makefile
📚 Learning: in the go/cmd/cli/commands/deploy/ directory, ogzhanolguncu prefers to keep potentially temporary fe...
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#3564
File: go/cmd/cli/commands/deploy/flags.go:17-20
Timestamp: 2025-07-15T14:45:18.920Z
Learning: In the go/cmd/cli/commands/deploy/ directory, ogzhanolguncu prefers to keep potentially temporary features (like UNKEY_DOCKER_REGISTRY environment variable) undocumented in help text if they might be deleted in the future, to avoid documentation churn.

Applied to files:

  • go/Makefile
  • apps/agent/Makefile
📚 Learning: applies to go/deploy/deploy/{assetmanagerd,billaged,builderd,metald}/**/makefile : use `make install...
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-21T18:05:58.236Z
Learning: Applies to go/deploy/deploy/{assetmanagerd,billaged,builderd,metald}/**/Makefile : Use `make install` to build and install the binary with systemd unit from `$SERVICE/contrib/systemd`.

Applied to files:

  • go/Makefile
  • apps/agent/Makefile
📚 Learning: never delete anything from this file....
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-21T18:05:58.236Z
Learning: Never delete anything from this file.

Applied to files:

  • apps/agent/Makefile
📚 Learning: applies to **/doc.go : use doc.go files for go package documentation...
Learnt from: CR
PR: unkeyed/unkey#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-04T07:44:39.438Z
Learning: Applies to **/doc.go : Use doc.go files for Go package documentation

Applied to files:

  • apps/agent/Makefile
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Analyze (javascript-typescript)

@vercel vercel bot temporarily deployed to Preview – dashboard August 7, 2025 15:57 Inactive
@graphite-app
Copy link

graphite-app bot commented Aug 7, 2025

TV gif. Jeremy Clarkson gives a thumbs up and a sarcastic, goofy, toothy smile. (Added via Giphy)

@graphite-app
Copy link

graphite-app bot commented Aug 7, 2025

Graphite Automations

"Post a GIF when PR approved" took an action on this PR • (08/07/25)

1 gif was posted to this PR based on Andreas Thomas's automation.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 849250b and 4d089cc.

📒 Files selected for processing (2)
  • deployment/04-seed-workspace.sql (3 hunks)
  • deployment/docker-compose.yaml (3 hunks)
🧰 Additional context used
🧠 Learnings (9)
📚 Learning: 2025-07-22T09:02:12.495Z
Learnt from: chronark
PR: unkeyed/unkey#3638
File: deployment/docker-compose.yaml:81-94
Timestamp: 2025-07-22T09:02:12.495Z
Learning: The docker-compose.yaml file in deployment/ is specifically for development environments, not production. Kafka and other service configurations in this file should be optimized for development convenience rather than production security/hardening.

Applied to files:

  • deployment/docker-compose.yaml
📚 Learning: 2025-04-22T14:43:11.724Z
Learnt from: chronark
PR: unkeyed/unkey#3161
File: go/pkg/clickhouse/schema/databases/002_ratelimits/006_ratelimits_per_day_v1.sql:1-13
Timestamp: 2025-04-22T14:43:11.724Z
Learning: In the unkey project, the SQL files in clickhouse/schema/databases represent the current production schema and shouldn't be modified directly in PRs. Schema changes require dedicated migration scripts.

Applied to files:

  • deployment/04-seed-workspace.sql
📚 Learning: 2025-04-08T09:34:24.576Z
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#2872
File: apps/dashboard/lib/trpc/routers/ratelimit/createNamespace.ts:36-39
Timestamp: 2025-04-08T09:34:24.576Z
Learning: When querying or updating namespaces in the Unkey dashboard, always scope the operations to the current workspace using `eq(table.workspaceId, ctx.workspace.id)` to prevent cross-workspace access.

Applied to files:

  • deployment/04-seed-workspace.sql
📚 Learning: 2024-10-04T20:44:38.489Z
Learnt from: chronark
PR: unkeyed/unkey#2180
File: apps/dashboard/lib/constants/workspace-navigations.tsx:56-118
Timestamp: 2024-10-04T20:44:38.489Z
Learning: When typing the `workspace` parameter in functions like `createWorkspaceNavigation`, prefer importing the `Workspace` type from the database module and picking the necessary keys (e.g., `features`) instead of redefining the interface.

Applied to files:

  • deployment/04-seed-workspace.sql
📚 Learning: 2025-07-07T13:22:10.989Z
Learnt from: chronark
PR: unkeyed/unkey#3426
File: deployment/init-databases.sql:0-0
Timestamp: 2025-07-07T13:22:10.989Z
Learning: The database initialization script in deployment/init-databases.sql is specifically for local development environments, not production. The permissive settings (wildcard host '%' and ALL PRIVILEGES) are intentionally used for local dev convenience.

Applied to files:

  • deployment/04-seed-workspace.sql
📚 Learning: 2025-04-08T09:34:24.576Z
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#2872
File: apps/dashboard/lib/trpc/routers/ratelimit/createNamespace.ts:36-39
Timestamp: 2025-04-08T09:34:24.576Z
Learning: In the Unkey dashboard, when making database queries involving workspaces, use `ctx.workspace.id` directly instead of fetching the workspace separately for better performance and security.

Applied to files:

  • deployment/04-seed-workspace.sql
📚 Learning: 2025-04-22T14:40:51.459Z
Learnt from: chronark
PR: unkeyed/unkey#3161
File: go/pkg/clickhouse/schema/databases/001_verifications/002_raw_key_verifications_v1.sql:31-33
Timestamp: 2025-04-22T14:40:51.459Z
Learning: The ClickHouse table schemas in the codebase mirror the production environment and cannot be modified directly in PRs without careful migration planning.

Applied to files:

  • deployment/04-seed-workspace.sql
📚 Learning: 2025-07-25T19:09:43.284Z
Learnt from: mcstepp
PR: unkeyed/unkey#3662
File: apps/dashboard/lib/trpc/routers/deployment/list.ts:11-11
Timestamp: 2025-07-25T19:09:43.284Z
Learning: In apps/dashboard/lib/trpc/routers/deployment/list.ts, the listDeployments procedure intentionally queries the versions table rather than a deployments table. The user mcstepp indicated that renaming the table would require a database migration, which was deferred for the current PR focused on UI features.

Applied to files:

  • deployment/04-seed-workspace.sql
📚 Learning: 2025-07-17T14:24:20.403Z
Learnt from: Flo4604
PR: unkeyed/unkey#3631
File: go/pkg/db/bulk_keyring_insert.sql.go:23-25
Timestamp: 2025-07-17T14:24:20.403Z
Learning: In go/pkg/db/bulk_keyring_insert.sql.go and similar bulk insert generated files, hardcoded zero values for fields like size_approx and size_last_updated_at are intentional and reflect the original SQL query structure, not missing parameters.

Applied to files:

  • deployment/04-seed-workspace.sql
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
  • GitHub Check: Test API / API Test Local
  • GitHub Check: Build / Build
  • GitHub Check: Test Go API Local / Test
  • GitHub Check: Test Packages / Test
  • GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (4)
deployment/04-seed-workspace.sql (3)

8-11: Column-list whitespace cleanup looks good
Purely cosmetic change – no functional impact.


23-35: ON DUPLICATE KEY clause is effectively a no-op & may hide real errors

workspace_id is (very likely) the primary/unique key of quota.
Updating it to the same literal value does nothing, so:

  1. If the row already exists, no fields are refreshed (e.g. retention limits).
  2. If schema ever adds an updated_at column, this script won’t touch it.
  3. It can mask typos in the VALUES section because the statement will
    “succeed” even when nothing was actually updated.

Consider updating a meaningful column (e.g. requests_per_month) or
dropping the clause entirely to let the insert fail loudly.

-) ON DUPLICATE KEY UPDATE workspace_id = 'ws_local_root';
+) ON DUPLICATE KEY UPDATE
+    requests_per_month      = VALUES(requests_per_month),
+    audit_logs_retention_days = VALUES(audit_logs_retention_days),
+    logs_retention_days     = VALUES(logs_retention_days);

If quota really should be immutable after bootstrap, use INSERT IGNORE
instead for clearer intent.
Would you confirm the actual primary-key definition and desired idempotency semantics?


63-64: Trailing newline – OK
Keeps POSIX tools happy; no action needed.

deployment/docker-compose.yaml (1)

20-27: Health-check addition is 👍

Fast-fails dependent services and documents readiness criteria.

@imeyer imeyer added this pull request to the merge queue Aug 7, 2025
Merged via the queue into main with commit 0d91d17 Aug 7, 2025
18 checks passed
@imeyer imeyer deleted the push-kzutyuvtvvkl branch August 7, 2025 17:20
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.

2 participants