Conversation
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 24
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
engine/project/indexer.go (2)
276-287: Apply default to copy, not original Config.Lines 276-278 mutate
memory.Resourceon the original struct inp.Memories, causing a persistent side effect. Defaults should be applied to the copy used in the goroutine to avoid modifying the input Config.Apply this diff:
- if memory.Resource == "" { - memory.Resource = string(resources.ResourceMemory) - } if err := memory.Validate(ctx); err != nil { return fmt.Errorf("memory '%s' validation failed: %w", memory.ID, err) } key := resources.ResourceKey{Project: p.Name, Type: resources.ResourceMemory, ID: memory.ID} keyCopy := key mem := memory + if mem.Resource == "" { + mem.Resource = string(resources.ResourceMemory) + } group.Go(func() error { - return p.putResourceWithMeta(ctx, store, metaSources, keyCopy, mem) + return p.putResourceWithMeta(groupCtx, store, metaSources, keyCopy, mem) })Additionally, the goroutine should use
groupCtxinstead ofctx(same issue as noted forindexProjectTools).
391-399: Apply default to copy, not original Config.Lines 391-393 mutate
knowledgeBase.Ingeston the original struct inp.KnowledgeBases, causing a persistent side effect. Defaults should be applied to the copy used in the goroutine to avoid modifying the input Config.Apply this diff:
- if knowledgeBase.Ingest == "" { - knowledgeBase.Ingest = knowledge.IngestManual - } key := resources.ResourceKey{Project: p.Name, Type: resources.ResourceKnowledgeBase, ID: knowledgeBase.ID} keyCopy := key knowledgeBaseCopy := knowledgeBase + if knowledgeBaseCopy.Ingest == "" { + knowledgeBaseCopy.Ingest = knowledge.IngestManual + } group.Go(func() error { - return p.putResourceWithMeta(ctx, store, metaSources, keyCopy, knowledgeBaseCopy) + return p.putResourceWithMeta(groupCtx, store, metaSources, keyCopy, knowledgeBaseCopy) })Additionally, the goroutine should use
groupCtxinstead ofctx(same issue as noted forindexProjectTools).
♻️ Duplicate comments (2)
docs/docs.go (1)
13402-13414: Previous duplicate enum issue resolved.The
memory.PrivacyScopeenum now contains only unique values ("global", "user", "session"). The duplicate entries mentioned in the previous review have been removed.sdk/compozy/engine_registration.go (1)
59-63: Rollback engine state when persistence fails. We assigne.projectbefore persisting; ifpersistResourcereturns an error (e.g., store already has the project or the store call fails) the method exits withe.projectstill pointing at the new config even though registration failed. That breaks duplicate detection and leaves the engine in an inconsistent state. Restoree.projectwhen persistence fails so the in-memory view matches the returned error.@@ - e.project = cfg - store := e.resourceStore - e.stateMu.Unlock() - return e.persistResource(e.ctx, store, name, resources.ResourceProject, name, cfg, source) + e.project = cfg + store := e.resourceStore + e.stateMu.Unlock() + if err := e.persistResource(e.ctx, store, name, resources.ResourceProject, name, cfg, source); err != nil { + e.stateMu.Lock() + if e.project == cfg { + e.project = nil + } + e.stateMu.Unlock() + return err + } + return nil
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📥 Commits
Reviewing files that changed from the base of the PR and between b111f351b8f386525ef93f83094115b4ae5bdfe0 and 9e18dd3.
⛔ Files ignored due to path filters (74)
.golangci.ymlis excluded by!**/*.ymlai-docs/reviews-pr-316/issues/011-issue.mdis excluded by!**/*.mdai-docs/reviews-pr-316/issues/012-issue.mdis excluded by!**/*.mdai-docs/reviews-pr-316/issues/013-issue.mdis excluded by!**/*.mdai-docs/reviews-pr-316/issues/014-issue.mdis excluded by!**/*.mdai-docs/reviews-pr-316/issues/015-issue.mdis excluded by!**/*.mdai-docs/reviews-pr-316/issues/016-issue.mdis excluded by!**/*.mdai-docs/reviews-pr-316/issues/017-issue.mdis excluded by!**/*.mdai-docs/reviews-pr-316/issues/018-issue.mdis excluded by!**/*.mdai-docs/reviews-pr-316/issues/019-issue.mdis excluded by!**/*.mdai-docs/reviews-pr-316/issues/020-issue.mdis excluded by!**/*.mdgo.sumis excluded by!**/*.sumgo.work.sumis excluded by!**/*.sumsdk/examples/README.mdis excluded by!**/*.mdtasks/prd-modes/TEMPLATE_SYSTEM_ANALYSIS.mdis excluded by!**/*.mdtasks/prd-modes/_docs.mdis excluded by!**/*.mdtasks/prd-modes/_examples.mdis excluded by!**/*.mdtasks/prd-modes/_prd.mdis excluded by!**/*.mdtasks/prd-modes/_task_1.0.mdis excluded by!**/*.mdtasks/prd-modes/_task_10.0.mdis excluded by!**/*.mdtasks/prd-modes/_task_11.0.mdis excluded by!**/*.mdtasks/prd-modes/_task_12.0.mdis excluded by!**/*.mdtasks/prd-modes/_task_13.0.mdis excluded by!**/*.mdtasks/prd-modes/_task_14.0.mdis excluded by!**/*.mdtasks/prd-modes/_task_15.0.mdis excluded by!**/*.mdtasks/prd-modes/_task_16.0.mdis excluded by!**/*.mdtasks/prd-modes/_task_17.0.mdis excluded by!**/*.mdtasks/prd-modes/_task_18.0.mdis excluded by!**/*.mdtasks/prd-modes/_task_19.0.mdis excluded by!**/*.mdtasks/prd-modes/_task_2.0.mdis excluded by!**/*.mdtasks/prd-modes/_task_20.0.mdis excluded by!**/*.mdtasks/prd-modes/_task_21.0.mdis excluded by!**/*.mdtasks/prd-modes/_task_22.0.mdis excluded by!**/*.mdtasks/prd-modes/_task_23.0.mdis excluded by!**/*.mdtasks/prd-modes/_task_24.0.mdis excluded by!**/*.mdtasks/prd-modes/_task_25.0.mdis excluded by!**/*.mdtasks/prd-modes/_task_26.0.mdis excluded by!**/*.mdtasks/prd-modes/_task_27.0.mdis excluded by!**/*.mdtasks/prd-modes/_task_28.0.mdis excluded by!**/*.mdtasks/prd-modes/_task_29.0.mdis excluded by!**/*.mdtasks/prd-modes/_task_3.0.mdis excluded by!**/*.mdtasks/prd-modes/_task_4.0.mdis excluded by!**/*.mdtasks/prd-modes/_task_5.0.mdis excluded by!**/*.mdtasks/prd-modes/_task_6.0.mdis excluded by!**/*.mdtasks/prd-modes/_task_7.0.mdis excluded by!**/*.mdtasks/prd-modes/_task_8.0.mdis excluded by!**/*.mdtasks/prd-modes/_task_9.0.mdis excluded by!**/*.mdtasks/prd-modes/_tasks.mdis excluded by!**/*.mdtasks/prd-modes/_techspec.mdis excluded by!**/*.mdtasks/prd-modes/_tests.mdis excluded by!**/*.mdtasks/prd-redis/_docs.mdis excluded by!**/*.mdtasks/prd-redis/_examples.mdis excluded by!**/*.mdtasks/prd-redis/_prd.mdis excluded by!**/*.mdtasks/prd-redis/_task_01.mdis excluded by!**/*.mdtasks/prd-redis/_task_02.mdis excluded by!**/*.mdtasks/prd-redis/_task_03.mdis excluded by!**/*.mdtasks/prd-redis/_task_04.mdis excluded by!**/*.mdtasks/prd-redis/_task_05.mdis excluded by!**/*.mdtasks/prd-redis/_task_06.mdis excluded by!**/*.mdtasks/prd-redis/_task_07.mdis excluded by!**/*.mdtasks/prd-redis/_task_08.mdis excluded by!**/*.mdtasks/prd-redis/_task_09.mdis excluded by!**/*.mdtasks/prd-redis/_task_10.mdis excluded by!**/*.mdtasks/prd-redis/_task_11.mdis excluded by!**/*.mdtasks/prd-redis/_task_12.mdis excluded by!**/*.mdtasks/prd-redis/_task_13.mdis excluded by!**/*.mdtasks/prd-redis/_tasks.mdis excluded by!**/*.mdtasks/prd-redis/_techspec.mdis excluded by!**/*.mdtasks/prd-redis/_tests.mdis excluded by!**/*.mdtasks/prd-tools/_task_1.mdis excluded by!**/*.mdtasks/prd-tools/_task_2.mdis excluded by!**/*.mdtasks/prd-tools/_task_3.mdis excluded by!**/*.mdtasks/prd-tools/_tasks.mdis excluded by!**/*.mdtasks/prd-tools/_techspec.mdis excluded by!**/*.md
📒 Files selected for processing (72)
docs/docs.go(27 hunks)engine/agent/action_config.go(3 hunks)engine/llm/service.go(3 hunks)engine/llm/tool_registry.go(2 hunks)engine/llm/tool_registry_test.go(2 hunks)engine/memory/config.go(6 hunks)engine/project/indexer.go(8 hunks)engine/project/schedule/config.go(1 hunks)engine/resources/store.go(1 hunks)engine/tool/config.go(7 hunks)engine/tool/inline/manager.go(1 hunks)engine/tool/inline/manager_test.go(1 hunks)engine/tool/nativeuser/registry.go(1 hunks)engine/tool/nativeuser/registry_test.go(1 hunks)engine/tool/router/dto.go(2 hunks)engine/tool/router/tools_top.go(1 hunks)engine/tool/uc/store_decode.go(2 hunks)engine/tool/uc/store_errors.go(1 hunks)engine/tool/uc/store_test.go(1 hunks)engine/worker/embedded/server.go(1 hunks)go.mod(3 hunks)magefile.go(1 hunks)scripts/markdown/check.go(29 hunks)sdk/agent/constructor.go(1 hunks)sdk/agent/constructor_test.go(1 hunks)sdk/agent/generate.go(1 hunks)sdk/agentaction/constructor_test.go(1 hunks)sdk/client/execution_helpers.go(1 hunks)sdk/client/http.go(1 hunks)sdk/client/task.go(1 hunks)sdk/client/workflow.go(1 hunks)sdk/compozy/app_test.go(1 hunks)sdk/compozy/cleanup_test.go(1 hunks)sdk/compozy/codegen/generator_test.go(1 hunks)sdk/compozy/config/yaml_loader_test.go(1 hunks)sdk/compozy/constructor.go(1 hunks)sdk/compozy/constructor_clone_test.go(1 hunks)sdk/compozy/engine.go(1 hunks)sdk/compozy/engine_loading.go(1 hunks)sdk/compozy/engine_loading_errors_test.go(1 hunks)sdk/compozy/engine_registration.go(1 hunks)sdk/compozy/integration/distributed_integration_test.go(1 hunks)sdk/compozy/integration/hybrid_yaml_integration_test.go(1 hunks)sdk/compozy/integration/standalone_integration_test.go(1 hunks)sdk/compozy/lifecycle.go(1 hunks)sdk/compozy/lifecycle_test.go(1 hunks)sdk/compozy/loader.go(1 hunks)sdk/compozy/loader_performance_test.go(1 hunks)sdk/compozy/loader_test.go(1 hunks)sdk/compozy/loading_test.go(1 hunks)sdk/compozy/migration/example_compat_test.go(1 hunks)sdk/compozy/mode_test.go(1 hunks)sdk/compozy/options_test.go(1 hunks)sdk/compozy/registration_errors_test.go(1 hunks)sdk/compozy/registration_test.go(1 hunks)sdk/compozy/resources_graph_test.go(1 hunks)sdk/compozy/validation.go(1 hunks)sdk/compozy/validation_nodes_test.go(1 hunks)sdk/compozy/validation_test.go(1 hunks)sdk/examples/00_shared.go(1 hunks)sdk/examples/01_simple_workflow.go(1 hunks)sdk/examples/02_parallel_tasks.go(1 hunks)sdk/examples/03_knowledge_rag.go(1 hunks)sdk/examples/04_memory_conversation.go(1 hunks)sdk/examples/05_runtime_native_tools.go(1 hunks)sdk/examples/06_scheduled_workflow.go(1 hunks)sdk/examples/07_signal_communication.go(1 hunks)sdk/examples/08_model_routing.go(1 hunks)sdk/examples/09_debugging_tracing.go(1 hunks)sdk/examples/10_complete_project.go(1 hunks)sdk/examples/main.go(1 hunks)sdk/internal/codegen/generator.go(1 hunks)
🧰 Additional context used
📓 Path-based instructions (11)
**/*.go
📄 CodeRabbit inference engine (.cursor/rules/api-standards.mdc)
**/*.go: Return properly structured error responses
Use middleware for cross-cutting concerns (e.g., logging, auth, rate limiting)
Implement proper authentication and authorization in the API
Apply rate limiting and request validation
Version API routes under the prefix /api/v0/
Use gin-gonic/gin for HTTP APIs
Ensure consistent response formats across all endpoints
Use appropriate HTTP status codes (200, 201, 400, 401, 403, 404, 500)
Return JSON responses with a consistent error structure
Update Swagger annotations for all API changes
Generate Swagger docs served at /swagger/index.html using swaggo/swag
Include request and response examples in Swagger annotations
Document all parameters, headers, and error responses in Swagger
Success response body should contain fields: data and message ("Success")
Error response body should contain fields: error and details
**/*.go: Retrieve the logger via logger.FromContext(ctx) inside runtime code (handlers, services, workers)
Retrieve configuration via config.FromContext(ctx) inside runtime code (handlers, services, workers)
Attach *config.Manager at process edges using config.ContextWithManager(ctx, mgr)
Build the logger with logger.SetupLogger(...) at startup and attach with logger.ContextWithLogger(ctx, log)
For HTTP servers, ensure request contexts inherit BOTH manager and logger via middleware
Do not pass loggers via function parameters or dependency injection; always use context-backed retrieval
Do not use a global configuration singleton; read config from context
CLI/root flow: parse flags → construct sources → mgr.Load(...) → attach manager to ctx → setup logger honoring cfg.CLI.Debug/Quiet → attach logger to ctx → propagate ctx
Server startup should use the CLI-provided ctx and add middleware that sets c.Request = c.Request.WithContext(logger.ContextWithLogger(config.ContextWithManager(c.Request.Context(), mgr), log))
Optionally set http.Server.BaseContext to a parent context carrying manager and logger so all req...
Files:
engine/tool/uc/store_errors.gosdk/compozy/resources_graph_test.goengine/worker/embedded/server.gosdk/agent/constructor_test.goengine/resources/store.goengine/tool/router/tools_top.goengine/project/indexer.gosdk/examples/03_knowledge_rag.gosdk/compozy/integration/distributed_integration_test.gosdk/compozy/loader_test.gosdk/compozy/loading_test.gosdk/examples/09_debugging_tracing.gosdk/examples/01_simple_workflow.goengine/project/schedule/config.gosdk/compozy/app_test.goengine/tool/nativeuser/registry_test.gosdk/compozy/constructor.gosdk/compozy/validation_test.gosdk/internal/codegen/generator.gosdk/examples/02_parallel_tasks.goengine/llm/service.gosdk/examples/06_scheduled_workflow.gosdk/client/http.gosdk/agent/constructor.gosdk/compozy/loader_performance_test.gosdk/compozy/registration_errors_test.gosdk/compozy/engine.gosdk/examples/05_runtime_native_tools.goengine/tool/config.goengine/tool/nativeuser/registry.gosdk/examples/00_shared.gosdk/compozy/integration/standalone_integration_test.gosdk/compozy/options_test.gosdk/examples/10_complete_project.gosdk/agent/generate.goengine/agent/action_config.gosdk/compozy/config/yaml_loader_test.gosdk/compozy/validation_nodes_test.goengine/tool/inline/manager_test.gosdk/examples/07_signal_communication.gosdk/compozy/loader.gosdk/examples/main.gosdk/compozy/engine_loading_errors_test.gosdk/compozy/migration/example_compat_test.gosdk/compozy/mode_test.gosdk/compozy/engine_loading.goengine/tool/inline/manager.gosdk/compozy/registration_test.gosdk/examples/04_memory_conversation.goengine/tool/uc/store_decode.goengine/llm/tool_registry.gosdk/compozy/validation.gosdk/examples/08_model_routing.gosdk/compozy/cleanup_test.goengine/tool/router/dto.gosdk/compozy/constructor_clone_test.goengine/memory/config.gosdk/agentaction/constructor_test.gosdk/client/execution_helpers.goengine/tool/uc/store_test.gosdk/compozy/codegen/generator_test.gosdk/compozy/engine_registration.gosdk/client/workflow.godocs/docs.gosdk/client/task.gomagefile.goscripts/markdown/check.gosdk/compozy/lifecycle_test.goengine/llm/tool_registry_test.gosdk/compozy/integration/hybrid_yaml_integration_test.gosdk/compozy/lifecycle.go
**/!(*_test).go
📄 CodeRabbit inference engine (.cursor/rules/magic-numbers.mdc)
**/!(*_test).go: NEVER introduce magic numbers in runtime Go code; replace unexplained numeric literals with named constants or configuration
Operator-tunable values MUST be sourced from configuration as defined in @.cursor/rules/global-config.mdc
Fetch configuration via config.FromContext(ctx); do not use global singletons
Promote tunable values (timeouts, deadlines, polling intervals, retry/backoff counts and factors, concurrency limits, queue sizes, buffer/payload limits, behavior thresholds) to configurationIn runtime code, properly inherit context; never use context.Background() in code paths
In runtime code paths, inherit context properly; never use context.Background()
In runtime code paths, never use context.Background(); properly inherit context
In runtime code, properly inherit context; never use context.Background()
Files:
engine/tool/uc/store_errors.goengine/worker/embedded/server.goengine/resources/store.goengine/tool/router/tools_top.goengine/project/indexer.gosdk/examples/03_knowledge_rag.gosdk/examples/09_debugging_tracing.gosdk/examples/01_simple_workflow.goengine/project/schedule/config.gosdk/compozy/constructor.gosdk/internal/codegen/generator.gosdk/examples/02_parallel_tasks.goengine/llm/service.gosdk/examples/06_scheduled_workflow.gosdk/client/http.gosdk/agent/constructor.gosdk/compozy/engine.gosdk/examples/05_runtime_native_tools.goengine/tool/config.goengine/tool/nativeuser/registry.gosdk/examples/00_shared.gosdk/examples/10_complete_project.gosdk/agent/generate.goengine/agent/action_config.gosdk/examples/07_signal_communication.gosdk/compozy/loader.gosdk/examples/main.gosdk/compozy/engine_loading.goengine/tool/inline/manager.gosdk/examples/04_memory_conversation.goengine/tool/uc/store_decode.goengine/llm/tool_registry.gosdk/compozy/validation.gosdk/examples/08_model_routing.goengine/tool/router/dto.goengine/memory/config.gosdk/client/execution_helpers.gosdk/compozy/engine_registration.gosdk/client/workflow.godocs/docs.gosdk/client/task.gomagefile.goscripts/markdown/check.gosdk/compozy/lifecycle.go
engine/{agent,task,task2,tool,workflow,runtime,llm,memory,memorycfg,knowledge,model,mcp,webhook,attachment,resources,auth,autoload,project,schema,worker}/**/*.go
📄 CodeRabbit inference engine (.cursor/rules/architecture.mdc)
engine/{agent,task,task2,tool,workflow,runtime,llm,memory,memorycfg,knowledge,model,mcp,webhook,attachment,resources,auth,autoload,project,schema,worker}/**/*.go: Application Layer (engine/): implement domain-specific business logic and ports; organize by domain with uc/ and router/ subpackages
Port interfaces (e.g., repositories, external services) are defined in Application Layer packages where they are used
Application layer depends inward: Application → Domain; avoid depending on infrastructure
Files:
engine/tool/uc/store_errors.goengine/worker/embedded/server.goengine/resources/store.goengine/tool/router/tools_top.goengine/project/indexer.goengine/project/schedule/config.goengine/tool/nativeuser/registry_test.goengine/llm/service.goengine/tool/config.goengine/tool/nativeuser/registry.goengine/agent/action_config.goengine/tool/inline/manager_test.goengine/tool/inline/manager.goengine/tool/uc/store_decode.goengine/llm/tool_registry.goengine/tool/router/dto.goengine/memory/config.goengine/tool/uc/store_test.goengine/llm/tool_registry_test.go
engine/*/uc/**/*.go
📄 CodeRabbit inference engine (.cursor/rules/architecture.mdc)
Use cases (uc) contain application logic only; coordinate validation, business rules, and persistence via ports
Files:
engine/tool/uc/store_errors.goengine/tool/uc/store_decode.goengine/tool/uc/store_test.go
{engine,pkg,cli}/**/*.go
📄 CodeRabbit inference engine (.cursor/rules/architecture.mdc)
Context as the first parameter for I/O or long-running operations; always propagate and handle cancellation
Files:
engine/tool/uc/store_errors.goengine/worker/embedded/server.goengine/resources/store.goengine/tool/router/tools_top.goengine/project/indexer.goengine/project/schedule/config.goengine/tool/nativeuser/registry_test.goengine/llm/service.goengine/tool/config.goengine/tool/nativeuser/registry.goengine/agent/action_config.goengine/tool/inline/manager_test.goengine/tool/inline/manager.goengine/tool/uc/store_decode.goengine/llm/tool_registry.goengine/tool/router/dto.goengine/memory/config.goengine/tool/uc/store_test.goengine/llm/tool_registry_test.go
**/*_test.go
📄 CodeRabbit inference engine (.cursor/rules/logger-config.mdc)
**/*_test.go: In tests, set up configuration with config.Initialize or config.NewManager(...).Load(...) and attach via config.ContextWithManager
In tests, prefer logger.NewForTests() and attach with logger.ContextWithLogger
In tests, mock or stub external tools/services; do not introduce DI of logger/config into code under test
**/*_test.go: Prefer local test constants (e.g., const testTimeout = 5 * time.Second) in tests
Inline trivial literals in tests when intent is clear
Do not duplicate production constants in tests; import them or assert relative behavior
**/*_test.go: All tests pass and follow the established testing patterns
Code is well-tested with both unit and integration tests
Tests should follow the t.Run("Should...") subtest naming pattern
Ensure adequate test coverage
**/*_test.go: All Go tests must use t.Run("Should describe behavior", ...) subtests for each behavior
Use stretchr/testify for assertions and mocks in tests
Do not use testify suite patterns (no suite.Suite embedding or suite-based structures)
Do not use suite methods like s.Equal(), s.NoError(), or s.T()
Avoid weak assertions like assert.Error(t, err); use specific error validation instead
Prefer specific error assertions like assert.ErrorContains and assert.ErrorAs for validating errors
Unit tests should be placed alongside implementation files as *_test.go
Use testify/mock for mocking external dependencies or complex interfacesIn tests, never use context.Background(); use t.Context() instead
Aim for 80%+ test coverage on business logic; write focused, isolated tests
**/*_test.go: Use stretchr/testify for assertions and mocks; import as github.com/stretchr/testify/assert and github.com/stretchr/testify/mock; replace custom mocks with testify/mock
Use project test helpers (utils.SetupTest, utils.SetupFixture, etc.) in testsIn tests, never use context.Background(); use t.Context()
Files:
sdk/compozy/resources_graph_test.gosdk/agent/constructor_test.gosdk/compozy/integration/distributed_integration_test.gosdk/compozy/loader_test.gosdk/compozy/loading_test.gosdk/compozy/app_test.goengine/tool/nativeuser/registry_test.gosdk/compozy/validation_test.gosdk/compozy/loader_performance_test.gosdk/compozy/registration_errors_test.gosdk/compozy/integration/standalone_integration_test.gosdk/compozy/options_test.gosdk/compozy/config/yaml_loader_test.gosdk/compozy/validation_nodes_test.goengine/tool/inline/manager_test.gosdk/compozy/engine_loading_errors_test.gosdk/compozy/migration/example_compat_test.gosdk/compozy/mode_test.gosdk/compozy/registration_test.gosdk/compozy/cleanup_test.gosdk/compozy/constructor_clone_test.gosdk/agentaction/constructor_test.goengine/tool/uc/store_test.gosdk/compozy/codegen/generator_test.gosdk/compozy/lifecycle_test.goengine/llm/tool_registry_test.gosdk/compozy/integration/hybrid_yaml_integration_test.go
engine/*/router/**/*.go
📄 CodeRabbit inference engine (.cursor/rules/architecture.mdc)
Routers contain HTTP handling only (transport mapping, DTOs); no business logic
Files:
engine/tool/router/tools_top.goengine/tool/router/dto.go
engine/*/service.go
📄 CodeRabbit inference engine (.cursor/rules/architecture.mdc)
Service construction must use dependency injection via constructors with nil-safe configuration handling; use factories for complex creation
Files:
engine/llm/service.go
engine/*/config.go
📄 CodeRabbit inference engine (.cursor/rules/architecture.mdc)
Provide domain-specific configuration with sensible defaults; never rely on global singletons
Files:
engine/tool/config.goengine/memory/config.go
engine/*/router/dto.go
📄 CodeRabbit inference engine (.cursor/rules/architecture.mdc)
Define transport DTOs in router/dto.go; keep them separate from domain entities
Files:
engine/tool/router/dto.go
docs/**/*.go
📄 CodeRabbit inference engine (docs/.cursor/rules/review-checklist.mdc)
docs/**/*.go: Code must be formatted withgo fmt
All linter warnings must be addressed
Error handling must follow project patterns
Security considerations must be addressed (e.g., no exposed secrets or unvalidated inputs)
Errors must be handled appropriately and with context
Interfaces must be used appropriately to define behavior
Code must pass all linter checks
Code must be secure and performant
Dependencies must be injected properly
Context must be propagated correctly
Hardcoded values that should be configurable must be avoided
Missing context propagation in functions that make external calls must be addressed
Performance issues like unnecessary allocations or inefficient algorithms must be avoided
docs/**/*.go: Use section comments with dashes for visual separation in Go code, formatted as:
// -----------------------------------------------------------------------------
// Section Name
// -----------------------------------------------------------------------------
Files:
docs/docs.go
🧠 Learnings (1)
📚 Learning: 2025-10-20T04:54:19.496Z
Learnt from: CR
Repo: compozy/compozy PR: 0
File: .cursor/rules/critical-validation.mdc:0-0
Timestamp: 2025-10-20T04:54:19.496Z
Learning: Applies to **/*_test.go : In tests, never use context.Background(); use t.Context() instead
Applied to files:
sdk/agent/constructor_test.go
🧬 Code graph analysis (60)
engine/tool/uc/store_errors.go (1)
sdk/tool/constructor.go (1)
New(26-73)
sdk/agent/constructor_test.go (6)
sdk/agent/constructor.go (1)
New(16-63)engine/core/config.go (1)
ConfigAgent(35-35)sdk/internal/errors/build_error.go (1)
BuildError(11-13)engine/core/memory.go (1)
MemoryReference(19-28)engine/agent/action_config.go (1)
ActionConfig(65-116)engine/core/provider.go (1)
ProviderOpenAI(15-15)
engine/tool/router/tools_top.go (1)
engine/tool/uc/store_errors.go (2)
ErrIDMissing(8-8)ErrNativeImplementation(14-14)
sdk/examples/03_knowledge_rag.go (4)
engine/knowledge/config.go (6)
EmbedderConfig(202-208)VectorDBTypeFilesystem(191-191)IngestOnStart(174-174)ChunkingConfig(307-311)ChunkStrategyRecursiveTextSplitter(181-181)SourceConfig(294-304)sdk/client/client.go (1)
WithAPIKey(33-37)sdk/knowledge/vectordb_options.go (2)
WithPath(25-29)WithVectorDBDimension(39-43)engine/project/config.go (1)
WorkflowSourceConfig(168-182)
sdk/compozy/integration/distributed_integration_test.go (2)
pkg/config/resolver.go (1)
ModeDistributed(8-8)sdk/compozy/constructor.go (1)
New(21-60)
sdk/compozy/loader_test.go (1)
sdk/compozy/engine.go (1)
Engine(29-76)
sdk/compozy/loading_test.go (4)
engine/core/provider.go (1)
ProviderName(12-12)pkg/mcp-proxy/types.go (1)
TransportStdio(23-23)sdk/internal/validate/validate.go (1)
Cron(164-176)engine/webhook/config.go (1)
EventConfig(19-24)
sdk/examples/09_debugging_tracing.go (1)
engine/core/provider.go (1)
ProviderMock(24-24)
sdk/examples/01_simple_workflow.go (3)
sdk/compozy/engine.go (1)
Engine(29-76)engine/core/params.go (2)
Output(5-5)OutputRootKey(12-12)engine/core/provider.go (1)
ProviderMock(24-24)
engine/project/schedule/config.go (3)
sdk/internal/validate/validate.go (3)
ID(85-97)Cron(164-176)Duration(136-144)engine/workflow/schedule/config_definition.go (2)
RetryPolicy(10-10)Config(7-7)engine/project/config.go (1)
Config(276-495)
sdk/compozy/app_test.go (2)
sdk/compozy/constructor.go (1)
New(21-60)pkg/config/resolver.go (2)
ModeStandalone(7-7)ModeDistributed(8-8)
engine/tool/nativeuser/registry_test.go (1)
engine/tool/nativeuser/registry.go (8)
Reset(78-83)Register(33-46)Lookup(49-63)Handler(14-14)ErrInvalidID(25-25)ErrNilHandler(27-27)ErrAlreadyRegistered(29-29)IDs(66-75)
sdk/compozy/constructor.go (2)
sdk/compozy/engine.go (1)
Engine(29-76)engine/core/copy.go (1)
DeepCopy(78-92)
sdk/compozy/validation_test.go (6)
sdk/compozy/engine.go (1)
Engine(29-76)engine/memory/config.go (1)
Config(121-231)engine/project/config.go (1)
Config(276-495)sdk/internal/validate/validate.go (1)
ID(85-97)engine/core/global.go (1)
SuccessTransition(16-23)engine/core/provider.go (1)
ProviderName(12-12)
sdk/examples/02_parallel_tasks.go (3)
engine/core/provider.go (1)
ProviderMock(24-24)engine/task/config.go (1)
StrategyWaitAll(906-906)engine/core/global.go (1)
SuccessTransition(16-23)
engine/llm/service.go (2)
pkg/logger/mod.go (1)
FromContext(48-56)engine/llm/tool_registry.go (2)
NewNativeToolAdapter(595-597)NewLocalToolAdapter(539-544)
sdk/examples/06_scheduled_workflow.go (3)
sdk/internal/validate/validate.go (1)
Cron(164-176)sdk/compozy/engine.go (1)
Engine(29-76)engine/core/provider.go (1)
ProviderMock(24-24)
sdk/client/http.go (1)
engine/infra/server/router/errors.go (1)
ErrorInfo(131-135)
sdk/agent/constructor.go (5)
engine/core/config.go (1)
ConfigAgent(35-35)sdk/internal/validate/validate.go (1)
NonEmpty(100-111)sdk/internal/errors/build_error.go (1)
BuildError(11-13)engine/core/copy.go (1)
DeepCopy(78-92)engine/core/provider.go (1)
ProviderName(12-12)
sdk/compozy/loader_performance_test.go (3)
pkg/logger/mod.go (2)
ContextWithLogger(39-41)NewForTests(199-201)pkg/config/provider.go (1)
NewDefaultProvider(233-237)pkg/config/context.go (1)
ContextWithManager(19-21)
sdk/compozy/registration_errors_test.go (2)
engine/resources/store.go (6)
ResourceKey(40-45)ETag(60-60)ResourceMeta(35-35)ResourceType(15-15)StoredItem(71-75)ResourceWorkflow(19-19)sdk/compozy/engine.go (1)
Engine(29-76)
sdk/compozy/engine.go (3)
engine/resources/store.go (1)
ResourceStore(82-123)engine/worker/embedded/server.go (1)
Server(30-38)engine/tool/inline/manager.go (1)
Manager(37-52)
sdk/examples/05_runtime_native_tools.go (3)
sdk/tool/options_generated.go (6)
Option(11-11)WithName(41-45)WithDescription(54-58)WithRuntime(65-69)WithCode(88-92)WithWith(143-147)engine/tool/config.go (1)
Config(104-208)sdk/tool/constructor.go (2)
New(26-73)WithNativeHandler(213-219)
engine/tool/config.go (4)
engine/schema/schema.go (1)
Schema(19-19)engine/core/params.go (1)
Input(4-4)engine/project/config.go (1)
Config(276-495)engine/core/env.go (1)
EnvMap(12-12)
engine/tool/nativeuser/registry.go (1)
sdk/tool/constructor.go (1)
New(26-73)
sdk/examples/00_shared.go (4)
engine/core/copy.go (1)
CopyMaps(39-47)engine/core/params.go (2)
NewInput(18-23)Output(5-5)engine/core/provider.go (1)
ProviderOpenAI(15-15)engine/knowledge/config.go (2)
SourceConfig(294-304)SourceTypeMarkdownGlob(166-166)
sdk/compozy/integration/standalone_integration_test.go (1)
sdk/compozy/constructor.go (1)
New(21-60)
sdk/compozy/options_test.go (2)
pkg/config/resolver.go (1)
ModeDistributed(8-8)engine/core/provider.go (1)
ProviderName(12-12)
sdk/examples/10_complete_project.go (7)
engine/memory/core/types.go (2)
PersistenceConfig(425-434)InMemoryPersistence(421-421)sdk/tool/options_generated.go (2)
WithName(41-45)WithDescription(54-58)sdk/tool/constructor.go (1)
WithNativeHandler(213-219)sdk/internal/validate/validate.go (1)
Cron(164-176)engine/core/memory.go (1)
MemoryReference(19-28)engine/knowledge/config.go (1)
EmbedderConfig(202-208)engine/project/config.go (1)
WorkflowSourceConfig(168-182)
engine/agent/action_config.go (3)
engine/tool/config.go (1)
Config(104-208)engine/core/global.go (3)
SuccessTransition(16-23)ErrorTransition(49-57)RetryPolicyConfig(84-104)engine/core/time.go (1)
ParseHumanDuration(17-28)
sdk/compozy/config/yaml_loader_test.go (5)
pkg/logger/mod.go (2)
ContextWithLogger(39-41)NewForTests(199-201)pkg/config/provider.go (1)
NewDefaultProvider(233-237)pkg/config/context.go (1)
ContextWithManager(19-21)engine/core/global.go (1)
SuccessTransition(16-23)sdk/compozy/constructor.go (1)
New(21-60)
sdk/compozy/validation_nodes_test.go (3)
sdk/compozy/constructor.go (1)
New(21-60)engine/core/global.go (1)
SuccessTransition(16-23)sdk/compozy/engine.go (1)
Engine(29-76)
engine/tool/inline/manager_test.go (4)
sdk/internal/testutil/context.go (1)
NewTestContext(12-18)engine/tool/inline/manager.go (2)
NewManager(66-103)Options(29-35)engine/tool/config.go (2)
Config(104-208)ImplementationRuntime(23-23)engine/resources/store.go (2)
ResourceKey(40-45)ResourceTool(22-22)
sdk/examples/07_signal_communication.go (2)
engine/task/config.go (3)
TaskTypeSignal(392-392)SignalConfig(1801-1811)TaskTypeWait(401-401)engine/core/global.go (1)
SuccessTransition(16-23)
sdk/compozy/loader.go (2)
sdk/compozy/engine.go (1)
Engine(29-76)engine/memory/config.go (1)
Config(121-231)
sdk/examples/main.go (5)
sdk/examples/01_simple_workflow.go (1)
RunSimpleWorkflow(19-42)pkg/config/provider.go (2)
NewDefaultProvider(233-237)NewEnvProvider(21-23)pkg/config/context.go (1)
ContextWithManager(19-21)pkg/logger/mod.go (4)
InfoLevel(22-22)DisabledLevel(27-27)DebugLevel(21-21)ContextWithLogger(39-41)pkg/logger/setup.go (1)
SetupLogger(9-18)
sdk/compozy/engine_loading_errors_test.go (1)
sdk/compozy/engine.go (1)
Engine(29-76)
sdk/compozy/migration/example_compat_test.go (7)
engine/core/provider.go (1)
ProviderName(12-12)sdk/compozy/constructor.go (1)
New(21-60)sdk/tool/options_generated.go (1)
WithWith(143-147)engine/core/params.go (1)
Output(5-5)pkg/logger/mod.go (2)
ContextWithLogger(39-41)NewForTests(199-201)pkg/config/provider.go (1)
NewDefaultProvider(233-237)pkg/config/context.go (1)
ContextWithManager(19-21)
sdk/compozy/mode_test.go (2)
sdk/compozy/constructor.go (1)
New(21-60)sdk/compozy/engine.go (1)
Engine(29-76)
sdk/compozy/engine_loading.go (1)
sdk/compozy/engine.go (1)
Engine(29-76)
engine/tool/inline/manager.go (3)
engine/resources/store.go (2)
ResourceStore(82-123)ResourceTool(22-22)engine/core/types.go (1)
GetStoreDir(15-20)engine/tool/config.go (2)
Config(104-208)ImplementationRuntime(23-23)
sdk/compozy/registration_test.go (5)
pkg/mcp-proxy/types.go (1)
TransportStdio(23-23)engine/resources/store.go (4)
ResourceKey(40-45)ResourceSchedule(31-31)ResourceWebhook(32-32)ResourceStore(82-123)engine/webhook/config.go (1)
EventConfig(19-24)sdk/compozy/engine.go (1)
Engine(29-76)sdk/compozy/constructor.go (1)
New(21-60)
sdk/examples/04_memory_conversation.go (3)
engine/memory/core/types.go (2)
PersistenceConfig(425-434)InMemoryPersistence(421-421)engine/core/provider.go (1)
ProviderMock(24-24)engine/core/memory.go (1)
MemoryReference(19-28)
engine/tool/uc/store_decode.go (2)
engine/tool/config.go (1)
ImplementationNative(24-24)engine/tool/uc/store_errors.go (1)
ErrNativeImplementation(14-14)
engine/llm/tool_registry.go (5)
engine/tool/config.go (1)
Config(104-208)pkg/logger/mod.go (2)
Config(129-135)FromContext(48-56)engine/core/copy.go (2)
DeepCopy(78-92)CloneMap(29-34)engine/tool/nativeuser/registry.go (2)
Lookup(49-63)Handler(14-14)engine/core/params.go (2)
NewInput(18-23)Output(5-5)
sdk/compozy/validation.go (3)
engine/project/config.go (1)
Config(276-495)engine/schema/schema.go (1)
GetID(171-181)sdk/compozy/engine.go (1)
Engine(29-76)
sdk/examples/08_model_routing.go (1)
engine/core/provider.go (1)
ProviderMock(24-24)
sdk/compozy/cleanup_test.go (2)
engine/resources/store.go (3)
ResourceStore(82-123)ResourceKey(40-45)ETag(60-60)sdk/compozy/engine.go (1)
Engine(29-76)
engine/tool/router/dto.go (2)
engine/tool/config.go (1)
Config(104-208)engine/core/env.go (1)
EnvMap(12-12)
sdk/compozy/constructor_clone_test.go (1)
engine/core/provider.go (1)
ProviderName(12-12)
engine/memory/config.go (3)
engine/memory/core/types.go (2)
PrivacyScope(47-47)PrivacyGlobalScope(51-51)engine/memory/types.go (2)
PrivacyScope(6-6)PrivacyGlobalScope(10-10)engine/core/time.go (1)
ParseHumanDuration(17-28)
sdk/agentaction/constructor_test.go (4)
sdk/agent/constructor_test.go (1)
TestNew(15-165)sdk/agent/constructor.go (1)
New(16-63)sdk/internal/errors/build_error.go (1)
BuildError(11-13)engine/core/global.go (3)
SuccessTransition(16-23)ErrorTransition(49-57)RetryPolicyConfig(84-104)
engine/tool/uc/store_test.go (1)
engine/tool/uc/store_errors.go (1)
ErrNativeImplementation(14-14)
sdk/compozy/engine_registration.go (3)
sdk/compozy/engine.go (1)
Engine(29-76)engine/resources/store.go (15)
ResourceProject(18-18)ResourceWorkflow(19-19)ResourceAgent(21-21)ResourceTool(22-22)ResourceKnowledgeBase(25-25)ResourceMemory(24-24)ResourceMCP(23-23)ResourceSchema(29-29)ResourceModel(30-30)ResourceSchedule(31-31)ResourceWebhook(32-32)ResourceStore(82-123)ResourceType(15-15)ResourceKey(40-45)ErrNotFound(127-127)engine/schema/schema.go (1)
GetID(171-181)
sdk/client/workflow.go (2)
sdk/client/types.go (2)
WorkflowExecuteRequest(12-12)WorkflowExecuteResponse(15-15)engine/infra/server/routes/routes.go (2)
Workflows(40-42)Executions(35-37)
sdk/client/task.go (2)
sdk/client/types.go (3)
TaskExecuteRequest(24-24)TaskExecuteResponse(27-27)TaskSyncResponse(30-30)engine/infra/server/routes/routes.go (2)
Tasks(54-54)Executions(35-37)
sdk/compozy/lifecycle_test.go (6)
engine/resources/store.go (3)
ResourceType(15-15)ResourceStore(82-123)ResourceKey(40-45)pkg/logger/mod.go (2)
ContextWithLogger(39-41)NewForTests(199-201)sdk/compozy/engine.go (1)
Engine(29-76)engine/tool/inline/manager.go (2)
Manager(37-52)NewManager(66-103)pkg/config/provider.go (1)
NewDefaultProvider(233-237)pkg/config/context.go (1)
ContextWithManager(19-21)
engine/llm/tool_registry_test.go (3)
engine/tool/config.go (2)
Config(104-208)ImplementationNative(24-24)engine/tool/nativeuser/registry.go (2)
Reset(78-83)Register(33-46)engine/llm/tool_registry.go (1)
NewNativeToolAdapter(595-597)
sdk/compozy/integration/hybrid_yaml_integration_test.go (1)
engine/core/global.go (1)
SuccessTransition(16-23)
sdk/compozy/lifecycle.go (3)
sdk/compozy/engine.go (1)
Engine(29-76)engine/resources/store.go (1)
ResourceStore(82-123)engine/tool/inline/manager.go (3)
Manager(37-52)NewManager(66-103)Options(29-35)
⏰ 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). (7)
- GitHub Check: test
- GitHub Check: Container Security Scan
- GitHub Check: SDK Integration Tests
- GitHub Check: Security Scan (gosec)
- GitHub Check: Analyze (go)
- GitHub Check: SDK Unit Tests
- GitHub Check: SDK Benchmarks
There was a problem hiding this comment.
Actionable comments posted: 14
♻️ Duplicate comments (2)
engine/worker/embedded/server.go (1)
245-253: LGTM! Timeout now correctly treated as an error.The implementation properly addresses the past review feedback by returning a descriptive error when a timeout occurs during port availability checks. This fail-fast approach provides better diagnostics and prevents the server from attempting to bind to potentially conflicting ports.
The error message includes all relevant context (port, bindIP) and wraps the original error, following the project's error handling patterns.
sdk/compozy/registration_errors_test.go (1)
206-216: Wrap this test in a Should-style subtestThis package mandates
t.Run("Should ...")subtests for every behavior. Please wrap the body in at.Runblock so the test complies. As per coding guidelines.func TestRegisterProjectResetsStateOnPersistFailure(t *testing.T) { - store := newResourceStoreStub() - store.putErr = errors.New("persist failure") - engine := &Engine{ctx: t.Context(), resourceStore: store} - cfg := &engineproject.Config{Name: "helios"} - err := engine.registerProject(cfg, registrationSourceProgrammatic) - require.Error(t, err) - engine.stateMu.RLock() - defer engine.stateMu.RUnlock() - assert.Nil(t, engine.project) + t.Run("Should reset project state on persist failure", func(t *testing.T) { + store := newResourceStoreStub() + store.putErr = errors.New("persist failure") + engine := &Engine{ctx: t.Context(), resourceStore: store} + cfg := &engineproject.Config{Name: "helios"} + err := engine.registerProject(cfg, registrationSourceProgrammatic) + require.Error(t, err) + engine.stateMu.RLock() + defer engine.stateMu.RUnlock() + assert.Nil(t, engine.project) + }) }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (19)
ai-docs/reviews-pr-316/issues/001-issue.mdis excluded by!**/*.mdai-docs/reviews-pr-316/issues/002-issue.mdis excluded by!**/*.mdai-docs/reviews-pr-316/issues/003-issue.mdis excluded by!**/*.mdai-docs/reviews-pr-316/issues/004-issue.mdis excluded by!**/*.mdai-docs/reviews-pr-316/issues/005-issue.mdis excluded by!**/*.mdai-docs/reviews-pr-316/issues/006-issue.mdis excluded by!**/*.mdai-docs/reviews-pr-316/issues/007-issue.mdis excluded by!**/*.mdai-docs/reviews-pr-316/issues/011-issue.mdis excluded by!**/*.mdai-docs/reviews-pr-316/issues/012-issue.mdis excluded by!**/*.mdai-docs/reviews-pr-316/issues/013-issue.mdis excluded by!**/*.mdai-docs/reviews-pr-316/issues/014-issue.mdis excluded by!**/*.mdai-docs/reviews-pr-316/issues/015-issue.mdis excluded by!**/*.mdai-docs/reviews-pr-316/issues/016-issue.mdis excluded by!**/*.mdai-docs/reviews-pr-316/issues/017-issue.mdis excluded by!**/*.mdai-docs/reviews-pr-316/issues/018-issue.mdis excluded by!**/*.mdai-docs/reviews-pr-316/issues/019-issue.mdis excluded by!**/*.mdai-docs/reviews-pr-316/issues/020-issue.mdis excluded by!**/*.mdai-docs/reviews-pr-316/issues/021-issue.mdis excluded by!**/*.mdgo.work.sumis excluded by!**/*.sum
📒 Files selected for processing (22)
engine/llm/tool_registry_test.go(2 hunks)engine/project/indexer.go(8 hunks)engine/project/schedule/config.go(1 hunks)engine/tool/inline/manager.go(1 hunks)engine/tool/nativeuser/registry_test.go(1 hunks)engine/worker/embedded/server.go(1 hunks)magefile.go(1 hunks)sdk/client/client_test.go(1 hunks)sdk/client/workflow.go(1 hunks)sdk/compozy/codegen/generator_test.go(1 hunks)sdk/compozy/config/yaml_loader_test.go(1 hunks)sdk/compozy/constructor_clone_test.go(1 hunks)sdk/compozy/engine_registration.go(1 hunks)sdk/compozy/execution_client_test.go(1 hunks)sdk/compozy/integration/distributed_integration_test.go(1 hunks)sdk/compozy/integration/hybrid_yaml_integration_test.go(1 hunks)sdk/compozy/lifecycle.go(1 hunks)sdk/compozy/lifecycle_helpers_test.go(1 hunks)sdk/compozy/loader_test.go(1 hunks)sdk/compozy/mode_test.go(1 hunks)sdk/compozy/registration_errors_test.go(1 hunks)sdk/examples/10_complete_project.go(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.go
📄 CodeRabbit inference engine (.cursor/rules/api-standards.mdc)
**/*.go: Return properly structured error responses
Use middleware for cross-cutting concerns (e.g., logging, auth, rate limiting)
Implement proper authentication and authorization in the API
Apply rate limiting and request validation
Version API routes under the prefix /api/v0/
Use gin-gonic/gin for HTTP APIs
Ensure consistent response formats across all endpoints
Use appropriate HTTP status codes (200, 201, 400, 401, 403, 404, 500)
Return JSON responses with a consistent error structure
Update Swagger annotations for all API changes
Generate Swagger docs served at /swagger/index.html using swaggo/swag
Include request and response examples in Swagger annotations
Document all parameters, headers, and error responses in Swagger
Success response body should contain fields: data and message ("Success")
Error response body should contain fields: error and details
**/*.go: Retrieve the logger via logger.FromContext(ctx) inside runtime code (handlers, services, workers)
Retrieve configuration via config.FromContext(ctx) inside runtime code (handlers, services, workers)
Attach *config.Manager at process edges using config.ContextWithManager(ctx, mgr)
Build the logger with logger.SetupLogger(...) at startup and attach with logger.ContextWithLogger(ctx, log)
For HTTP servers, ensure request contexts inherit BOTH manager and logger via middleware
Do not pass loggers via function parameters or dependency injection; always use context-backed retrieval
Do not use a global configuration singleton; read config from context
CLI/root flow: parse flags → construct sources → mgr.Load(...) → attach manager to ctx → setup logger honoring cfg.CLI.Debug/Quiet → attach logger to ctx → propagate ctx
Server startup should use the CLI-provided ctx and add middleware that sets c.Request = c.Request.WithContext(logger.ContextWithLogger(config.ContextWithManager(c.Request.Context(), mgr), log))
Optionally set http.Server.BaseContext to a parent context carrying manager and logger so all req...
Files:
sdk/compozy/integration/distributed_integration_test.gosdk/compozy/config/yaml_loader_test.goengine/tool/nativeuser/registry_test.gosdk/compozy/codegen/generator_test.gosdk/compozy/loader_test.gosdk/compozy/registration_errors_test.goengine/llm/tool_registry_test.goengine/worker/embedded/server.gosdk/compozy/mode_test.gosdk/compozy/integration/hybrid_yaml_integration_test.goengine/project/schedule/config.gosdk/client/workflow.goengine/tool/inline/manager.gosdk/compozy/execution_client_test.gosdk/compozy/lifecycle_helpers_test.gosdk/compozy/constructor_clone_test.gosdk/examples/10_complete_project.gosdk/client/client_test.gomagefile.goengine/project/indexer.gosdk/compozy/engine_registration.gosdk/compozy/lifecycle.go
**/*_test.go
📄 CodeRabbit inference engine (.cursor/rules/logger-config.mdc)
**/*_test.go: In tests, set up configuration with config.Initialize or config.NewManager(...).Load(...) and attach via config.ContextWithManager
In tests, prefer logger.NewForTests() and attach with logger.ContextWithLogger
In tests, mock or stub external tools/services; do not introduce DI of logger/config into code under test
**/*_test.go: Prefer local test constants (e.g., const testTimeout = 5 * time.Second) in tests
Inline trivial literals in tests when intent is clear
Do not duplicate production constants in tests; import them or assert relative behavior
**/*_test.go: All tests pass and follow the established testing patterns
Code is well-tested with both unit and integration tests
Tests should follow the t.Run("Should...") subtest naming pattern
Ensure adequate test coverage
**/*_test.go: All Go tests must use t.Run("Should describe behavior", ...) subtests for each behavior
Use stretchr/testify for assertions and mocks in tests
Do not use testify suite patterns (no suite.Suite embedding or suite-based structures)
Do not use suite methods like s.Equal(), s.NoError(), or s.T()
Avoid weak assertions like assert.Error(t, err); use specific error validation instead
Prefer specific error assertions like assert.ErrorContains and assert.ErrorAs for validating errors
Unit tests should be placed alongside implementation files as *_test.go
Use testify/mock for mocking external dependencies or complex interfacesIn tests, never use context.Background(); use t.Context() instead
Aim for 80%+ test coverage on business logic; write focused, isolated tests
**/*_test.go: Use stretchr/testify for assertions and mocks; import as github.com/stretchr/testify/assert and github.com/stretchr/testify/mock; replace custom mocks with testify/mock
Use project test helpers (utils.SetupTest, utils.SetupFixture, etc.) in testsIn tests, never use context.Background(); use t.Context()
Files:
sdk/compozy/integration/distributed_integration_test.gosdk/compozy/config/yaml_loader_test.goengine/tool/nativeuser/registry_test.gosdk/compozy/codegen/generator_test.gosdk/compozy/loader_test.gosdk/compozy/registration_errors_test.goengine/llm/tool_registry_test.gosdk/compozy/mode_test.gosdk/compozy/integration/hybrid_yaml_integration_test.gosdk/compozy/execution_client_test.gosdk/compozy/lifecycle_helpers_test.gosdk/compozy/constructor_clone_test.gosdk/client/client_test.go
engine/{agent,task,task2,tool,workflow,runtime,llm,memory,memorycfg,knowledge,model,mcp,webhook,attachment,resources,auth,autoload,project,schema,worker}/**/*.go
📄 CodeRabbit inference engine (.cursor/rules/architecture.mdc)
engine/{agent,task,task2,tool,workflow,runtime,llm,memory,memorycfg,knowledge,model,mcp,webhook,attachment,resources,auth,autoload,project,schema,worker}/**/*.go: Application Layer (engine/): implement domain-specific business logic and ports; organize by domain with uc/ and router/ subpackages
Port interfaces (e.g., repositories, external services) are defined in Application Layer packages where they are used
Application layer depends inward: Application → Domain; avoid depending on infrastructure
Files:
engine/tool/nativeuser/registry_test.goengine/llm/tool_registry_test.goengine/worker/embedded/server.goengine/project/schedule/config.goengine/tool/inline/manager.goengine/project/indexer.go
{engine,pkg,cli}/**/*.go
📄 CodeRabbit inference engine (.cursor/rules/architecture.mdc)
Context as the first parameter for I/O or long-running operations; always propagate and handle cancellation
Files:
engine/tool/nativeuser/registry_test.goengine/llm/tool_registry_test.goengine/worker/embedded/server.goengine/project/schedule/config.goengine/tool/inline/manager.goengine/project/indexer.go
**/!(*_test).go
📄 CodeRabbit inference engine (.cursor/rules/magic-numbers.mdc)
**/!(*_test).go: NEVER introduce magic numbers in runtime Go code; replace unexplained numeric literals with named constants or configuration
Operator-tunable values MUST be sourced from configuration as defined in @.cursor/rules/global-config.mdc
Fetch configuration via config.FromContext(ctx); do not use global singletons
Promote tunable values (timeouts, deadlines, polling intervals, retry/backoff counts and factors, concurrency limits, queue sizes, buffer/payload limits, behavior thresholds) to configurationIn runtime code, properly inherit context; never use context.Background() in code paths
In runtime code paths, inherit context properly; never use context.Background()
In runtime code paths, never use context.Background(); properly inherit context
In runtime code, properly inherit context; never use context.Background()
Files:
engine/worker/embedded/server.goengine/project/schedule/config.gosdk/client/workflow.goengine/tool/inline/manager.gosdk/examples/10_complete_project.gomagefile.goengine/project/indexer.gosdk/compozy/engine_registration.gosdk/compozy/lifecycle.go
🧬 Code graph analysis (19)
sdk/compozy/integration/distributed_integration_test.go (3)
pkg/config/resolver.go (1)
ModeDistributed(8-8)sdk/compozy/constructor.go (1)
New(21-60)engine/resources/store.go (1)
ResourceStore(82-123)
sdk/compozy/config/yaml_loader_test.go (5)
pkg/logger/mod.go (2)
ContextWithLogger(39-41)NewForTests(199-201)pkg/config/provider.go (1)
NewDefaultProvider(233-237)pkg/config/context.go (1)
ContextWithManager(19-21)engine/core/global.go (1)
SuccessTransition(16-23)sdk/compozy/constructor.go (1)
New(21-60)
engine/tool/nativeuser/registry_test.go (1)
engine/tool/nativeuser/registry.go (8)
Reset(78-83)Register(33-46)Lookup(49-63)Handler(14-14)ErrInvalidID(25-25)ErrNilHandler(27-27)ErrAlreadyRegistered(29-29)IDs(66-75)
sdk/compozy/loader_test.go (1)
sdk/compozy/engine.go (1)
Engine(29-76)
sdk/compozy/registration_errors_test.go (2)
engine/resources/store.go (6)
ResourceKey(40-45)ETag(60-60)ResourceMeta(35-35)ResourceType(15-15)StoredItem(71-75)ResourceWorkflow(19-19)sdk/compozy/engine.go (1)
Engine(29-76)
engine/llm/tool_registry_test.go (3)
engine/tool/config.go (2)
Config(104-208)ImplementationNative(24-24)engine/tool/nativeuser/registry.go (2)
Reset(78-83)Register(33-46)engine/llm/tool_registry.go (1)
NewNativeToolAdapter(595-597)
sdk/compozy/mode_test.go (2)
sdk/compozy/constructor.go (1)
New(21-60)sdk/compozy/engine.go (1)
Engine(29-76)
sdk/compozy/integration/hybrid_yaml_integration_test.go (1)
engine/core/global.go (1)
SuccessTransition(16-23)
engine/project/schedule/config.go (3)
sdk/internal/validate/validate.go (3)
ID(85-97)Cron(164-176)Duration(136-144)engine/workflow/schedule/config_definition.go (2)
RetryPolicy(10-10)Config(7-7)engine/project/config.go (1)
Config(276-495)
sdk/client/workflow.go (2)
sdk/client/types.go (2)
WorkflowExecuteRequest(12-12)WorkflowExecuteResponse(15-15)engine/infra/server/routes/routes.go (2)
Workflows(40-42)Executions(35-37)
engine/tool/inline/manager.go (3)
engine/resources/store.go (2)
ResourceStore(82-123)ResourceTool(22-22)engine/core/types.go (1)
GetStoreDir(15-20)engine/tool/config.go (2)
Config(104-208)ImplementationRuntime(23-23)
sdk/compozy/execution_client_test.go (3)
sdk/compozy/engine.go (1)
Engine(29-76)sdk/client/client.go (1)
WithHTTPClient(40-46)pkg/logger/mod.go (2)
ContextWithLogger(39-41)NewForTests(199-201)
sdk/compozy/lifecycle_helpers_test.go (2)
sdk/compozy/engine.go (1)
Engine(29-76)pkg/logger/mod.go (2)
ContextWithLogger(39-41)NewForTests(199-201)
sdk/compozy/constructor_clone_test.go (1)
engine/core/provider.go (1)
ProviderName(12-12)
sdk/examples/10_complete_project.go (6)
engine/knowledge/config.go (1)
EmbedderConfig(202-208)engine/memory/core/types.go (2)
PersistenceConfig(425-434)InMemoryPersistence(421-421)sdk/tool/options_generated.go (2)
WithName(41-45)WithDescription(54-58)sdk/tool/constructor.go (1)
WithNativeHandler(213-219)engine/core/memory.go (1)
MemoryReference(19-28)engine/project/config.go (1)
WorkflowSourceConfig(168-182)
sdk/client/client_test.go (4)
sdk/client/client.go (2)
New(56-92)WithAPIKey(33-37)sdk/client/types.go (4)
WorkflowExecuteRequest(12-12)TaskExecuteRequest(24-24)AgentExecuteRequest(33-33)StreamEvent(52-56)sdk/client/http.go (1)
APIError(24-29)pkg/logger/mod.go (2)
ContextWithLogger(39-41)NewForTests(199-201)
engine/project/indexer.go (4)
engine/project/config.go (1)
Config(276-495)engine/resources/store.go (3)
ResourceStore(82-123)ResourceMemory(24-24)ResourceKey(40-45)engine/memory/core/types.go (1)
Resource(70-141)engine/knowledge/config.go (1)
IngestManual(173-173)
sdk/compozy/engine_registration.go (3)
sdk/compozy/engine.go (1)
Engine(29-76)engine/resources/store.go (15)
ResourceProject(18-18)ResourceWorkflow(19-19)ResourceAgent(21-21)ResourceTool(22-22)ResourceKnowledgeBase(25-25)ResourceMemory(24-24)ResourceMCP(23-23)ResourceSchema(29-29)ResourceModel(30-30)ResourceSchedule(31-31)ResourceWebhook(32-32)ResourceStore(82-123)ResourceType(15-15)ResourceKey(40-45)ErrNotFound(127-127)engine/schema/schema.go (1)
GetID(171-181)
sdk/compozy/lifecycle.go (3)
sdk/compozy/engine.go (1)
Engine(29-76)engine/resources/store.go (1)
ResourceStore(82-123)engine/tool/inline/manager.go (3)
Manager(41-57)NewManager(73-110)Options(31-37)
⏰ 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
- GitHub Check: SDK Unit Tests
- GitHub Check: Container Security Scan
- GitHub Check: Security Scan (gosec)
- GitHub Check: Analyze (go)
🔇 Additional comments (5)
sdk/compozy/config/yaml_loader_test.go (1)
17-43: Excellent test implementation—past review comments addressed!This test correctly follows all coding guidelines:
- ✅ Uses
t.Context()instead ofcontext.Background()(line 19) - addressing the previous review comment- ✅ Properly wraps test logic in
t.Run("Should...")subtest (line 18) - addressing the previous review comment- ✅ Uses
testify/requirefor assertions with specific error validation viarequire.Contains- ✅ Logger obtained via
logger.NewForTests()and attached to context- ✅ Config manager properly initialized and attached to context
- ✅ Context propagated correctly throughout the test
- ✅ Automatic cleanup with
t.TempDir()- ✅ Function length well under 50 lines
- ✅ Focused test validating a single behavior (file path propagation in error messages)
The test effectively validates that
LoadToolsFromDirincludes the filename in error messages when YAML parsing fails, which aids debugging.sdk/compozy/integration/hybrid_yaml_integration_test.go (2)
20-44: LGTM! Past review feedback has been addressed.The test now properly wraps all assertions inside
t.Run("Should validate hybrid YAML integration", ...)as requested in the previous review. The implementation follows all testing guidelines:
- Uses the required
t.Run("Should...")subtest pattern- Context obtained via
lifecycleTestContext(t)helper- Proper error handling with
require.NoErrorfor setup andassert.Truefor validation- Appropriate file permissions (
0o755for directory,0o600for file)- Function length well under 50 lines
The test effectively validates the hybrid workflow scenario by combining a programmatic workflow with YAML-loaded workflows and confirming reference validation passes.
46-64: LGTM! Clean test helper implementation.The
programmaticWorkflowhelper is well-structured and appropriate for its purpose:
- Creates a simple two-task workflow with clear task dependencies
- The "start" task properly transitions to "finish" via
OnSuccess.Next- The "finish" task is correctly marked as final
- Function length (~18 lines) is well within the 50-line limit
- Clear, intention-revealing implementation suitable for integration testing
sdk/client/workflow.go (1)
14-72: LGTM!The workflow execution methods are well-structured:
- Function lengths are within limits (≤50 lines)
- Context is properly propagated
- Logging uses
logger.FromContext(ctx)as required- Error handling follows project patterns
sdk/compozy/execution_client_test.go (1)
20-469: Excellent test structure!This test file exemplifies best practices:
- All tests use
t.Run("Should...")subtests as required- Subtests correctly use
t.Parallel()- Uses
t.Context()via theexecutionContexthelper- Test utilities (
stubTransport,jsonHandler,streamHandler) are well-designed and reusable- Comprehensive coverage across async, sync, and streaming execution paths
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (9)
engine/llm/tool_registry_test.go(2 hunks)engine/tool/inline/manager.go(1 hunks)engine/tool/nativeuser/registry_test.go(1 hunks)magefile.go(1 hunks)sdk/client/client_test.go(1 hunks)sdk/compozy/lifecycle.go(1 hunks)sdk/compozy/lifecycle_helpers_test.go(1 hunks)sdk/compozy/loader_test.go(1 hunks)sdk/compozy/registration_errors_test.go(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.go
📄 CodeRabbit inference engine (.cursor/rules/api-standards.mdc)
**/*.go: Return properly structured error responses
Use middleware for cross-cutting concerns (e.g., logging, auth, rate limiting)
Implement proper authentication and authorization in the API
Apply rate limiting and request validation
Version API routes under the prefix /api/v0/
Use gin-gonic/gin for HTTP APIs
Ensure consistent response formats across all endpoints
Use appropriate HTTP status codes (200, 201, 400, 401, 403, 404, 500)
Return JSON responses with a consistent error structure
Update Swagger annotations for all API changes
Generate Swagger docs served at /swagger/index.html using swaggo/swag
Include request and response examples in Swagger annotations
Document all parameters, headers, and error responses in Swagger
Success response body should contain fields: data and message ("Success")
Error response body should contain fields: error and details
**/*.go: Retrieve the logger via logger.FromContext(ctx) inside runtime code (handlers, services, workers)
Retrieve configuration via config.FromContext(ctx) inside runtime code (handlers, services, workers)
Attach *config.Manager at process edges using config.ContextWithManager(ctx, mgr)
Build the logger with logger.SetupLogger(...) at startup and attach with logger.ContextWithLogger(ctx, log)
For HTTP servers, ensure request contexts inherit BOTH manager and logger via middleware
Do not pass loggers via function parameters or dependency injection; always use context-backed retrieval
Do not use a global configuration singleton; read config from context
CLI/root flow: parse flags → construct sources → mgr.Load(...) → attach manager to ctx → setup logger honoring cfg.CLI.Debug/Quiet → attach logger to ctx → propagate ctx
Server startup should use the CLI-provided ctx and add middleware that sets c.Request = c.Request.WithContext(logger.ContextWithLogger(config.ContextWithManager(c.Request.Context(), mgr), log))
Optionally set http.Server.BaseContext to a parent context carrying manager and logger so all req...
Files:
engine/llm/tool_registry_test.goengine/tool/nativeuser/registry_test.gosdk/client/client_test.gosdk/compozy/loader_test.gosdk/compozy/registration_errors_test.gosdk/compozy/lifecycle.gosdk/compozy/lifecycle_helpers_test.goengine/tool/inline/manager.gomagefile.go
**/*_test.go
📄 CodeRabbit inference engine (.cursor/rules/logger-config.mdc)
**/*_test.go: In tests, set up configuration with config.Initialize or config.NewManager(...).Load(...) and attach via config.ContextWithManager
In tests, prefer logger.NewForTests() and attach with logger.ContextWithLogger
In tests, mock or stub external tools/services; do not introduce DI of logger/config into code under test
**/*_test.go: Prefer local test constants (e.g., const testTimeout = 5 * time.Second) in tests
Inline trivial literals in tests when intent is clear
Do not duplicate production constants in tests; import them or assert relative behavior
**/*_test.go: All tests pass and follow the established testing patterns
Code is well-tested with both unit and integration tests
Tests should follow the t.Run("Should...") subtest naming pattern
Ensure adequate test coverage
**/*_test.go: All Go tests must use t.Run("Should describe behavior", ...) subtests for each behavior
Use stretchr/testify for assertions and mocks in tests
Do not use testify suite patterns (no suite.Suite embedding or suite-based structures)
Do not use suite methods like s.Equal(), s.NoError(), or s.T()
Avoid weak assertions like assert.Error(t, err); use specific error validation instead
Prefer specific error assertions like assert.ErrorContains and assert.ErrorAs for validating errors
Unit tests should be placed alongside implementation files as *_test.go
Use testify/mock for mocking external dependencies or complex interfacesIn tests, never use context.Background(); use t.Context() instead
Aim for 80%+ test coverage on business logic; write focused, isolated tests
**/*_test.go: Use stretchr/testify for assertions and mocks; import as github.com/stretchr/testify/assert and github.com/stretchr/testify/mock; replace custom mocks with testify/mock
Use project test helpers (utils.SetupTest, utils.SetupFixture, etc.) in testsIn tests, never use context.Background(); use t.Context()
Files:
engine/llm/tool_registry_test.goengine/tool/nativeuser/registry_test.gosdk/client/client_test.gosdk/compozy/loader_test.gosdk/compozy/registration_errors_test.gosdk/compozy/lifecycle_helpers_test.go
engine/{agent,task,task2,tool,workflow,runtime,llm,memory,memorycfg,knowledge,model,mcp,webhook,attachment,resources,auth,autoload,project,schema,worker}/**/*.go
📄 CodeRabbit inference engine (.cursor/rules/architecture.mdc)
engine/{agent,task,task2,tool,workflow,runtime,llm,memory,memorycfg,knowledge,model,mcp,webhook,attachment,resources,auth,autoload,project,schema,worker}/**/*.go: Application Layer (engine/): implement domain-specific business logic and ports; organize by domain with uc/ and router/ subpackages
Port interfaces (e.g., repositories, external services) are defined in Application Layer packages where they are used
Application layer depends inward: Application → Domain; avoid depending on infrastructure
Files:
engine/llm/tool_registry_test.goengine/tool/nativeuser/registry_test.goengine/tool/inline/manager.go
{engine,pkg,cli}/**/*.go
📄 CodeRabbit inference engine (.cursor/rules/architecture.mdc)
Context as the first parameter for I/O or long-running operations; always propagate and handle cancellation
Files:
engine/llm/tool_registry_test.goengine/tool/nativeuser/registry_test.goengine/tool/inline/manager.go
**/!(*_test).go
📄 CodeRabbit inference engine (.cursor/rules/magic-numbers.mdc)
**/!(*_test).go: NEVER introduce magic numbers in runtime Go code; replace unexplained numeric literals with named constants or configuration
Operator-tunable values MUST be sourced from configuration as defined in @.cursor/rules/global-config.mdc
Fetch configuration via config.FromContext(ctx); do not use global singletons
Promote tunable values (timeouts, deadlines, polling intervals, retry/backoff counts and factors, concurrency limits, queue sizes, buffer/payload limits, behavior thresholds) to configurationIn runtime code, properly inherit context; never use context.Background() in code paths
In runtime code paths, inherit context properly; never use context.Background()
In runtime code paths, never use context.Background(); properly inherit context
In runtime code, properly inherit context; never use context.Background()
Files:
sdk/compozy/lifecycle.goengine/tool/inline/manager.gomagefile.go
🧠 Learnings (2)
📚 Learning: 2025-10-20T04:55:38.336Z
Learnt from: CR
Repo: compozy/compozy PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-20T04:55:38.336Z
Learning: Applies to **/*.go : Prefer sync.WaitGroup.Go(func()) over manual Add/go func/Done pattern (Go 1.25+)
Applied to files:
engine/tool/nativeuser/registry_test.go
📚 Learning: 2025-10-20T04:54:19.496Z
Learnt from: CR
Repo: compozy/compozy PR: 0
File: .cursor/rules/critical-validation.mdc:0-0
Timestamp: 2025-10-20T04:54:19.496Z
Learning: Applies to **/*.go : Prefer sync.WaitGroup.Go(func()) over manual Add/go/Done in Go 1.25+
Applied to files:
engine/tool/nativeuser/registry_test.go
🧬 Code graph analysis (8)
engine/llm/tool_registry_test.go (3)
engine/tool/config.go (2)
Config(104-208)ImplementationNative(24-24)engine/tool/nativeuser/registry.go (2)
Reset(78-83)Register(33-46)engine/llm/tool_registry.go (1)
NewNativeToolAdapter(595-597)
engine/tool/nativeuser/registry_test.go (1)
engine/tool/nativeuser/registry.go (8)
Reset(78-83)Register(33-46)Lookup(49-63)Handler(14-14)ErrInvalidID(25-25)ErrNilHandler(27-27)ErrAlreadyRegistered(29-29)IDs(66-75)
sdk/client/client_test.go (4)
sdk/client/client.go (2)
New(56-92)WithAPIKey(33-37)sdk/client/types.go (4)
WorkflowExecuteRequest(12-12)TaskExecuteRequest(24-24)AgentExecuteRequest(33-33)StreamEvent(52-56)sdk/client/http.go (1)
APIError(24-29)pkg/logger/mod.go (2)
ContextWithLogger(39-41)NewForTests(199-201)
sdk/compozy/loader_test.go (1)
sdk/compozy/engine.go (1)
Engine(29-76)
sdk/compozy/registration_errors_test.go (3)
engine/resources/store.go (6)
ResourceKey(40-45)ETag(60-60)ResourceMeta(35-35)ResourceType(15-15)StoredItem(71-75)ResourceWorkflow(19-19)sdk/compozy/constructor.go (1)
New(21-60)sdk/compozy/engine.go (1)
Engine(29-76)
sdk/compozy/lifecycle.go (3)
sdk/compozy/engine.go (1)
Engine(29-76)engine/resources/store.go (1)
ResourceStore(82-123)engine/tool/inline/manager.go (3)
Manager(41-57)NewManager(73-110)Options(31-37)
sdk/compozy/lifecycle_helpers_test.go (2)
sdk/compozy/engine.go (1)
Engine(29-76)pkg/logger/mod.go (2)
ContextWithLogger(39-41)NewForTests(199-201)
engine/tool/inline/manager.go (3)
engine/resources/store.go (2)
ResourceStore(82-123)ResourceTool(22-22)engine/core/types.go (1)
GetStoreDir(15-20)engine/tool/config.go (2)
Config(104-208)ImplementationRuntime(23-23)
⏰ 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
- GitHub Check: Container Security Scan
- GitHub Check: SDK Unit Tests
- GitHub Check: Analyze (go)
- GitHub Check: Security Scan (gosec)
🔇 Additional comments (25)
sdk/compozy/registration_errors_test.go (6)
24-99: LGTM! Well-designed test stub.The
resourceStoreStubprovides targeted error injection for testing edge cases while maintaining a simple in-memory store. The distinction between meta and non-meta resource errors is particularly useful for validating error propagation paths.
101-145: LGTM! Proper subtest structure adopted.All scenarios are now properly wrapped in
t.Run("Should...")subtests, addressing the previous review feedback. The test correctly validates required identifiers and gracefully handles nil context/store scenarios.
147-219: LGTM! Comprehensive error and state handling tests.These tests verify critical error propagation paths and state consistency. The state reset test (lines 206-219) correctly uses locking to verify that the engine's project field is properly cleaned up after persistence failure, preventing inconsistent state.
221-325: LGTM! Comprehensive nil config validation.Table-driven test properly validates nil config handling across all resource types. Each case is correctly wrapped in a
t.Run("Should...")subtest, ensuring clear test output and proper isolation.
327-421: LGTM! Thorough identifier validation coverage.Table-driven test validates empty/missing identifiers for all resource types with clear error message verification. Properly structured with
t.Run("Should...")subtests.
423-471: LGTM! Previous feedback addressed.The test now properly wraps all logic in a
t.Run("Should detect duplicate registrations across resources", ...)subtest, resolving the prior review concern. The test comprehensively validates duplicate detection across all ten resource types with clear separation between successful initial registrations and expected duplicate errors.sdk/compozy/loader_test.go (7)
17-35: LGTM! Test follows all guidelines.This test correctly uses t.Run with "Should..." naming, t.Parallel(), lifecycleTestContext(t), and testify assertions. The test structure is clear and validates the success path for YAML loading.
37-60: LGTM! Error accumulation test is well-structured.This test properly validates error accumulation during directory loading, ensuring all files are processed and errors are reported with the correct file paths.
62-94: LGTM! Error conditions properly split into subtests.All previous feedback has been addressed. Each error scenario is now properly isolated in its own subtest with appropriate validation. The intentional nil context test (line 82) correctly includes a lint ignore directive.
96-110: LGTM! Context cancellation test is correct.This test properly validates that canceled contexts are handled correctly during YAML loading operations.
112-121: LGTM! Stat failure test is well-structured.This test correctly validates error handling when attempting to stat a non-existent file.
138-153: LGTM! Input validation properly split into subtests.All previous feedback has been addressed. Each validation scenario is now properly isolated in its own subtest with clear assertions.
155-167: LGTM! Directory context cancellation test is correct.This test properly validates that canceled contexts are handled correctly during directory loading operations.
engine/tool/nativeuser/registry_test.go (3)
13-29: LGTM! Previous review feedback has been addressed.The test properly uses
t.Context(), follows thet.Run("Should...")pattern, and includes proper cleanup witht.Cleanup(Reset). The test coverage for the registration and lookup flow is appropriate.
51-63: LGTM!The test correctly validates duplicate registration handling with proper use of
assert.ErrorIsfor error validation.
65-96: LGTM! All previous review feedback has been addressed.The test properly uses:
t.Context()instead ofcontext.Background()sync.WaitGroup.Go()pattern (Go 1.25+) instead of manual Add/go/Donet.Run()with descriptive subtest naming- Proper cleanup with
t.Cleanup(Reset)The concurrent registration test provides good coverage for race conditions and registry thread-safety.
magefile.go (4)
26-35: LGTM - Past review comments addressed!The magic numbers have been properly replaced with named constants (
testParallelism,testParallelFlag,minSDKCoverage), and the constants have clear, intention-revealing names with appropriate comments.
451-469: LGTM - Modern concurrency pattern adopted!The function now uses
errgroup.WithContextfor better error handling and automatic context cancellation, addressing the previous review comment. This is cleaner than the manual WaitGroup + error channel pattern.
546-615: LGTM - Mage targets properly exposed!All per-module helper functions now have the
//mage:exposedirective, making them callable from the Makefile. This addresses the previous issue wheremake lint-main,make fmt-main, etc. would fail with "unknown target".
632-634: LGTM - Simplified rebuild logic!The function now unconditionally returns
trueto force Swagger rebuild, ensuring documentation never goes stale. This is the simpler, safer approach compared to tracking specific source files.sdk/client/client_test.go (4)
21-54: LGTM! Past review feedback addressed.The test now correctly uses
t.Runsubtest wrapper, follows the "Should..." naming pattern, usest.Context(), and includes comprehensive assertions for authorization headers, request body validation, and response field verification.
80-124: LGTM! Comprehensive streaming test.The test thoroughly validates Server-Sent Events streaming, including event parsing, EOF handling, proper cleanup with defer, and timeout protection in the helper. The SSE format and assertions are correct.
126-147: LGTM! Proper error validation.The test correctly uses
ErrorAsfor specific error type validation (as per coding guidelines) and thoroughly validates theAPIErrorfields including code and HTTP status mapping.
149-187: LGTM! Well-structured test helpers.All helper functions properly use
t.Helper(), stay within size limits, and follow testing best practices. ThewithTestLoggerhelper correctly useslogger.NewForTests()andContextWithLoggeras per guidelines.engine/llm/tool_registry_test.go (1)
245-319: Great coverage for the native tool adapter.The subtests now follow the required “Should …” naming, reset the native registry per case, and assert the expected success/error behaviors—solid isolation and coverage.
| func TestRegisterValidation(t *testing.T) { | ||
| t.Run("Should reject empty ID", func(t *testing.T) { | ||
| Reset() | ||
| t.Cleanup(Reset) | ||
| assert.Equal( | ||
| t, | ||
| ErrInvalidID, | ||
| Register("", func(context.Context, map[string]any, map[string]any) (map[string]any, error) { | ||
| return nil, nil | ||
| }), | ||
| ) | ||
| }) | ||
|
|
||
| t.Run("Should reject nil handler", func(t *testing.T) { | ||
| Reset() | ||
| t.Cleanup(Reset) | ||
| assert.Equal(t, ErrNilHandler, Register("tool", nil)) | ||
| }) | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider using assert.ErrorIs for error comparisons.
While assert.Equal works for sentinel errors, assert.ErrorIs is more idiomatic and consistent with line 61. This improves readability and aligns with Go error handling best practices.
Apply this diff for consistency:
t.Run("Should reject empty ID", func(t *testing.T) {
Reset()
t.Cleanup(Reset)
- assert.Equal(
- t,
- ErrInvalidID,
- Register("", func(context.Context, map[string]any, map[string]any) (map[string]any, error) {
- return nil, nil
- }),
- )
+ err := Register("", func(context.Context, map[string]any, map[string]any) (map[string]any, error) {
+ return nil, nil
+ })
+ assert.ErrorIs(t, err, ErrInvalidID)
})
t.Run("Should reject nil handler", func(t *testing.T) {
Reset()
t.Cleanup(Reset)
- assert.Equal(t, ErrNilHandler, Register("tool", nil))
+ err := Register("tool", nil)
+ assert.ErrorIs(t, err, ErrNilHandler)
})📝 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.
| func TestRegisterValidation(t *testing.T) { | |
| t.Run("Should reject empty ID", func(t *testing.T) { | |
| Reset() | |
| t.Cleanup(Reset) | |
| assert.Equal( | |
| t, | |
| ErrInvalidID, | |
| Register("", func(context.Context, map[string]any, map[string]any) (map[string]any, error) { | |
| return nil, nil | |
| }), | |
| ) | |
| }) | |
| t.Run("Should reject nil handler", func(t *testing.T) { | |
| Reset() | |
| t.Cleanup(Reset) | |
| assert.Equal(t, ErrNilHandler, Register("tool", nil)) | |
| }) | |
| } | |
| func TestRegisterValidation(t *testing.T) { | |
| t.Run("Should reject empty ID", func(t *testing.T) { | |
| Reset() | |
| t.Cleanup(Reset) | |
| err := Register("", func(context.Context, map[string]any, map[string]any) (map[string]any, error) { | |
| return nil, nil | |
| }) | |
| assert.ErrorIs(t, err, ErrInvalidID) | |
| }) | |
| t.Run("Should reject nil handler", func(t *testing.T) { | |
| Reset() | |
| t.Cleanup(Reset) | |
| err := Register("tool", nil) | |
| assert.ErrorIs(t, err, ErrNilHandler) | |
| }) | |
| } |
🤖 Prompt for AI Agents
In engine/tool/nativeuser/registry_test.go around lines 31 to 49, the tests
currently use assert.Equal to compare sentinel errors; replace those with
assert.ErrorIs to be idiomatic and consistent with line 61. For the first
subtest, call Register(...) and use assert.ErrorIs(t, err, ErrInvalidID)
(capture the returned error into a variable if needed); for the second subtest,
call Register("tool", nil) and use assert.ErrorIs(t, err, ErrNilHandler). Ensure
the assert imports remain and adjust any variable names accordingly.
| func (Integration) SdkCompozy(ctx context.Context) error { | ||
| cmd := exec.CommandContext(ctx, "gotestsum", | ||
| "--format", "pkgname", | ||
| "--", | ||
| "-race", | ||
| "-parallel=4", | ||
| "./sdk/compozy/...", | ||
| ) | ||
| cmd.Stdout = os.Stdout | ||
| cmd.Stderr = os.Stderr | ||
| return cmd.Run() | ||
| } |
There was a problem hiding this comment.
Use testParallelFlag constant for consistency.
This function hardcodes "-parallel=4" while all other test functions use the testParallelFlag constant (lines 475, 484, 513, 526, 534, 543). This inconsistency means changes to parallelism settings would miss this location.
Apply this diff:
func (Integration) SdkCompozy(ctx context.Context) error {
cmd := exec.CommandContext(ctx, "gotestsum",
"--format", "pkgname",
"--",
"-race",
- "-parallel=4",
+ testParallelFlag,
"./sdk/compozy/...",
)Based on coding guidelines.
🤖 Prompt for AI Agents
In magefile.go around lines 364 to 375, the SdkCompozy function hardcodes
"-parallel=4" instead of using the shared testParallelFlag constant; replace the
literal "-parallel=4" argument with the testParallelFlag constant so this
function follows the same configuration source as the other test tasks and will
pick up future changes to parallelism.
| resp, err := cl.ExecuteTaskSync(ctx, "build", &client.TaskExecuteRequest{}) | ||
| require.NoError(t, err) | ||
| require.Equal(t, "task-exec", resp.ExecID) | ||
| require.Equal(t, "binary", (*resp.Output)["artifact"]) |
There was a problem hiding this comment.
Add nil check before dereferencing pointer.
Line 76 dereferences resp.Output without verifying it's non-nil. If Output is unexpectedly nil, the test will panic instead of failing with a clear assertion message.
Apply this diff to add defensive nil check:
resp, err := cl.ExecuteTaskSync(ctx, "build", &client.TaskExecuteRequest{})
require.NoError(t, err)
+ require.NotNil(t, resp.Output)
require.Equal(t, "task-exec", resp.ExecID)
require.Equal(t, "binary", (*resp.Output)["artifact"])📝 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.
| resp, err := cl.ExecuteTaskSync(ctx, "build", &client.TaskExecuteRequest{}) | |
| require.NoError(t, err) | |
| require.Equal(t, "task-exec", resp.ExecID) | |
| require.Equal(t, "binary", (*resp.Output)["artifact"]) | |
| resp, err := cl.ExecuteTaskSync(ctx, "build", &client.TaskExecuteRequest{}) | |
| require.NoError(t, err) | |
| require.NotNil(t, resp.Output) | |
| require.Equal(t, "task-exec", resp.ExecID) | |
| require.Equal(t, "binary", (*resp.Output)["artifact"]) |
🤖 Prompt for AI Agents
In sdk/client/client_test.go around lines 73 to 76, the test dereferences
resp.Output without checking for nil which can cause a panic; add a defensive
check (e.g., require.NotNil(t, resp.Output)) immediately after asserting no
error and before accessing (*resp.Output)["artifact"], then proceed with the
existing assertion (or use require.Contains/require.Equal on the map) so the
test fails with a clear message instead of panicking.
| cfg := appconfig.FromContext(ctx) | ||
| require.NotNil(t, cfg) |
There was a problem hiding this comment.
Remove unused config retrieval.
The config is retrieved and asserted but never used in the test. These lines serve no purpose for validating YAML decode failures.
Apply this diff to remove the unused code:
t.Run("Should report decode failure for invalid YAML", func(t *testing.T) {
t.Parallel()
ctx := lifecycleTestContext(t)
engine := &Engine{ctx: ctx}
- cfg := appconfig.FromContext(ctx)
- require.NotNil(t, cfg)
bad := filepath.Join(t.TempDir(), "invalid.yaml")
require.NoError(t, os.WriteFile(bad, []byte("{"), 0o600))
_, _, err := loadYAML[*enginetool.Config](ctx, engine, bad)📝 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.
| cfg := appconfig.FromContext(ctx) | |
| require.NotNil(t, cfg) | |
| t.Run("Should report decode failure for invalid YAML", func(t *testing.T) { | |
| t.Parallel() | |
| ctx := lifecycleTestContext(t) | |
| engine := &Engine{ctx: ctx} | |
| bad := filepath.Join(t.TempDir(), "invalid.yaml") | |
| require.NoError(t, os.WriteFile(bad, []byte("{"), 0o600)) | |
| _, _, err := loadYAML[*enginetool.Config](ctx, engine, bad) | |
| require.Error(t, err) | |
| }) |
🤖 Prompt for AI Agents
In sdk/compozy/loader_test.go around lines 128-129, the test retrieves cfg via
appconfig.FromContext(ctx) and asserts require.NotNil(t, cfg) but never uses
cfg; remove these two lines to eliminate the unused retrieval and assertion and
update any surrounding test code if necessary so compilation and tests still
pass.
1 participant
Summary by CodeRabbit
New Features
Improvements
Build & Tooling