refactor(repo): Add temporal standalone mode#305
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughAdds embedded Temporal server support (config, builder, Server/UIServer, namespace creation), integrates optional standalone startup into server dependencies, extends configuration and CLI flags for standalone mode, introduces activity-monitoring and retry behavior for the markdown check script, and adds many unit and integration tests and example files. Changes
Sequence Diagram(s)sequenceDiagram
participant App as Application
participant Deps as Dependency Setup
participant Embedded as Embedded Package
participant Temporal as Temporal Server (in-process)
participant UI as Temporal UI (optional)
App->>Deps: boot
Deps->>Embedded: maybeStartStandaloneTemporal(cfg)
Embedded->>Embedded: validate & build embedded.Config
Embedded->>Embedded: ensure ports available
Embedded->>Temporal: construct & start server
Temporal-->>Embedded: started (frontend address)
Embedded->>UI: start UI server (if enabled)
UI-->>Embedded: UI ready
Embedded-->>Deps: register cleanup, expose HostPort
Note over Temporal,UI: Normal operation (workflows, workers)
App->>Deps: shutdown
Deps->>Embedded: invoke cleanup
Embedded->>UI: Stop UI server
Embedded->>Temporal: Stop server (with timeout)
Embedded-->>Deps: cleanup complete
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Areas requiring extra attention:
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 27
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
scripts/markdown/check.go (4)
1066-1079: Extract shutdown timeout to a named constant.The shutdown timeout
30 * time.Secondon line 1067 is a magic number. Define it as a named constant to make the shutdown policy explicit.Apply this diff:
const ( unknownFileName = "unknown" ideCodex = "codex" ideClaude = "claude" ideDroid = "droid" defaultCodexModel = "gpt-5-codex" defaultClaudeModel = "claude-sonnet-4-5-20250929" + gracefulShutdownTimeout = 30 * time.Second thinkPromptMedium = "Think hard through problems carefully before acting. Balance speed with thoroughness."Then use it:
func (j *jobExecutionContext) awaitShutdownAfterCancel(done <-chan struct{}) (int32, []failInfo, int, error) { - shutdownTimeout := 30 * time.Second + shutdownTimeout := gracefulShutdownTimeout shutdownCtx, shutdownCancel := context.WithTimeout(context.Background(), shutdownTimeout)
992-998: Extract UI cleanup delay to a named constant.The
80 * time.Milliseconddelay on line 995 is a magic number. Extract it to a named constant with a comment explaining it allows UI message draining before quit.Apply this diff:
const ( unknownFileName = "unknown" ideCodex = "codex" ideClaude = "claude" ideDroid = "droid" defaultCodexModel = "gpt-5-codex" defaultClaudeModel = "claude-sonnet-4-5-20250929" + uiMessageDrainDelay = 80 * time.Millisecond // Allow pending UI messages to drain before quit thinkPromptMedium = "Think hard through problems carefully before acting. Balance speed with thoroughness."Then use it:
func (j *jobExecutionContext) cleanup() { if j.uiProg != nil { close(j.uiCh) - time.Sleep(80 * time.Millisecond) + time.Sleep(uiMessageDrainDelay) j.uiProg.Quit() } }
2147-2149: Extract UI tick interval to a named constant.The
120 * time.MillisecondUI refresh interval on line 2148 is a magic number. Define it as a named constant to make the refresh rate explicit and tunable.Apply this diff:
const ( unknownFileName = "unknown" ideCodex = "codex" ideClaude = "claude" ideDroid = "droid" defaultCodexModel = "gpt-5-codex" defaultClaudeModel = "claude-sonnet-4-5-20250929" + uiTickInterval = 120 * time.Millisecond thinkPromptMedium = "Think hard through problems carefully before acting. Balance speed with thoroughness."Then use it:
func (m *uiModel) tick() tea.Cmd { - return tea.Tick(120*time.Millisecond, func(time.Time) tea.Msg { return tickMsg{} }) + return tea.Tick(uiTickInterval, func(time.Time) tea.Msg { return tickMsg{} }) }
618-643: Consider adding validation for retry parameters.The validation currently checks mode, IDE, and batch size, but doesn't validate:
maxRetriescould be negative (though defaults to 3)retryBackoffMultipliercould be zero or negative, preventing timeout increasesConsider adding bounds checks to ensure sensible retry behavior:
func (c *cliArgs) validate() error { if c.mode != modeCodeReview && c.mode != modePRDTasks { return fmt.Errorf( "invalid --mode value '%s': must be '%s' or '%s'", c.mode, modeCodeReview, modePRDTasks, ) } if c.ide != ideClaude && c.ide != ideCodex && c.ide != ideDroid { return fmt.Errorf( "invalid --ide value '%s': must be '%s', '%s', or '%s'", c.ide, ideClaude, ideCodex, ideDroid, ) } if c.mode == modePRDTasks && c.batchSize != 1 { return fmt.Errorf( "batch size must be 1 for prd-tasks mode (got %d)", c.batchSize, ) } + if c.maxRetries < 0 { + return fmt.Errorf("max-retries cannot be negative (got %d)", c.maxRetries) + } + if c.retryBackoffMultiplier <= 0 { + return fmt.Errorf("retry-backoff-multiplier must be positive (got %.2f)", c.retryBackoffMultiplier) + } return nil }pkg/config/loader.go (1)
376-392: Fix Temporal mode handling: wrong constant, and host_port rules per mode.
- Use a Temporal-specific constant (not mcpProxyModeStandalone).
- Require host_port only in remote mode; in standalone, allow empty (derived) or enforce match with BindIP:FrontendPort.
Apply:
@@ -func validateTemporal(cfg *Config) error { - if cfg.Temporal.HostPort == "" { - return fmt.Errorf("temporal host_port is required") - } - mode := cfg.Temporal.Mode +func validateTemporal(cfg *Config) error { + mode := strings.TrimSpace(cfg.Temporal.Mode) if mode == "" { return fmt.Errorf("temporal.mode is required") } switch mode { case "remote": - return nil - case mcpProxyModeStandalone: - return validateStandaloneTemporalConfig(cfg) + if cfg.Temporal.HostPort == "" { + return fmt.Errorf("temporal.host_port is required in remote mode") + } + return nil + case temporalModeStandalone: + // Derive or validate host_port when provided + if cfg.Temporal.HostPort != "" { + want := net.JoinHostPort( + cfg.Temporal.Standalone.BindIP, + strconv.Itoa(cfg.Temporal.Standalone.FrontendPort), + ) + if cfg.Temporal.Standalone.BindIP != "" && + cfg.Temporal.Standalone.FrontendPort != 0 && + cfg.Temporal.HostPort != want { + return fmt.Errorf("temporal.host_port (%s) must match %s when mode=standalone", cfg.Temporal.HostPort, want) + } + } + return validateStandaloneTemporalConfig(cfg) default: return fmt.Errorf("temporal.mode must be one of [remote standalone], got %q", mode) } }Also add at file scope:
+const ( + maxTCPPort = 65535 + temporalServiceSpan = 3 // Frontend + History + Matching + Worker + temporalModeStandalone = "standalone" +)As per coding guidelines.
📜 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 (65)
README.mdis excluded by!**/*.mdcli/help/global-flags.mdis excluded by!**/*.mddocs/content/docs/architecture/embedded-temporal.mdxis excluded by!**/*.mdxdocs/content/docs/architecture/meta.jsonis excluded by!**/*.jsondocs/content/docs/cli/compozy-start.mdxis excluded by!**/*.mdxdocs/content/docs/cli/meta.jsonis excluded by!**/*.jsondocs/content/docs/configuration/meta.jsonis excluded by!**/*.jsondocs/content/docs/configuration/temporal.mdxis excluded by!**/*.mdxdocs/content/docs/core/deployment/meta.jsonis excluded by!**/*.jsondocs/content/docs/core/tasks/collection-tasks.mdxis excluded by!**/*.mdxdocs/content/docs/core/tasks/memory-tasks.mdxis excluded by!**/*.mdxdocs/content/docs/core/tasks/parallel-processing.mdxis excluded by!**/*.mdxdocs/content/docs/deployment/docker.mdxis excluded by!**/*.mdxdocs/content/docs/deployment/kubernetes.mdxis excluded by!**/*.mdxdocs/content/docs/deployment/meta.jsonis excluded by!**/*.jsondocs/content/docs/deployment/production.mdxis excluded by!**/*.mdxdocs/content/docs/deployment/temporal-modes.mdxis excluded by!**/*.mdxdocs/content/docs/meta.jsonis excluded by!**/*.jsondocs/content/docs/quick-start/index.mdxis excluded by!**/*.mdxdocs/content/docs/quick-start/meta.jsonis excluded by!**/*.jsondocs/content/docs/troubleshooting/meta.jsonis excluded by!**/*.jsondocs/content/docs/troubleshooting/temporal.mdxis excluded by!**/*.mdxexamples/temporal-standalone/basic/README.mdis excluded by!**/*.mdexamples/temporal-standalone/basic/compozy.yamlis excluded by!**/*.yamlexamples/temporal-standalone/basic/workflow.yamlis excluded by!**/*.yamlexamples/temporal-standalone/custom-ports/README.mdis excluded by!**/*.mdexamples/temporal-standalone/custom-ports/compozy.yamlis excluded by!**/*.yamlexamples/temporal-standalone/custom-ports/workflow.yamlis excluded by!**/*.yamlexamples/temporal-standalone/debugging/README.mdis excluded by!**/*.mdexamples/temporal-standalone/debugging/compozy.yamlis excluded by!**/*.yamlexamples/temporal-standalone/debugging/workflow.yamlis excluded by!**/*.yamlexamples/temporal-standalone/integration-testing/README.mdis excluded by!**/*.mdexamples/temporal-standalone/integration-testing/compozy.yamlis excluded by!**/*.yamlexamples/temporal-standalone/integration-testing/workflow.yamlis excluded by!**/*.yamlexamples/temporal-standalone/migration-from-remote/README.mdis excluded by!**/*.mdexamples/temporal-standalone/migration-from-remote/compozy.remote.yamlis excluded by!**/*.yamlexamples/temporal-standalone/migration-from-remote/compozy.standalone.yamlis excluded by!**/*.yamlexamples/temporal-standalone/migration-from-remote/compozy.yamlis excluded by!**/*.yamlexamples/temporal-standalone/migration-from-remote/docker-compose.ymlis excluded by!**/*.ymlexamples/temporal-standalone/migration-from-remote/workflow.yamlis excluded by!**/*.yamlexamples/temporal-standalone/no-ui/README.mdis excluded by!**/*.mdexamples/temporal-standalone/no-ui/compozy.yamlis excluded by!**/*.yamlexamples/temporal-standalone/no-ui/workflow.yamlis excluded by!**/*.yamlexamples/temporal-standalone/persistent/README.mdis excluded by!**/*.mdexamples/temporal-standalone/persistent/compozy.yamlis excluded by!**/*.yamlexamples/temporal-standalone/persistent/workflow.yamlis excluded by!**/*.yamlgo.sumis excluded by!**/*.sumschemas/config.jsonis excluded by!**/*.jsontasks/prd-temporal/README.mdis excluded by!**/*.mdtasks/prd-temporal/_docs.mdis excluded by!**/*.mdtasks/prd-temporal/_examples.mdis excluded by!**/*.mdtasks/prd-temporal/_task_01.mdis excluded by!**/*.mdtasks/prd-temporal/_task_02.mdis excluded by!**/*.mdtasks/prd-temporal/_task_03.mdis excluded by!**/*.mdtasks/prd-temporal/_task_04.mdis excluded by!**/*.mdtasks/prd-temporal/_task_05.mdis excluded by!**/*.mdtasks/prd-temporal/_task_06.mdis excluded by!**/*.mdtasks/prd-temporal/_task_07.mdis excluded by!**/*.mdtasks/prd-temporal/_task_08.mdis excluded by!**/*.mdtasks/prd-temporal/_task_09.mdis excluded by!**/*.mdtasks/prd-temporal/_task_10.mdis excluded by!**/*.mdtasks/prd-temporal/_tasks.mdis excluded by!**/*.mdtasks/prd-temporal/_techspec.mdis excluded by!**/*.mdtasks/prd-temporal/_tests.mdis excluded by!**/*.mdtest/integration/temporal/testdata/workflow_input.jsonis excluded by!**/*.json
📒 Files selected for processing (36)
cli/helpers/flag_categories.go(1 hunks)docs/source.config.ts(2 hunks)engine/infra/server/dependencies.go(3 hunks)engine/worker/embedded/builder.go(1 hunks)engine/worker/embedded/builder_test.go(1 hunks)engine/worker/embedded/config.go(1 hunks)engine/worker/embedded/config_test.go(1 hunks)engine/worker/embedded/namespace.go(1 hunks)engine/worker/embedded/namespace_test.go(1 hunks)engine/worker/embedded/server.go(1 hunks)engine/worker/embedded/server_test.go(1 hunks)engine/worker/embedded/ui.go(1 hunks)engine/worker/embedded/ui_test.go(1 hunks)examples/temporal-standalone/basic/.env.example(1 hunks)examples/temporal-standalone/custom-ports/.env.example(1 hunks)examples/temporal-standalone/debugging/.env.example(1 hunks)examples/temporal-standalone/debugging/tools/index.ts(1 hunks)examples/temporal-standalone/integration-testing/.env.example(1 hunks)examples/temporal-standalone/integration-testing/Makefile(1 hunks)examples/temporal-standalone/integration-testing/tests/integration_test.go(1 hunks)examples/temporal-standalone/migration-from-remote/.env.example(1 hunks)examples/temporal-standalone/no-ui/.env.example(1 hunks)examples/temporal-standalone/persistent/.env.example(1 hunks)examples/temporal-standalone/persistent/.gitignore(1 hunks)go.mod(9 hunks)pkg/config/config.go(3 hunks)pkg/config/config_test.go(3 hunks)pkg/config/definition/schema.go(2 hunks)pkg/config/loader.go(2 hunks)pkg/config/provider.go(1 hunks)scripts/markdown/check.go(36 hunks)test/integration/temporal/errors_test.go(1 hunks)test/integration/temporal/mode_switching_test.go(1 hunks)test/integration/temporal/persistence_test.go(1 hunks)test/integration/temporal/standalone_test.go(1 hunks)test/integration/temporal/startup_lifecycle_test.go(1 hunks)
🧰 Additional context used
📓 Path-based instructions (22)
**/*.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/worker/embedded/ui_test.goengine/worker/embedded/namespace_test.gopkg/config/provider.goengine/worker/embedded/server_test.gocli/helpers/flag_categories.goengine/worker/embedded/config.gotest/integration/temporal/errors_test.goengine/worker/embedded/builder_test.gotest/integration/temporal/mode_switching_test.gopkg/config/config.gotest/integration/temporal/startup_lifecycle_test.goengine/worker/embedded/server.gotest/integration/temporal/persistence_test.goengine/worker/embedded/namespace.goengine/worker/embedded/builder.goexamples/temporal-standalone/integration-testing/tests/integration_test.goengine/infra/server/dependencies.goengine/worker/embedded/ui.gopkg/config/definition/schema.gopkg/config/config_test.goengine/worker/embedded/config_test.gotest/integration/temporal/standalone_test.gopkg/config/loader.goscripts/markdown/check.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/worker/embedded/ui_test.goengine/worker/embedded/namespace_test.goengine/worker/embedded/server_test.gotest/integration/temporal/errors_test.goengine/worker/embedded/builder_test.gotest/integration/temporal/mode_switching_test.gotest/integration/temporal/startup_lifecycle_test.gotest/integration/temporal/persistence_test.goexamples/temporal-standalone/integration-testing/tests/integration_test.gopkg/config/config_test.goengine/worker/embedded/config_test.gotest/integration/temporal/standalone_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/worker/embedded/ui_test.goengine/worker/embedded/namespace_test.goengine/worker/embedded/server_test.goengine/worker/embedded/config.goengine/worker/embedded/builder_test.goengine/worker/embedded/server.goengine/worker/embedded/namespace.goengine/worker/embedded/builder.goengine/worker/embedded/ui.goengine/worker/embedded/config_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/worker/embedded/ui_test.goengine/worker/embedded/namespace_test.gopkg/config/provider.goengine/worker/embedded/server_test.gocli/helpers/flag_categories.goengine/worker/embedded/config.goengine/worker/embedded/builder_test.gopkg/config/config.goengine/worker/embedded/server.goengine/worker/embedded/namespace.goengine/worker/embedded/builder.goengine/infra/server/dependencies.goengine/worker/embedded/ui.gopkg/config/definition/schema.gopkg/config/config_test.goengine/worker/embedded/config_test.gopkg/config/loader.go
pkg/config/provider.go
📄 CodeRabbit inference engine (.cursor/rules/global-config.mdc)
pkg/config/provider.go: Rely on Default() built from the registry for defaults; only add to createDefaults if special formatting (e.g., duration String()) is needed
Add createDefaults and include it in add<Service|Infra|Core>Defaults for new categories
Stringify time.Duration defaults when building the defaults map if special formatting is required
Files:
pkg/config/provider.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:
pkg/config/provider.gocli/helpers/flag_categories.goengine/worker/embedded/config.gopkg/config/config.goengine/worker/embedded/server.goengine/worker/embedded/namespace.goengine/worker/embedded/builder.goengine/infra/server/dependencies.goengine/worker/embedded/ui.gopkg/config/definition/schema.gopkg/config/loader.goscripts/markdown/check.go
pkg/**/*.go
📄 CodeRabbit inference engine (.cursor/rules/architecture.mdc)
Shared Packages (pkg): provide framework-level utilities (config, logging, templates, schema generation); must not contain business logic
Files:
pkg/config/provider.gopkg/config/config.gopkg/config/definition/schema.gopkg/config/config_test.gopkg/config/loader.go
pkg/config/**/*.go
📄 CodeRabbit inference engine (.cursor/rules/architecture.mdc)
Global configuration resides in pkg/config; expose retrieval via context (config.FromContext) and provide defaults; avoid global singletons
Files:
pkg/config/provider.gopkg/config/config.gopkg/config/definition/schema.gopkg/config/config_test.gopkg/config/loader.go
cli/helpers/flag_categories.go
📄 CodeRabbit inference engine (.cursor/rules/global-config.mdc)
cli/helpers/flag_categories.go: Categorize new CLI flags by adding their names to the appropriate category in flag_categories.go for better help UX
For new categories, add a category block or include flags in an existing category in flag_categories.goEnvironment variable overrides follow ATTACHMENTS_* naming (e.g., ATTACHMENTS_DOWNLOAD_TIMEOUT); ensure mapping from flags to env vars uses this prefix
Files:
cli/helpers/flag_categories.go
cli/**/*.go
📄 CodeRabbit inference engine (.cursor/rules/architecture.mdc)
cli/**/*.go: Presentation Layer (cli): implement CLI, user interaction/formatting, and API client; no business logic
CLI depends inward: CLI → Application → Domain; do not depend directly on infrastructure
Files:
cli/helpers/flag_categories.go
test/integration/**/*_test.go
📄 CodeRabbit inference engine (.cursor/rules/test-standards.mdc)
Integration tests must reside under test/integration/
Files:
test/integration/temporal/errors_test.gotest/integration/temporal/mode_switching_test.gotest/integration/temporal/startup_lifecycle_test.gotest/integration/temporal/persistence_test.gotest/integration/temporal/standalone_test.go
test/integration/**/*.go
📄 CodeRabbit inference engine (.cursor/rules/architecture.mdc)
Place integration tests under test/integration; keep them exercising cross-component behavior
Files:
test/integration/temporal/errors_test.gotest/integration/temporal/mode_switching_test.gotest/integration/temporal/startup_lifecycle_test.gotest/integration/temporal/persistence_test.gotest/integration/temporal/standalone_test.go
pkg/config/config.go
📄 CodeRabbit inference engine (.cursor/rules/global-config.mdc)
pkg/config/config.go: Add the new field to the typed config struct with tags: koanf/json/yaml/mapstructure; include env tag when applicable; add validate tag if needed; use SensitiveString or sensitive:"true" for secrets
Map from registry to typed struct in buildConfig using getString/getInt/getBool/getDuration/getInt64/getStringSlice
For secrets, prefer SensitiveString type or sensitive:"true" tag to enable automatic JSON redaction
When creating a new category, define typeConfig struct with full tagging and validation
Add the new section to the root Config with koanf/json/yaml/mapstructure tags
Implement buildConfig(registry *definition.Registry) to map defaults from the registry to the typed struct
Always include koanf, json, yaml, and mapstructure tags on config struct fields; include env when applicable
Files:
pkg/config/config.go
{pkg/config/config.go,pkg/config/definition/schema.go}
📄 CodeRabbit inference engine (.cursor/rules/global-config.mdc)
Environment variable names must be UPPER_SNAKE_CASE with domain prefixes (e.g., SERVER_, DB_, RUNTIME_, CLI uses COMPOZY_)
Files:
pkg/config/config.gopkg/config/definition/schema.go
docs/**/*.{ts,tsx,js,jsx,mjs,cjs}
📄 CodeRabbit inference engine (docs/.cursor/rules/bunjs.mdc)
docs/**/*.{ts,tsx,js,jsx,mjs,cjs}: Use Bun's native APIs for file operations, HTTP server, subprocess management, and JSON parsing
Use native APIs instead of polyfills for performance
Files:
docs/source.config.ts
docs/**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (docs/.cursor/rules/nextjs.mdc)
docs/**/*.{js,jsx,ts,tsx}: Use early returns whenever possible to make the code more readable.
Use descriptive variable and function/const names. Also, event functions should be named with a “handle” prefix, like “handleClick” for onClick and “handleKeyDown” for onKeyDown.
Use consts instead of functions, for example, “const toggle = () =>”. Also, define a type if possible.
Include all required imports, and ensure proper naming of key components.
Files:
docs/source.config.ts
docs/**/*.{ts,tsx}
📄 CodeRabbit inference engine (docs/.cursor/rules/react.mdc)
docs/**/*.{ts,tsx}: Use functional components exclusively - class components are legacy
Keep components small, focused on single responsibility
Use theuse()hook for unwrapping promises in components (React 19+)
Adopt Actions for form submissions and data mutations (React 19+)
LeverageuseFormStatus()anduseFormState()for form handling (React 19+)
ImplementuseOptimistic()for instant UI updates with rollback (React 19+)
Don't use React.FC - type props directly on the function
Use React.ComponentProps<'element'> for extending HTML elements
Leverage const type parameters for better type inference
Type your custom hooks' inputs and outputs explicitly
Implement code splitting with React.lazy() and dynamic imports
Use React.memo sparingly and only after profiling
Implement virtualization for long lists (TanStack Virtual)
Code-split at route level with TanStack Router
Preload critical resources with React 19 asset loading
Use Web Workers for heavy computations
Use TanStack Query for all server state management
Implement optimistic updates for better UX
Use proper loading states with Suspense boundaries
Handle errors with Error Boundaries at strategic levels
Prefetch data on route/interaction predictions
Use React 19 Actions for form submissions
Integrate @tanstack/react-form with zod for complex forms
Always provide proper validation and error messages in forms
Use Error Boundaries to catch and display errors gracefully
Implement fallback UI for error states
Log errors to monitoring service (Sentry, etc.)
Type your errors properly - never assume Error type
Sanitize user inputs to prevent XSS
Use environment variables for sensitive data
Implement proper authentication/authorization
Validate data on both client and server
Compound components for flexible APIs
Render props sparingly, prefer composition
Use portals for modals and tooltips
Implement progressive enhancement
docs/**/*.{ts,tsx}: Never remove Radix UI's accessibility attributes from components.
Te...
Files:
docs/source.config.ts
docs/**/*.{ts,tsx,mdx}
📄 CodeRabbit inference engine (docs/.cursor/rules/shadcn.mdc)
docs/**/*.{ts,tsx,mdx}: Always use design system tokens (e.g., bg-background, text-foreground, border-border, bg-primary, text-primary-foreground) instead of explicit color values (e.g., bg-white, text-black, border-gray-200, bg-blue-500, text-green-400) for theme switching.
Use required design tokens for common use cases: backgrounds (bg-background, bg-card, bg-muted, bg-popover), text (text-foreground, text-muted-foreground, text-card-foreground), borders (border-border, border-input, border-ring), actions (bg-primary text-primary-foreground, bg-secondary text-secondary-foreground), and states (bg-destructive text-destructive-foreground, bg-accent text-accent-foreground).
docs/**/*.{ts,tsx,mdx}: Always use design system tokens (e.g., bg-background, text-foreground, border-border, bg-primary, text-primary-foreground) instead of explicit color values (e.g., bg-white, text-black, border-gray-200, bg-blue-500, text-green-400) to ensure theme switching works correctly across light and dark modes.
Order Tailwind utility classes systematically: Layout → Box Model → Typography → Visual → Misc, and use consistent ordering across components.
Use Tailwind CSS v4 modern utilities: size-* for squares, dynamic values without brackets (e.g., grid-cols-15), text-shadow-, mask-, @container for container queries, and 3D transforms like rotate-x-* and scale-z-*.
Use a mobile-first responsive strategy, leverage container queries for component-level responsiveness, and use responsive variants consistently.
Use 'class' or 'media' strategy for dark mode, apply dark: variants thoughtfully, and test both themes during development.
Use CSS variables in parentheses for arbitrary values in Tailwind CSS v4 (e.g., bg-(--custom-color)), and prefer built-in dynamic utilities where possible.
Use Tailwind's built-in animations and extend with custom keyframes when needed; keep animations performant and use new v4 features like gradient animations with @Property.
Use new Tailwind CSS v4 feat...
Files:
docs/source.config.ts
engine/infra/**/*.go
📄 CodeRabbit inference engine (.cursor/rules/architecture.mdc)
engine/infra/**/*.go: Infrastructure Layer (engine/infra): implement adapters for DB, cache, HTTP, storage, monitoring; must implement Application Layer interfaces
Adapter implementations belong in Infrastructure Layer and must implement Application Layer interfaces (no business logic leakage)
Infrastructure depends inward: Infrastructure → Application → Domain; avoid imports from cli and cross-infra coupling
Files:
engine/infra/server/dependencies.go
pkg/config/definition/schema.go
📄 CodeRabbit inference engine (.cursor/rules/global-config.mdc)
pkg/config/definition/schema.go: Use pkg/config/definition/schema.go as the single source of truth for configuration by registering fields in the registry
Add new property by registering it with registry.Register(&FieldDef{Path, Default, CLIFlag, EnvVar, Type, Help}); keep path namingcategory.property_name(snake_case)
Register all fields of a new category in registerFields and include it in CreateRegistry()
Config path format must be section.snake_case_property when registering fields
CLI flags in registry must use kebab-case (CLIFlag)
Durations should use time.Duration and defaults in the registry must be time.Duration values
Files:
pkg/config/definition/schema.go
pkg/config/**/*_test.go
📄 CodeRabbit inference engine (.cursor/rules/global-config.mdc)
Add or extend tests under pkg/config/*_test.go when introducing validation or complex defaults
Files:
pkg/config/config_test.go
pkg/config/loader.go
📄 CodeRabbit inference engine (.cursor/rules/global-config.mdc)
pkg/config/loader.go: Implement cross-field validation in validateCustom when struct tag validation is insufficient
Extend validateCustom for cross-field constraints in new categories when struct tags are insufficient
Files:
pkg/config/loader.go
🧬 Code graph analysis (17)
engine/worker/embedded/ui_test.go (1)
pkg/logger/mod.go (2)
ContextWithLogger(39-41)NewForTests(199-201)
engine/worker/embedded/namespace_test.go (1)
engine/worker/embedded/config.go (1)
Config(34-63)
pkg/config/provider.go (2)
cli/tui/models/base.go (1)
Mode(10-10)pkg/logger/mod.go (1)
LogLevel(13-13)
engine/worker/embedded/server_test.go (2)
pkg/logger/mod.go (2)
ContextWithLogger(39-41)NewForTests(199-201)engine/worker/embedded/server.go (2)
NewServer(41-82)Server(30-37)
engine/worker/embedded/config.go (1)
pkg/logger/mod.go (1)
LogLevel(13-13)
test/integration/temporal/errors_test.go (1)
cli/helpers/io.go (1)
WriteFile(105-117)
engine/worker/embedded/builder_test.go (1)
engine/worker/embedded/config.go (1)
Config(34-63)
test/integration/temporal/mode_switching_test.go (1)
pkg/config/config.go (1)
StandaloneConfig(442-471)
engine/worker/embedded/server.go (1)
engine/worker/embedded/ui.go (1)
UIServer(18-26)
engine/worker/embedded/namespace.go (2)
engine/worker/embedded/config.go (1)
Config(34-63)engine/core/copy.go (1)
CloneMap(29-34)
engine/worker/embedded/builder.go (2)
engine/worker/embedded/config.go (1)
Config(34-63)engine/core/copy.go (1)
CloneMap(29-34)
engine/infra/server/dependencies.go (2)
pkg/config/config.go (1)
Config(46-130)engine/infra/server/server.go (1)
Server(31-61)
engine/worker/embedded/ui.go (1)
engine/worker/embedded/server.go (2)
Server(30-37)NewServer(41-82)
pkg/config/definition/schema.go (1)
pkg/config/definition/field.go (2)
Registry(21-23)FieldDef(10-18)
pkg/config/config_test.go (4)
pkg/config/manager.go (1)
NewManager(32-45)pkg/config/loader.go (1)
NewService(49-62)pkg/config/provider.go (2)
NewDefaultProvider(233-237)NewCLIProvider(51-55)pkg/config/config.go (2)
Default(1828-1831)TemporalConfig(401-436)
engine/worker/embedded/config_test.go (1)
engine/worker/embedded/config.go (1)
Config(34-63)
pkg/config/loader.go (1)
pkg/config/config.go (2)
Config(46-130)StandaloneConfig(442-471)
🪛 checkmake (0.2.2)
examples/temporal-standalone/integration-testing/Makefile
[warning] 1-1: Missing required phony target "all"
(minphony)
[warning] 1-1: Missing required phony target "clean"
(minphony)
🪛 OSV Scanner (2.2.3)
go.mod
[LOW] 124-124: github.com/aws/aws-sdk-go 1.55.6: In-band key negotiation issue in AWS S3 Crypto SDK for golang in github.com/aws/aws-sdk-go
(GO-2022-0635)
[LOW] 124-124: github.com/aws/aws-sdk-go 1.55.6: CBC padding oracle issue in AWS S3 Crypto SDK for golang in github.com/aws/aws-sdk-go
(GO-2022-0646)
[HIGH] 275-275: github.com/quic-go/quic-go 0.54.0: quic-go: Panic occurs when queuing undecryptable packets after handshake completion
⏰ 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: Pre-commit
- GitHub Check: Security Scan (gosec)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (go)
🔇 Additional comments (34)
examples/temporal-standalone/persistent/.gitignore (1)
1-1: LGTM!The
.gitignoreentry correctly excludes thedata/directory, ensuring that runtime artifacts and transient data from the standalone Temporal deployment are not committed to version control. This is appropriate for a development/testing example directory.scripts/markdown/check.go (6)
3593-3617: LGTM: Activity monitor implementation is clean and thread-safe.The
activityMonitortype is well-designed with:
- Proper mutex protection for concurrent access
- Clear, focused responsibility (tracking last activity time)
- Simple, maintainable implementation
1658-1706: LGTM: Command execution with timeout is well-structured.The timeout handling logic is correct:
- Activity watchdog monitors for inactivity
- Proper coordination via channels and select statement
- Graceful handling of timeout vs cancellation vs completion
- Function length is within the 50-line guideline
Note: The exit code -2 on line 1699 should be replaced with the named constant suggested in the earlier comment.
3619-3673: LGTM: Activity monitoring is correctly integrated into I/O taps.The monitor integration is well-designed:
- Both
uiLogTapandjsonFormatterrecord activity when data is written- Proper nil checks before recording activity
- Non-invasive changes that maintain existing functionality
- Consistent pattern across both writer types
This ensures that the activity-based timeout detection works correctly by resetting the timer whenever the process produces output.
Also applies to: 3675-3716
3190-3290: LGTM: PRD task prompt generation is well-structured.The prompt building logic is clean and maintainable:
- Functions are properly sized (all under 50 lines)
- Clear separation of concerns (context, critical execution, implementation, completion)
- Comprehensive instructions for task completion
- Good use of helper functions to build sections
The generated prompts provide clear guidance with completion criteria and validation requirements.
570-572: Context usage is appropriate for a CLI script.The
context.Background()calls at lines 570, 1068, and 2828 are correctly used:
- Line 570: Entry point for the CLI command (signal context created from Background)
- Line 1068: Independent timeout context for graceful shutdown
- Line 2828: One-time validation check with its own timeout
These are appropriate for a CLI script. The coding guideline about avoiding
context.Background()in runtime code paths applies to the main application server/worker code, not CLI scripts.Also applies to: 1068-1069, 2828-2829
2985-2985: LGTM: Task file regex pattern is correct.The
reTaskFileregex pattern^_task_\d+\.md$correctly matches task files like_task_001.md,_task_123.md, etc. It's used consistently for filtering task files.examples/temporal-standalone/integration-testing/.env.example (1)
1-1: Standard environment placeholder.examples/temporal-standalone/integration-testing/Makefile (1)
1-5: Makefile is fit for purpose.This specialized test runner correctly uses build tags and target patterns. The static analysis warnings about "all" and "clean" targets are not applicable to focused integration test runners.
engine/worker/embedded/builder_test.go (1)
13-126: LGTM! Comprehensive test coverage for embedded builder.The test suite effectively validates:
- Temporal config construction with all key fields (persistence, ports, services, cluster metadata)
- Static host mapping for all services
- SQLite connection attributes for both in-memory and file-backed modes
- Error handling for invalid port ranges
- Cluster metadata population
The tests are well-structured with proper use of
t.Parallel()and focused assertions.pkg/config/config.go (2)
402-436: LGTM! Well-structured Temporal mode and standalone configuration.The new fields are properly designed:
Modefield with validation ensures only "remote" or "standalone" valuesStandaloneConfigprovides comprehensive embedded server options- All fields have proper struct tags (koanf, env, json, yaml, mapstructure)
- Environment variables follow consistent
TEMPORAL_STANDALONE_*naming- Documentation clearly explains each field's purpose
This aligns well with the embedded Temporal server implementation.
2181-2198: LGTM! Config builder correctly populates standalone fields.The
buildTemporalConfigfunction properly:
- Retrieves the Mode field from registry
- Constructs the complete StandaloneConfig with all fields
- Uses type-safe helper functions (getString, getInt, getBool, getDuration)
pkg/config/provider.go (1)
366-381: LGTM! Temporal defaults properly extended with mode and standalone configuration.The changes correctly:
- Add the
modefield at the top level- Create a nested
standaloneobject with all StandaloneConfig fields- Use
.String()conversion for the duration field (start_timeout)- Maintain consistency with the struct definition in config.go
This ensures standalone configuration defaults are properly propagated through the config system.
cli/helpers/flag_categories.go (1)
176-178: Review comment is incorrect; all exposed CLI flags are already included.The config schema in
pkg/config/definition/schema.goshows that only 3 of 9 temporal-standalone configuration fields have CLI flag definitions:
temporal-standalone-database✓temporal-standalone-frontend-port✓temporal-standalone-ui-port✓The remaining fields (
bind_ip,namespace,cluster_name,enable_ui,log_level,start_timeout) are not exposed as CLI flags—they are accessible only via environment variables. The flag_categories.go correctly lists only the flags that actually exist.docs/source.config.ts (1)
15-31: All documentation URLs are valid and reference existing files.Verification confirms that all three hardcoded URLs point to actual documentation pages:
- ✓
/docs/deployment/temporal-modes→docs/content/docs/deployment/temporal-modes.mdx- ✓
/docs/architecture/embedded-temporal→docs/content/docs/architecture/embedded-temporal.mdx- ✓
/docs/troubleshooting/temporal→docs/content/docs/troubleshooting/temporal.mdxNo changes required; the navigation links are correctly configured.
go.mod (1)
78-78: The langchaingo downgrade to v0.1.13 appears intentional and is justified by codebase constraints.The codebase explicitly references v0.1.13 in a code comment: "OpenAI Chat Completions (via langchaingo v0.1.13) does not expose an input_audio content part type". This indicates the project deliberately pins to v0.1.13 to handle audio/video BinaryParts consistently across providers without triggering 400 errors.
The downgrade also avoids v0.1.14's changes to OpenAI token-field handling (max_tokens vs max_completion_tokens), which could break compatibility with older or self-hosted OpenAI-compatible endpoints—a practical concern given the project's heavy use of langchaingo for OpenAI integration across multiple adapter and embedding modules.
engine/infra/server/dependencies.go (3)
22-22: Layering check: importing embedded server is acceptable hereInfra depending inward on the application-layer embedded server is aligned with our layering rules; no issues.
187-192: Startup order is correct; cleanup properly chainedStarting standalone Temporal before constructing Temporal client config and appending its cleanup ensures worker wiring sees the right HostPort and shutdown is LIFO.
363-376: Config adapter is clear and minimalMapping Standalone → embedded.Config is straightforward and SRP-compliant. LGTM.
engine/worker/embedded/server_test.go (2)
19-43: Constructor tests are comprehensive and preciseValid and invalid config paths are well exercised; assertions are minimal and targeted. LGTM.
121-169: Helpful test helpers; port allocator guards contiguous rangeHelper patterns are clean and reuse reduces duplication. LGTM.
engine/worker/embedded/namespace_test.go (1)
16-58: Good end-to-end namespace creation and idempotence checkValidation, creation, and direct DB assertion give strong coverage. Cleanup handled. LGTM.
examples/temporal-standalone/integration-testing/tests/integration_test.go (1)
18-44: Lifecycle smoke test is concise and effectiveStarts embedded server, asserts address, and ensures graceful stop. Build tag scoping is correct. LGTM.
test/integration/temporal/mode_switching_test.go (1)
44-59: Mapping looks correct.toEmbeddedConfig copies all expected fields (incl. StartTimeout). LGTM.
pkg/config/config_test.go (1)
48-62: Defaults assertions for Temporal.Standalone look correct.Values match schema defaults; good coverage.
test/integration/temporal/standalone_test.go (1)
209-219: Lifecycle management LGTM.Client dial/close and worker start/stop are handled correctly via require/defer.
Also applies to: 221-232
pkg/config/definition/schema.go (1)
930-934: Temporal schema refactor and standalone fields look good.
- Clear split of core vs standalone fields.
- Defaults/CLI/env names consistent and match tests.
LGTM.Also applies to: 936-969, 971-1010, 1011-1041
engine/worker/embedded/ui.go (1)
139-156: No action required—all referenced symbols are properly defined in-package.The constants
readyDialTimeoutandreadyPollIntervalare defined inengine/worker/embedded/server.go(lines 20–21) as typedtime.Durationvalues. Error variableserrNilContextanderrAlreadyStartedare defined at lines 25–26, and the helper functionisConnRefused()is implemented at line 243. All are correctly referenced and used throughoutui.go.engine/worker/embedded/builder.go (1)
125-137: LGTM! Proper use of core.CloneMap for map operations.The use of
core.CloneMapon line 126 correctly follows the project's map operation guidelines. The function structure is clean and focused.test/integration/temporal/startup_lifecycle_test.go (2)
115-124: LGTM! Correct use of Go 1.25+ sync.WaitGroup.Go.Line 118 properly uses
wg.Go(func())which is the preferred pattern in Go 1.25+ over manualAdd/go/Donesequencing.
202-221: LGTM! Proper Temporal context separation.The workflow function correctly uses
workflow.Context(lines 203-211) while the activity function usescontext.Context(lines 214-221). The activity also properly handles context cancellation with a select statement.engine/worker/embedded/server.go (3)
41-54: Remove workaround after fixing config.go EnableUI bug.Lines 45-54 implement a workaround for the inverted
EnableUIdefault logic bug inconfig.go(lines 93-95). Once that bug is fixed, this workaround should be removed for cleaner code.This comment is contingent on addressing the critical issue raised in
config.go.
112-157: LGTM! Robust Start implementation with proper error handling.The Start method correctly:
- Validates context and prevents duplicate starts with thread-safe checks
- Uses configured timeout for startup
- Implements proper cleanup on readiness failure (lines 132-138)
- Handles UI server startup as best-effort with warnings (lines 150-154)
223-241: LGTM! Proactive port conflict detection.The
ensurePortsAvailablefunction provides excellent user experience by detecting port conflicts early and providing a clear, actionable error message before attempting to start the Temporal server.
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
- Issue 1: Add comment explaining type assertion in docs/source.config.ts - Issue 2 & 3: Refactor maybeStartStandaloneTemporal to fetch config from context and simplify standaloneTemporalCleanup signature - Issue 4: Rename test cases to 'Should ...' naming convention - Issue 5: Remove inverted EnableUI default logic and simplify server.go workaround All changes follow project coding standards and pass lint + tests.
Issue 6: Add context to createNamespace, use logger, make idempotent - Added context.Context parameter to createNamespace function - Retrieved logger from context for debug/error messages - Made namespace creation idempotent by treating 'already exists' as success - Updated all call sites to pass context Issue 7: Reduce dial timeout to 500ms in server_test.go - Changed successful-start test dial timeout from 1s to 500ms - Improves CI feedback speed on slow/no-response cases Issue 8: Add timeout wrapper in ui_test.go Start call - Wrapped ui.Start() with 5-second timeout context - Prevents test hangs in port conflict scenarios Issue 9: Store bindIP/uiPort in UIServer struct - Added bindIP and uiPort fields to UIServer struct - Populated fields in constructor - Updated Start method to use stored fields instead of config reads - Eliminates runtime config reads per coding guidelines Issue 10: Fix race condition in UIServer.Start - Set runErrCh under lock before starting goroutine - Prevents double-start race where two callers could both pass checks - Clear runErrCh on failure to maintain correct state - Moved started flag setting to after successful readiness check All tests passing, linting clean.
- Issue 11: Make runErrCh drain non-blocking in embedded UI server to prevent edge-case hangs - Issue 12: Standardize .env.example comments across all temporal-standalone examples - Issue 13: Simplify validation using Number.isFinite in debugging tools - Issue 14: Add documentation comment for findOpenPortRange explaining 4-port requirement - Issue 15: Verified go.temporal.io/api v1.53.0 is correct (not v1.34.0 as suggested) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
- Issue 16: Document AWS SDK low-severity vulnerabilities in go.mod - Added inline comment documenting acceptance of GO-2022-0635 and GO-2022-0646 - Verified codebase does not use S3 client-side encryption - Issue 17: Upgrade quic-go to v0.54.1 to fix HIGH severity vulnerability - Fixed GHSA-47m2-4cr7-mhcw panic vulnerability - Issue 18: Add test case for UI disabled with zero port - Added test validating EnableUI=false allows UIPort=0 - Updated test logic to handle both error and no-error cases - Issue 19: Replace magic numbers and add port collision check - Introduced maxTCPPort and temporalServiceSpan constants - Added collision check preventing UIPort in reserved service range - Issue 20: Extract default timeout constant in scripts/markdown/check.go - Created defaultActivityTimeout constant for consistent timeout handling
- Issue 21: Define exit code constants (exitCodeTimeout=-2, exitCodeCanceled=-1) - Issue 22: Extract activity check interval (5s) to named constant - Issue 23: Extract process termination grace period (5s) to named constant - Issue 24: Add t.Cleanup for server cleanup in errors_test.go - Issue 25: Wrap test behaviors in t.Run subtest in mode_switching_test.go All constants now defined at package level for better maintainability. Test cleanup ensures proper resource management even on early failures. Subtest structure follows project testing conventions. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
- Issue 26: Add explicit namespace assignment in persistence test restart config - Issue 27: Document +1000 port offset probe for Temporal UI availability - Issue 30: Extract magic numbers to named constants in markdown check script - Add gracefulShutdownTimeout, uiMessageDrainDelay, uiTickInterval constants - Add retry parameter validation (maxRetries >= 0, retryBackoffMultiplier > 0) - Issue 31: Fix Temporal mode handling with proper constant usage - Add temporalModeStandalone constant instead of reusing mcpProxyModeStandalone - Require host_port only in remote mode, allow derived value in standalone mode - Trim mode whitespace for consistent validation All tests and linting pass.
There was a problem hiding this comment.
Actionable comments posted: 18
♻️ Duplicate comments (8)
docs/source.config.ts (1)
58-59: Comment addresses previous feedback, but module augmentation remains the better solution.The added comment on line 58 addresses the previous review suggestion to document why the type assertion is necessary. However, TypeScript module augmentation would provide compile-time type safety rather than runtime assignment. Consider declaring a module augmentation block:
declare module 'fumadocs-mdx/config' { interface Config { navigationLinks?: NavigationLink[]; } }Then cast once with full type safety:
-// fumadocs-mdx does not expose navigationLinks in the config type; attach via runtime assignment -(config as { navigationLinks?: typeof navigationLinks }).navigationLinks = navigationLinks; +const augmentedConfig: Config & { navigationLinks: NavigationLink[] } = { + ...config, + navigationLinks, +}; + +export default augmentedConfig;scripts/markdown/check.go (1)
1868-1877: Use the named constant instead of literal value.Line 1874 uses the literal
5 * time.Secondinstead of theprocessTerminationGracePeriodconstant defined at the top of the file. For consistency and maintainability, use the constant.Apply this diff:
select { case <-cmdDone: if !useUI { fmt.Fprintf(os.Stderr, "Job %d terminated gracefully after timeout\n", index+1) } - case <-time.After(5 * time.Second): + case <-time.After(processTerminationGracePeriod): forceKillProcess(cmd, index, useUI) }test/integration/temporal/persistence_test.go (1)
38-39: Namespace pin on restart addressed.Explicitly setting restartCfg.Namespace ensures describe targets the correct namespace across restarts. Good follow‑through on the earlier suggestion.
examples/temporal-standalone/integration-testing/tests/integration_test.go (1)
46-48: Nice: documented 4‑port assumption for embedded services.Clear and sufficient for future maintainers; aligns with prior suggestion.
engine/worker/embedded/ui_test.go (1)
48-51: Start/Stop guarded by timeouts; lifecycle test is robust.Prevents hangs and validates readiness and teardown. Good use of t.Context() and short client timeouts.
Also applies to: 60-67
test/integration/temporal/mode_switching_test.go (1)
29-45: Good: subtest style adopted.Wrapping behavior in t.Run aligns with project test conventions.
engine/worker/embedded/server_test.go (1)
46-61: Faster ready check is good.Reduced dial timeout to 500ms improves CI feedback without flakiness.
go.mod (1)
87-89: Temporal API ↔ Server version mismatch; fix before merge.Still pinned to go.temporal.io/api v1.53.0 while using go.temporal.io/server v1.29.0. This pairing is likely incompatible and can break protobuf/gRPC contracts during build/runtime. Align API to the server’s supported version (previous review: v1.34.0) and run go mod tidy.
Apply:
- go.temporal.io/api v1.53.0 + go.temporal.io/api v1.34.0What go.temporal.io/api version matches go.temporal.io/server v1.29.0 compatibility?
📜 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 (16)
docs/native-tools.mdis excluded by!**/*.mdgo.sumis excluded by!**/*.sumtasks/prd-postgres/_docs.mdis excluded by!**/*.mdtasks/prd-postgres/_examples.mdis excluded by!**/*.mdtasks/prd-postgres/_prd.mdis excluded by!**/*.mdtasks/prd-postgres/_task_1.mdis excluded by!**/*.mdtasks/prd-postgres/_task_2.mdis excluded by!**/*.mdtasks/prd-postgres/_task_3.mdis excluded by!**/*.mdtasks/prd-postgres/_task_4.mdis excluded by!**/*.mdtasks/prd-postgres/_task_5.mdis excluded by!**/*.mdtasks/prd-postgres/_task_6.mdis excluded by!**/*.mdtasks/prd-postgres/_task_7.mdis excluded by!**/*.mdtasks/prd-postgres/_task_8.mdis excluded by!**/*.mdtasks/prd-postgres/_tasks.mdis excluded by!**/*.mdtasks/prd-postgres/_techspec.mdis excluded by!**/*.mdtasks/prd-postgres/_tests.mdis excluded by!**/*.md
📒 Files selected for processing (27)
docs/source.config.ts(2 hunks)engine/infra/server/dependencies.go(3 hunks)engine/worker/embedded/config.go(1 hunks)engine/worker/embedded/config_test.go(1 hunks)engine/worker/embedded/namespace.go(1 hunks)engine/worker/embedded/namespace_test.go(1 hunks)engine/worker/embedded/server.go(1 hunks)engine/worker/embedded/server_test.go(1 hunks)engine/worker/embedded/ui.go(1 hunks)engine/worker/embedded/ui_test.go(1 hunks)examples/temporal-standalone/custom-ports/.env.example(1 hunks)examples/temporal-standalone/debugging/.env.example(1 hunks)examples/temporal-standalone/debugging/tools/index.ts(1 hunks)examples/temporal-standalone/integration-testing/.env.example(1 hunks)examples/temporal-standalone/integration-testing/tests/integration_test.go(1 hunks)examples/temporal-standalone/migration-from-remote/.env.example(1 hunks)examples/temporal-standalone/no-ui/.env.example(1 hunks)examples/temporal-standalone/persistent/.env.example(1 hunks)go.mod(9 hunks)pkg/config/config_test.go(3 hunks)pkg/config/loader.go(3 hunks)scripts/markdown/check.go(41 hunks)test/integration/temporal/errors_test.go(1 hunks)test/integration/temporal/mode_switching_test.go(1 hunks)test/integration/temporal/persistence_test.go(1 hunks)test/integration/temporal/standalone_test.go(1 hunks)test/integration/temporal/startup_lifecycle_test.go(1 hunks)
🧰 Additional context used
📓 Path-based instructions (16)
**/*.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/worker/embedded/config.gotest/integration/temporal/persistence_test.goengine/worker/embedded/ui_test.gotest/integration/temporal/standalone_test.goengine/worker/embedded/ui.gotest/integration/temporal/startup_lifecycle_test.goengine/worker/embedded/config_test.goengine/worker/embedded/namespace.gotest/integration/temporal/errors_test.goexamples/temporal-standalone/integration-testing/tests/integration_test.goengine/worker/embedded/server_test.gopkg/config/loader.goengine/worker/embedded/server.gopkg/config/config_test.gotest/integration/temporal/mode_switching_test.goengine/worker/embedded/namespace_test.goengine/infra/server/dependencies.goscripts/markdown/check.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/config.goengine/worker/embedded/ui.goengine/worker/embedded/namespace.gopkg/config/loader.goengine/worker/embedded/server.goengine/infra/server/dependencies.goscripts/markdown/check.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/worker/embedded/config.goengine/worker/embedded/ui_test.goengine/worker/embedded/ui.goengine/worker/embedded/config_test.goengine/worker/embedded/namespace.goengine/worker/embedded/server_test.goengine/worker/embedded/server.goengine/worker/embedded/namespace_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/worker/embedded/config.goengine/worker/embedded/ui_test.goengine/worker/embedded/ui.goengine/worker/embedded/config_test.goengine/worker/embedded/namespace.goengine/worker/embedded/server_test.gopkg/config/loader.goengine/worker/embedded/server.gopkg/config/config_test.goengine/worker/embedded/namespace_test.goengine/infra/server/dependencies.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:
test/integration/temporal/persistence_test.goengine/worker/embedded/ui_test.gotest/integration/temporal/standalone_test.gotest/integration/temporal/startup_lifecycle_test.goengine/worker/embedded/config_test.gotest/integration/temporal/errors_test.goexamples/temporal-standalone/integration-testing/tests/integration_test.goengine/worker/embedded/server_test.gopkg/config/config_test.gotest/integration/temporal/mode_switching_test.goengine/worker/embedded/namespace_test.go
test/integration/**/*_test.go
📄 CodeRabbit inference engine (.cursor/rules/test-standards.mdc)
Integration tests must reside under test/integration/
Files:
test/integration/temporal/persistence_test.gotest/integration/temporal/standalone_test.gotest/integration/temporal/startup_lifecycle_test.gotest/integration/temporal/errors_test.gotest/integration/temporal/mode_switching_test.go
test/integration/**/*.go
📄 CodeRabbit inference engine (.cursor/rules/architecture.mdc)
Place integration tests under test/integration; keep them exercising cross-component behavior
Files:
test/integration/temporal/persistence_test.gotest/integration/temporal/standalone_test.gotest/integration/temporal/startup_lifecycle_test.gotest/integration/temporal/errors_test.gotest/integration/temporal/mode_switching_test.go
docs/**/*.{ts,tsx,js,jsx,mjs,cjs}
📄 CodeRabbit inference engine (docs/.cursor/rules/bunjs.mdc)
docs/**/*.{ts,tsx,js,jsx,mjs,cjs}: Use Bun's native APIs for file operations, HTTP server, subprocess management, and JSON parsing
Use native APIs instead of polyfills for performance
Files:
docs/source.config.ts
docs/**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (docs/.cursor/rules/nextjs.mdc)
docs/**/*.{js,jsx,ts,tsx}: Use early returns whenever possible to make the code more readable.
Use descriptive variable and function/const names. Also, event functions should be named with a “handle” prefix, like “handleClick” for onClick and “handleKeyDown” for onKeyDown.
Use consts instead of functions, for example, “const toggle = () =>”. Also, define a type if possible.
Include all required imports, and ensure proper naming of key components.
Files:
docs/source.config.ts
docs/**/*.{ts,tsx}
📄 CodeRabbit inference engine (docs/.cursor/rules/react.mdc)
docs/**/*.{ts,tsx}: Use functional components exclusively - class components are legacy
Keep components small, focused on single responsibility
Use theuse()hook for unwrapping promises in components (React 19+)
Adopt Actions for form submissions and data mutations (React 19+)
LeverageuseFormStatus()anduseFormState()for form handling (React 19+)
ImplementuseOptimistic()for instant UI updates with rollback (React 19+)
Don't use React.FC - type props directly on the function
Use React.ComponentProps<'element'> for extending HTML elements
Leverage const type parameters for better type inference
Type your custom hooks' inputs and outputs explicitly
Implement code splitting with React.lazy() and dynamic imports
Use React.memo sparingly and only after profiling
Implement virtualization for long lists (TanStack Virtual)
Code-split at route level with TanStack Router
Preload critical resources with React 19 asset loading
Use Web Workers for heavy computations
Use TanStack Query for all server state management
Implement optimistic updates for better UX
Use proper loading states with Suspense boundaries
Handle errors with Error Boundaries at strategic levels
Prefetch data on route/interaction predictions
Use React 19 Actions for form submissions
Integrate @tanstack/react-form with zod for complex forms
Always provide proper validation and error messages in forms
Use Error Boundaries to catch and display errors gracefully
Implement fallback UI for error states
Log errors to monitoring service (Sentry, etc.)
Type your errors properly - never assume Error type
Sanitize user inputs to prevent XSS
Use environment variables for sensitive data
Implement proper authentication/authorization
Validate data on both client and server
Compound components for flexible APIs
Render props sparingly, prefer composition
Use portals for modals and tooltips
Implement progressive enhancement
docs/**/*.{ts,tsx}: Never remove Radix UI's accessibility attributes from components.
Te...
Files:
docs/source.config.ts
docs/**/*.{ts,tsx,mdx}
📄 CodeRabbit inference engine (docs/.cursor/rules/shadcn.mdc)
docs/**/*.{ts,tsx,mdx}: Always use design system tokens (e.g., bg-background, text-foreground, border-border, bg-primary, text-primary-foreground) instead of explicit color values (e.g., bg-white, text-black, border-gray-200, bg-blue-500, text-green-400) for theme switching.
Use required design tokens for common use cases: backgrounds (bg-background, bg-card, bg-muted, bg-popover), text (text-foreground, text-muted-foreground, text-card-foreground), borders (border-border, border-input, border-ring), actions (bg-primary text-primary-foreground, bg-secondary text-secondary-foreground), and states (bg-destructive text-destructive-foreground, bg-accent text-accent-foreground).
docs/**/*.{ts,tsx,mdx}: Always use design system tokens (e.g., bg-background, text-foreground, border-border, bg-primary, text-primary-foreground) instead of explicit color values (e.g., bg-white, text-black, border-gray-200, bg-blue-500, text-green-400) to ensure theme switching works correctly across light and dark modes.
Order Tailwind utility classes systematically: Layout → Box Model → Typography → Visual → Misc, and use consistent ordering across components.
Use Tailwind CSS v4 modern utilities: size-* for squares, dynamic values without brackets (e.g., grid-cols-15), text-shadow-, mask-, @container for container queries, and 3D transforms like rotate-x-* and scale-z-*.
Use a mobile-first responsive strategy, leverage container queries for component-level responsiveness, and use responsive variants consistently.
Use 'class' or 'media' strategy for dark mode, apply dark: variants thoughtfully, and test both themes during development.
Use CSS variables in parentheses for arbitrary values in Tailwind CSS v4 (e.g., bg-(--custom-color)), and prefer built-in dynamic utilities where possible.
Use Tailwind's built-in animations and extend with custom keyframes when needed; keep animations performant and use new v4 features like gradient animations with @Property.
Use new Tailwind CSS v4 feat...
Files:
docs/source.config.ts
pkg/config/loader.go
📄 CodeRabbit inference engine (.cursor/rules/global-config.mdc)
pkg/config/loader.go: Implement cross-field validation in validateCustom when struct tag validation is insufficient
Extend validateCustom for cross-field constraints in new categories when struct tags are insufficient
Files:
pkg/config/loader.go
pkg/**/*.go
📄 CodeRabbit inference engine (.cursor/rules/architecture.mdc)
Shared Packages (pkg): provide framework-level utilities (config, logging, templates, schema generation); must not contain business logic
Files:
pkg/config/loader.gopkg/config/config_test.go
pkg/config/**/*.go
📄 CodeRabbit inference engine (.cursor/rules/architecture.mdc)
Global configuration resides in pkg/config; expose retrieval via context (config.FromContext) and provide defaults; avoid global singletons
Files:
pkg/config/loader.gopkg/config/config_test.go
pkg/config/**/*_test.go
📄 CodeRabbit inference engine (.cursor/rules/global-config.mdc)
Add or extend tests under pkg/config/*_test.go when introducing validation or complex defaults
Files:
pkg/config/config_test.go
engine/infra/**/*.go
📄 CodeRabbit inference engine (.cursor/rules/architecture.mdc)
engine/infra/**/*.go: Infrastructure Layer (engine/infra): implement adapters for DB, cache, HTTP, storage, monitoring; must implement Application Layer interfaces
Adapter implementations belong in Infrastructure Layer and must implement Application Layer interfaces (no business logic leakage)
Infrastructure depends inward: Infrastructure → Application → Domain; avoid imports from cli and cross-infra coupling
Files:
engine/infra/server/dependencies.go
🧬 Code graph analysis (13)
engine/worker/embedded/config.go (1)
pkg/logger/mod.go (1)
LogLevel(13-13)
engine/worker/embedded/ui_test.go (1)
pkg/logger/mod.go (2)
ContextWithLogger(39-41)NewForTests(199-201)
engine/worker/embedded/ui.go (1)
engine/worker/embedded/server.go (2)
Server(30-37)NewServer(41-76)
engine/worker/embedded/config_test.go (1)
engine/worker/embedded/config.go (1)
Config(34-63)
engine/worker/embedded/namespace.go (2)
engine/worker/embedded/config.go (1)
Config(34-63)engine/core/copy.go (1)
CloneMap(29-34)
examples/temporal-standalone/integration-testing/tests/integration_test.go (1)
pkg/logger/mod.go (1)
LogLevel(13-13)
engine/worker/embedded/server_test.go (2)
pkg/logger/mod.go (2)
ContextWithLogger(39-41)NewForTests(199-201)engine/worker/embedded/server.go (2)
NewServer(41-76)Server(30-37)
pkg/config/loader.go (1)
pkg/config/config.go (2)
Config(46-130)StandaloneConfig(442-471)
engine/worker/embedded/server.go (2)
engine/worker/embedded/config.go (1)
Config(34-63)engine/worker/embedded/ui.go (1)
UIServer(18-28)
pkg/config/config_test.go (4)
pkg/config/manager.go (1)
NewManager(32-45)pkg/config/loader.go (1)
NewService(55-68)pkg/config/provider.go (2)
NewDefaultProvider(233-237)NewCLIProvider(51-55)pkg/config/config.go (2)
Default(1828-1831)TemporalConfig(401-436)
test/integration/temporal/mode_switching_test.go (2)
cli/tui/models/base.go (1)
Mode(10-10)pkg/config/config.go (1)
StandaloneConfig(442-471)
engine/worker/embedded/namespace_test.go (1)
engine/worker/embedded/config.go (1)
Config(34-63)
engine/infra/server/dependencies.go (4)
engine/worker/embedded/server.go (2)
NewServer(41-76)Server(30-37)engine/infra/server/server.go (2)
NewServer(63-81)Server(31-61)engine/worker/embedded/config.go (1)
Config(34-63)pkg/config/config.go (1)
Config(46-130)
🪛 OSV Scanner (2.2.3)
go.mod
[LOW] 126-126: github.com/aws/aws-sdk-go 1.55.6: In-band key negotiation issue in AWS S3 Crypto SDK for golang in github.com/aws/aws-sdk-go
(GO-2022-0635)
[LOW] 126-126: github.com/aws/aws-sdk-go 1.55.6: CBC padding oracle issue in AWS S3 Crypto SDK for golang in github.com/aws/aws-sdk-go
(GO-2022-0646)
⏰ 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). (6)
- GitHub Check: test
- GitHub Check: Analyze (go)
- GitHub Check: Container Security Scan
- GitHub Check: Security Scan (gosec)
- GitHub Check: Test Coverage
- GitHub Check: Pre-commit
🔇 Additional comments (50)
docs/source.config.ts (2)
33-33: LGTM: Necessary refactor to enable runtime augmentation.Changing to a local const allows attaching
navigationLinksbefore export, which is the correct approach for this pattern.
17-29: No issues found — all referenced documentation pages exist.The verification confirms that all three documentation files referenced in the navigationLinks are present in the repository:
docs/content/docs/deployment/temporal-modes.mdxdocs/content/docs/architecture/embedded-temporal.mdxdocs/content/docs/troubleshooting/temporal.mdxThe navigation links are valid and will not result in broken links.
examples/temporal-standalone/debugging/tools/index.ts (1)
1-7: LGTM: Type definitions are clear and appropriate.The type definitions for
ValidationInputandValidationOutputare simple, well-structured, and serve their purpose for the validation utility.scripts/markdown/check.go (14)
34-56: LGTM! Constants properly extracted.The magic numbers flagged in previous reviews have been correctly extracted to named constants. This improves maintainability and addresses all the nitpick concerns from past reviews.
85-101: LGTM! New CLI arguments properly structured.The timeout, maxRetries, and retryBackoffMultiplier fields are appropriately typed and support the activity-based timeout feature described in the AI summary.
216-233: LGTM! Flag definitions are clear and well-documented.The new timeout, max-retries, and retry-backoff-multiplier flags have appropriate defaults and helpful descriptions.
477-495: LGTM! Timeout field validation is robust.The duration validation using
time.ParseDurationensures valid input, and the error message is user-friendly.
601-625: LGTM! CLI args construction is straightforward.The timeout parsing properly falls back to
defaultActivityTimeouton error, ensuring safe defaults.
651-657: LGTM! Parameter validation is appropriate.The validation ensures maxRetries is non-negative and retryBackoffMultiplier is positive, preventing invalid configuration.
1177-1216: LGTM! Retry logic properly implemented.The retry mechanism correctly uses the
sethvargo/go-retrylibrary. The constant 1ms backoff withWithMaxRetriesis appropriate here since timeout adjustment is handled separately viacalculateRetryTimeoutusing the backoff multiplier.
1218-1242: LGTM! Timeout calculation is clear and correct.The exponential timeout increase on retries is properly implemented, and logging is appropriately conditional on UI mode.
1244-1273: LGTM! Job attempt execution is well-structured.The selective retry behavior (only on timeout, not other failures) is correct, and the UI message logic properly handles retry attempts.
1723-1750: LGTM! Activity watchdog is properly implemented.The goroutine properly handles all exit conditions (timeout, command completion, context cancellation) and uses the
activityCheckIntervalconstant for polling.
3206-3305: LGTM! PRD task prompt building is well-organized.The prompt generation functions are properly scoped, use appropriate path manipulation, and generate clear, structured instructions.
3608-3632: LGTM! Activity monitor implementation is thread-safe and correct.The mutex protection ensures safe concurrent access, and initializing
lastActivitytotime.Now()in the constructor provides a valid baseline for the first timeout check.
3634-3688: LGTM! Activity recording integrated correctly into log tap.The activity monitor is called appropriately when data is written, ensuring accurate timeout detection based on process output.
3690-3796: LGTM! JSON formatter activity recording is consistent.The activity monitoring in
jsonFormatterfollows the same pattern asuiLogTap, ensuring consistent behavior across output streams.examples/temporal-standalone/custom-ports/.env.example (1)
1-2: LGTM! Clear example template.The environment variable template is clear and follows standard conventions for API key placeholders.
examples/temporal-standalone/debugging/.env.example (1)
1-2: LGTM! Clear example template.The environment variable template is clear and follows standard conventions.
examples/temporal-standalone/no-ui/.env.example (1)
1-2: LGTM! Clear example template.The environment variable template is clear and follows standard conventions.
examples/temporal-standalone/migration-from-remote/.env.example (1)
1-2: LGTM! Clear example template.The environment variable template is clear and follows standard conventions.
engine/worker/embedded/config.go (4)
12-24: LGTM! Well-defined constants.All default values are properly named with clear intent, following the guideline to avoid magic numbers.
65-93: LGTM! Proper default application.The function correctly applies defaults for all zero-valued configuration fields. The absence of EnableUI default logic is appropriate given the bool zero-value ambiguity.
95-124: LGTM! Comprehensive validation.The validation logic is well-structured, with appropriate error checking for all configuration fields and conditional validation for UI-related settings.
126-186: LGTM! Robust validation helpers.All validation functions provide clear error messages, proper bounds checking, and appropriate use of standard library functions for validation.
examples/temporal-standalone/integration-testing/.env.example (1)
1-2: LGTM! Clear example template.The environment variable template is clear and follows standard conventions.
examples/temporal-standalone/persistent/.env.example (1)
1-2: LGTM! Clear example template.The environment variable template is clear and follows standard conventions.
engine/worker/embedded/namespace_test.go (1)
37-37: LGTM! Correct context usage.The test correctly uses
t.Context()instead ofcontext.Background(), following the test guidelines.engine/infra/server/dependencies.go (3)
187-191: Correct integration point for standalone Temporal startup.Hooking maybeStartStandaloneTemporal before worker init is the right order; cleanup is appended for graceful shutdown.
330-366: Context-backed startup with sane fallbacks looks solid.
- Reads cfg via config.FromContext(ctx); logs via logger.FromContext(ctx).
- Applies StartTimeout fallback when WorkerShutdown is unset.
- Mutates cfg.Temporal.HostPort only after successful Start.
All align with guidelines.
383-395: Shutdown uses context.WithoutCancel and timeout.Good defensive cleanup; avoids cancellation cascades and bounds Stop duration.
engine/worker/embedded/config_test.go (4)
28-107: Comprehensive validation cases with “Should …” naming.Covers key config errors and success path; assertions are specific and clear.
125-140: Defaults behavior is correct and explicit.Explicitly not defaulting EnableUI is called out; constants verified.
145-158: SQLite connect attributes verified for memory and file modes.Good parity check; maps assert expected attrs.
163-171: Static host/port mapping assertions are precise.Service offsets computed as expected; nice sanity checks.
test/integration/temporal/errors_test.go (1)
26-29: Good: server cleanup is now guaranteed.t.Cleanup registered immediately after start ensures no leaks on early failures. This resolves the prior leak concern.
engine/worker/embedded/ui.go (2)
47-54: Good: capture bindIP/uiPort at construction.Avoids Start-time config reads; aligns with context-backed retrieval guidance.
134-139: Good: non-blocking drain of runErrCh.Prevents Stop path from hanging in edge cases.
pkg/config/config_test.go (2)
49-62: Defaults coverage looks solid.Standalone defaults asserted comprehensively; matches expected values.
290-296: Nice: validates UI disabled allows zero port.Covers the important negative/positive case.
engine/worker/embedded/namespace.go (1)
16-23: Good: context/log usage and idempotent namespace creation.Accepts ctx, logs via logger.FromContext, and treats “already exists” as success. Clone of SQL config avoids mutation of shared state.
Also applies to: 41-48
test/integration/temporal/mode_switching_test.go (1)
33-33: No issues found; the function is defined in the same package and will compile correctly.The function
defaultNamespace()is defined intest/integration/temporal/standalone_test.goat line 312. Since bothmode_switching_test.goandstandalone_test.goare in the same package (test/integration/temporal), the function is accessible across files and the code will compile without errors.Likely an incorrect or invalid review comment.
go.mod (2)
277-277: quic-go vulnerability addressed.Upgraded to github.com/quic-go/quic-go v0.54.1 (patched). Good fix for GHSA-47m2-4cr7-mhcw.
73-73: Let me search for compatibility information between these versions:Let me search for more specific information about ui-server configuration and version requirements:
No compatibility issues identified; verify Temporal Server address configuration.
Temporal UI requires Temporal v1.16.0 or later, and v1.29.0 meets this requirement. No documented breaking changes exist between ui-server v2.41.0 and Temporal Server v1.29.0.
Ensure TEMPORAL_ADDRESS environment variable is set to the Temporal Server endpoint (e.g., 127.0.0.1:7233), and verify other required environment flags such as TEMPORAL_UI_PORT and TEMPORAL_UI_ENABLED are properly configured as documented in the Temporal Web UI references.
pkg/config/loader.go (1)
382-398: Standalone mode validation: solid structure and checks.Mode dispatch, port span collision, IP validation, metadata/log level/start timeout checks look correct and align with SRP. Nice use of named constants.
Ensure defaults in pkg/config/config.go for StandaloneConfig match these validators (e.g., FrontendPort, UIPort, LogLevel). If you want, I can script-verify defaults vs validators.
Also applies to: 400-418, 427-447, 449-486, 469-479, 481-486
test/integration/temporal/standalone_test.go (2)
261-271: Justify the +1000 UI offset or derive it from config.Good to see the clarifying comment. If UI default port is FrontendPort+1000, consider referencing a constant or deriving from embedded/config defaults to avoid drift. Otherwise, add a brief note pointing to where this offset is defined.
297-310: Config default helper is appropriate.newEmbeddedConfigFromDefaults correctly pulls Standalone defaults; good alignment with loader validation.
engine/worker/embedded/server_test.go (1)
90-119: Lifecycle scenarios well covered.Graceful stop, timeout, conflicts, and readiness look comprehensive. No issues.
test/integration/temporal/startup_lifecycle_test.go (1)
112-159: Concurrency test uses WaitGroup.Go appropriately.wg.Go aligns with Go 1.25.2 guidance; result collection and shutdown sequencing look correct.
If CI runs older Go, confirm toolchain is 1.25.2 to avoid build failures.
engine/worker/embedded/server.go (1)
78-103: Initialization order is correct; createNamespace performs no RPCs.Verification confirms that
createNamespaceoperates exclusively on the SQL persistence layer viasqliteschema.CreateNamespaces(), which writes the namespace schema directly to the database. It makes no network calls to the Temporal frontend service. The function receives SQL configuration and writes to the datastore before the server is constructed, which is the correct and safe initialization order. Additionally,BindIPdefaults to127.0.0.1(not0.0.0.0).Likely an incorrect or invalid review comment.
1 participant
Summary by CodeRabbit
New Features
Tests
Documentation