feat: implement first-run setup wizard#584
Conversation
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📜 Recent review details⏰ 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). (2)
🧰 Additional context used📓 Path-based instructions (3)**/*.{ts,tsx,vue}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
**/*.test.{ts,tsx}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
web/**/*.{ts,tsx,vue}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
🧠 Learnings (2)📓 Common learnings📚 Learning: 2026-03-19T11:29:37.522ZApplied to files:
🧬 Code graph analysis (1)web/src/__tests__/router/guards.test.ts (3)
🔇 Additional comments (4)
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds a first-run setup wizard across backend, CLI, frontend, settings, observability, and tests: new Litestar setup endpoints and setting, a CLI Changes
Sequence Diagram(s)sequenceDiagram
participant User as User (Browser)
participant Router as Router Guard
participant Page as SetupPage.vue
participant Store as Setup Store
participant API as Backend API (SetupController)
participant Settings as Settings (DB)
participant Providers as Provider Registry
User->>Router: Navigate to app
Router->>Store: fetchStatus()
Store->>API: GET /api/v1/setup/status
API->>Settings: read api.setup_complete & users
API->>Providers: check provider registry
API-->>Store: returns {needs_admin, needs_setup, has_providers}
alt needs_setup == true
Router-->>User: redirect /setup
User->>Page: open wizard
Page->>Store: fetchTemplates()
Store->>API: GET /api/v1/setup/templates
Page->>API: POST /api/v1/setup/company
API->>Settings: persist company + departments
Page->>API: POST /api/v1/setup/agent
API->>Settings: append agent (with lock)
Page->>API: POST /api/v1/setup/complete
API->>Settings: set api.setup_complete = "true"
API-->>Page: success
Page->>Router: navigate to dashboard
else
Router-->>User: allow dashboard access
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
📝 Coding Plan
Comment |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a streamlined onboarding experience for new users by implementing a first-run setup wizard. This wizard guides users through essential initial configurations, including creating an administrative account, setting up an LLM provider, defining their company structure with optional templates, and creating their first AI agent. The goal is to simplify the initial setup process, making the application more accessible and user-friendly from the very beginning. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #584 +/- ##
==========================================
- Coverage 92.66% 92.55% -0.12%
==========================================
Files 545 547 +2
Lines 27065 27274 +209
Branches 2603 2615 +12
==========================================
+ Hits 25080 25243 +163
- Misses 1568 1605 +37
- Partials 417 426 +9 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Warning Gemini is experiencing higher than usual traffic and was unable to create the review. Please try again in a few hours by commenting |
There was a problem hiding this comment.
Actionable comments posted: 17
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/synthorg/api/app.py (1)
647-658:⚠️ Potential issue | 🟠 MajorAlways exclude
/setup/statusfrom auth, even when custom exclusions are configured.Line 648 currently trusts
auth.exclude_pathsas-is; Line 656 is only applied in the fallback branch. This can break first-run setup for deployments that define custom auth exclusions.💡 Proposed fix
def _build_middleware(api_config: ApiConfig) -> list[Middleware]: @@ ws_path = f"^{prefix}/ws$" + setup_status_path = f"^{prefix}/setup/status$" @@ exclude_paths = ( auth.exclude_paths if auth.exclude_paths is not None else ( f"^{prefix}/health$", "^/docs", "^/api$", f"^{prefix}/auth/setup$", f"^{prefix}/auth/login$", - f"^{prefix}/setup/status$", + setup_status_path, ) ) + if setup_status_path not in exclude_paths: + exclude_paths = (*exclude_paths, setup_status_path) @@ if ws_path not in exclude_paths: exclude_paths = (*exclude_paths, ws_path)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/synthorg/api/app.py` around lines 647 - 658, The current assignment of exclude_paths trusts auth.exclude_paths entirely and only adds "^/setup/status$" in the fallback; update the logic in the exclude_paths assignment (where exclude_paths, auth.exclude_paths and prefix are used) to always ensure the literal "^/setup/status$" is present: if auth.exclude_paths is provided, append or merge "^/setup/status$" into that sequence (avoiding duplicates), otherwise use the existing fallback tuple that includes f"^{prefix}/health$", "^/docs", "^/api$", f"^{prefix}/auth/setup$", f"^{prefix}/auth/login$", and "^/setup/status$". Ensure the final exclude_paths type remains the expected sequence (tuple/list) used by the rest of the code.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cli/cmd/setup.go`:
- Around line 67-74: The CLI's resetSetupFlag() issues an unauthenticated DELETE
to /api/v1/settings/api/setup_complete which is protected by
require_write_access and will return 403, leaving setup_complete unchanged;
either make the endpoint publicly writable for this specific path/method or
(preferable) authenticate the CLI before calling resetSetupFlag(): update the
CLI flow to obtain credentials (reuse existing login/session logic) and include
auth headers when calling resetSetupFlag(), or alternatively add the DELETE path
to the server's auth-exclude list for that endpoint/method so
require_write_access won't block it; locate resetSetupFlag and the server auth
exclude configuration to implement the chosen fix.
- Around line 62-65: The current check treating psOut=="" is insufficient
because docker.ComposeExecOutput returns "[]" when no containers exist; update
the logic around docker.ComposeExecOutput (psOut variable) in the setup flow to
detect an empty container list by parsing the JSON (reuse
diagnostics.parseContainerDetails or json.Unmarshal into a slice) and treat an
empty slice as no containers, or at minimum check for the literal "[]" string;
then return the same error when no containers are found.
In `@src/synthorg/api/controllers/setup.py`:
- Around line 282-298: Move the company name persistence so it happens after
template validation and ensure departments are cleared when the template yields
no departments: call _extract_template_departments(data.template_name) and
validate the template first (using template_applied, departments_json and
department_count) before invoking settings_svc.set("company","company_name",
...); when departments_json is falsy still write an explicit empty departments
value (e.g. "[]" or null) via settings_svc.set("company","departments", ...) to
avoid leaving stale departments from previous runs; use the existing symbols
template_applied, departments_json, department_count,
_extract_template_departments, and settings_svc.set to implement this ordering
and explicit clear.
- Around line 423-428: The current completion handler only checks
app_state.provider_registry; update the setup completion logic in the controller
(where settings_svc.set("api","setup_complete","true") is called) to also verify
required entities exist: use app_state.company_service and
app_state.agent_service to asynchronously confirm there is at least one company
and at least one agent (e.g., via count/list/get methods) and if either is
missing raise ApiValidationError with a clear message; ensure you await the
service calls and keep the provider check intact before calling
settings_svc.set.
- Around line 494-504: The current helper swallows any Exception during
settings_svc.get_entry or json.loads and returns [], which lets create_agent
overwrite malformed stored agents; change the error handling so only the
"missing entry" case yields []: call settings_svc.get_entry("company","agents"),
if entry is None or entry.value is None return []; otherwise attempt
json.loads(entry.value) and if json.loads raises json.JSONDecodeError (or other
parsing errors) re-raise or surface the error instead of catching it silently;
do not blanket-catch Exception around the get_entry/json.loads block—keep the
existing MemoryError and RecursionError passthrough, and limit or remove the
logger.debug(SETUP_AGENTS_READ_FALLBACK, ...) fallback so it is only used when
the entry is truly absent, referencing settings_svc.get_entry, json.loads,
SETUP_AGENTS_READ_FALLBACK, and create_agent to locate where to make the change.
- Around line 81-82: The optional template identifier template_name should use
NotBlankStr to reject empty strings at DTO parsing time; change the field type
from str | None to NotBlankStr | None (keep default=None and max_length=100) and
ensure NotBlankStr is imported from core.types, so invalid payloads fail
validation instead of reaching _extract_template_departments() and producing a
404.
In `@tests/unit/api/controllers/test_setup.py`:
- Around line 177-250: Add a positive-path unit test (e.g., in
tests/unit/api/controllers/test_setup.py alongside
TestSetupAgent/TestSetupComplete) that seeds a valid provider, uses test_client
to POST to "/api/v1/setup/agent" with valid payload (name, role, model_provider
matching the seeded provider, model_id, optional personality_preset), asserts
success (200/201) and that the returned agent exists, then POSTs to
"/api/v1/setup/complete" (with appropriate auth headers if needed via
make_auth_headers), asserts success (200/204) and verifies the application
settings/state reflect setup completed (e.g., check settings endpoint or DB
flag). Ensure the test uses existing fixtures (test_client, make_auth_headers)
and cleans up or isolates state so it won’t interfere with other tests.
In `@web/src/__tests__/views/SetupPage.test.ts`:
- Around line 140-165: Tests are using fragile visual/CSS selectors
('.rounded-full') and loose text matching for the logo; update the tests to use
stable selectors and accessible labels instead. Modify the test cases that mount
SetupPage (the three it blocks referencing SetupPage) to look for a data-testid
or accessible role/alt text: replace the '.rounded-full' check with something
like wrapper.findAll('[data-testid="wizard-step-dot"]') or
findAllByRole('listitem') for the step indicator, and replace the generic text
check for the logo with a query for an element with a stable identifier such as
'[data-testid="branding-logo"]' or an img/button with a specific alt or
aria-label; if the template/component lacks these attributes, add data-testid or
accessible alt/aria-label to the logo and step indicator in the SetupPage
component so tests target them reliably.
In `@web/src/components/setup/SetupAdmin.vue`:
- Line 7: The form currently uses the hard-coded MIN_PASSWORD_LENGTH constant
for validation and helper text; replace those usages so the form reads the
backend-configured value api.auth.min_password_length (when available) and only
falls back to MIN_PASSWORD_LENGTH if the server value is undefined. Concretely:
add a reactive/computed binding (e.g., passwordMinLength) that returns
api.auth.min_password_length || MIN_PASSWORD_LENGTH, update the password
validation rule(s) and helper text in SetupAdmin.vue to use that binding instead
of MIN_PASSWORD_LENGTH, and ensure any submit/validator logic (where the
constant is referenced) consults this new binding so runtime/YAML overrides are
respected.
In `@web/src/components/setup/SetupAgent.vue`:
- Around line 87-91: When the provider changes the previously chosen model must
be cleared; add logic that resets selectedModel (e.g., set selectedModel.value =
null/undefined) whenever selectedProvider changes or when computed
filteredModels no longer contains the current selectedModel. Update the
SetupAgent component by adding a watcher on selectedProvider (or by augmenting
the filteredModels computed to clear selectedModel if it’s not present in
modelOptions filtered by selectedProvider) so the form validation (isValid) and
submit payload always reflect the visible model selection.
- Line 128: The agent budget field is initialized to null so
budget_limit_monthly is always sent as null; change the SetupAgent.vue data
property for budget_limit_monthly to undefined (or omit it) and ensure the
form/model binding uses that reactive property, and update the submit handler
(e.g., createAgent/handleSubmit) to include budget_limit_monthly only when it is
a number (or non-empty) so the API receives the actual value instead of a
persistent null.
- Around line 119-136: The current flow calls setupApi.createAgent and
setupStore.markComplete together so a failed markComplete on retry will re-run
the non-idempotent createAgent; change the flow so createAgent(...) is performed
once and its result is persisted (e.g., savedAgent variable) and any failures
from setupStore.markComplete() are handled/retried separately; specifically,
after a successful setupApi.createAgent(...) store the returned result in a
local reactive (or state) variable, then call setupStore.markComplete() inside
its own try/catch/retry block and only call emit('complete', ...) after
markComplete succeeds — ensure subsequent retry logic checks for the saved agent
and calls only setupStore.markComplete() rather than re-invoking
setupApi.createAgent().
In `@web/src/components/setup/SetupProvider.vue`:
- Around line 20-25: The component currently only sets the reactive error ref
for add/test actions, but onMounted() calls fetchPresets() and fetchProviders()
(and relies on store.presets) so failures or an empty presets array can leave
the wizard stuck; update the onMounted flow and related handlers (referencing
onMounted, fetchPresets, fetchProviders, and store.presets) to catch and set the
error ref on failure, and explicitly handle the empty-presets case by setting a
user-facing error or fallback state (e.g., set error.value to a descriptive
message and ensure createdProviderName/testResult fallbacks are set) so the UI
renders an actionable path instead of dead-ending; apply the same pattern to
other handlers that assume success (the add/test/result flows) so they all
surface errors via the error reactive.
In `@web/src/router/guards.ts`:
- Around line 24-46: The router guard in guards.ts relies on
setup.status/setup.isSetupNeeded but nothing bootstraps that state; update the
guard to call and await setup.fetchStatus() when setup.status === null (or
otherwise ensure fetchStatus() runs before routing) so the redirect logic
executes correctly; make the guard async, handle and log errors from
fetchStatus(), and keep current checks that redirect to 'setup' or '/' based on
setup.isSetupNeeded; alternatively, move the fetch into a top-level initializer
or a layout wrapper so setup.status is populated prior to evaluating the
existing guard logic.
In `@web/src/stores/setup.ts`:
- Around line 14-15: The wizard allows setting arbitrary step numbers because
MAX_STEPS is a static constant disconnected from the actual steps and setStep()
does not validate; update the implementation so that step bounds are determined
from the actual steps collection (e.g., derive maxStep = steps.length - 1 or
similar) and clamp any incoming value in setStep(stepNumber) to the valid range
(min 0, max maxStep); replace or remove the hardcoded MAX_STEPS usage and ensure
all callers and code paths referenced around setStep and the MAX_STEPS constant
(including the code around lines 40-54) use the clamped value.
- Around line 16-29: The store currently treats status === null as "no setup
needed", causing guards to fail open on fetch errors; add an explicit loaded
flag (e.g., statusLoaded or status.valueLoaded) and change fetchStatus to only
set status.value and mark loaded = true on success, while leaving loaded = false
on error; update the computed selectors isSetupNeeded and isAdminNeeded to first
check loaded and return a conservative "need setup / need admin" value (i.e.,
fail closed) when loaded is false, otherwise derive from
status.value.needs_setup and status.value.needs_admin; reference the existing
symbols fetchStatus, status, loading, error, isSetupNeeded, and isAdminNeeded
when making these changes.
In `@web/src/views/SetupPage.vue`:
- Around line 68-73: After calling setup.fetchStatus() in the onMounted hook,
check the returned/updated setup.status (the object populated by
setup.fetchStatus) and if setup.status.needs_setup === false perform a
programmatic redirect via the view's router (e.g. router.replace or router.push)
to the app home or appropriate guarded route so a completed install cannot
render the setup wizard; update the onMounted handler to run this check
immediately after await setup.fetchStatus() (also apply same check in any other
mount/refresh paths that currently render the wizard, e.g. the logic controlling
the v-else-if="setup.status" branch).
---
Outside diff comments:
In `@src/synthorg/api/app.py`:
- Around line 647-658: The current assignment of exclude_paths trusts
auth.exclude_paths entirely and only adds "^/setup/status$" in the fallback;
update the logic in the exclude_paths assignment (where exclude_paths,
auth.exclude_paths and prefix are used) to always ensure the literal
"^/setup/status$" is present: if auth.exclude_paths is provided, append or merge
"^/setup/status$" into that sequence (avoiding duplicates), otherwise use the
existing fallback tuple that includes f"^{prefix}/health$", "^/docs", "^/api$",
f"^{prefix}/auth/setup$", f"^{prefix}/auth/login$", and "^/setup/status$".
Ensure the final exclude_paths type remains the expected sequence (tuple/list)
used by the rest of the code.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: e020f58a-61c8-45dd-ab4c-66890b684215
📒 Files selected for processing (23)
CLAUDE.mdcli/cmd/setup.godocs/design/operations.mdsrc/synthorg/api/app.pysrc/synthorg/api/controllers/__init__.pysrc/synthorg/api/controllers/setup.pysrc/synthorg/api/openapi.pysrc/synthorg/observability/events/setup.pysrc/synthorg/settings/definitions/api.pytests/unit/api/controllers/test_setup.pytests/unit/observability/test_events.pyweb/src/__tests__/views/SetupPage.test.tsweb/src/api/endpoints/setup.tsweb/src/api/types.tsweb/src/components/setup/SetupAdmin.vueweb/src/components/setup/SetupAgent.vueweb/src/components/setup/SetupCompany.vueweb/src/components/setup/SetupComplete.vueweb/src/components/setup/SetupProvider.vueweb/src/components/setup/SetupWelcome.vueweb/src/router/guards.tsweb/src/stores/setup.tsweb/src/views/SetupPage.vue
📜 Review details
⏰ 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 (Python 3.14)
- GitHub Check: Build Sandbox
- GitHub Check: Build Web
- GitHub Check: Build Backend
- GitHub Check: CLI Test (macos-latest)
- GitHub Check: CLI Test (windows-latest)
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (8)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Never usefrom __future__ import annotations— Python 3.14 has PEP 649 native lazy annotations
Useexcept A, B:(no parentheses) syntax for exception handling — PEP 758 enforced by ruff on Python 3.14
All public functions must have type hints; enforce mypy strict mode
Use Google-style docstrings on all public classes and functions; enforced by ruff D rules
Create new objects instead of mutating existing ones; usecopy.deepcopy()at construction and system boundaries, and wrap internal non-Pydantic collections withMappingProxyTypefor read-only enforcement
Use frozen Pydantic models for config/identity; use separate mutable-via-copy models (withmodel_copy(update=...)) for runtime state. Never mix static config fields with mutable runtime fields in one model
Use Pydantic v2 with@computed_fieldfor derived values instead of storing redundant fields; useNotBlankStr(fromcore.types) for all identifier/name fields including optional and tuple variants instead of manual validators
Preferasyncio.TaskGroupfor fan-out/fan-in parallel operations in new code instead of barecreate_task
Line length must be 88 characters; enforced by ruff
Functions must be under 50 lines; files must be under 800 lines
Handle all errors explicitly; never silently swallow exceptions
Validate inputs at system boundaries (user input, external APIs, config files)
Files:
tests/unit/observability/test_events.pysrc/synthorg/api/controllers/__init__.pysrc/synthorg/api/openapi.pysrc/synthorg/settings/definitions/api.pysrc/synthorg/api/app.pysrc/synthorg/observability/events/setup.pytests/unit/api/controllers/test_setup.pysrc/synthorg/api/controllers/setup.py
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
tests/**/*.py: Mark all tests with@pytest.mark.unit,@pytest.mark.integration,@pytest.mark.e2e, or@pytest.mark.slow
Maintain minimum 80% code coverage enforced in CI
Useasyncio_mode = "auto"in pytest configuration — no manual@pytest.mark.asyncioneeded
Always include-n autowhen running pytest viapytest-xdist; never run tests sequentially
Never use real vendor names (Anthropic, OpenAI, Claude, GPT, etc.) in tests or project code — use generic names:test-provider,test-small-001, etc.; vendor names only in Operations design page,.claude/files, or third-party import paths
Use Hypothesis for property-based testing with@given+@settings; useHYPOTHESIS_PROFILEenv var to control examples (ci: 50, dev: 1000)
Never skip or ignore flaky tests — fix them fundamentally by mockingtime.monotonic()andasyncio.sleep()to make timing-sensitive tests deterministic
Files:
tests/unit/observability/test_events.pytests/unit/api/controllers/test_setup.py
src/synthorg/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/synthorg/**/*.py: Every module with business logic must import logger viafrom synthorg.observability import get_loggerand initialize withlogger = get_logger(__name__)
Never useimport logging,logging.getLogger(), orprint()in application code; use the injected logger instead
Always use variable namelogger(not_logger, notlog)
Use event constants fromsynthorg.observability.events.<domain>(e.g.,API_REQUEST_STARTEDfromevents.api); import directly and log with structured kwargs:logger.info(EVENT, key=value), never interpolated strings
Log all error paths at WARNING or ERROR with context before raising; log all state transitions at INFO; log object creation/internal flow/entry-exit at DEBUG
All provider calls go throughBaseCompletionProviderwhich applies retry + rate limiting automatically; never implement retry logic in driver subclasses or calling code
SetRetryConfigandRateLimiterConfigper-provider inProviderConfig; retryable errors includeRateLimitError,ProviderTimeoutError,ProviderConnectionError,ProviderInternalError; non-retryable errors raise immediately
Pure data models, enums, and re-exports do NOT need logging
Files:
src/synthorg/api/controllers/__init__.pysrc/synthorg/api/openapi.pysrc/synthorg/settings/definitions/api.pysrc/synthorg/api/app.pysrc/synthorg/observability/events/setup.pysrc/synthorg/api/controllers/setup.py
web/**/*.vue
📄 CodeRabbit inference engine (CLAUDE.md)
Never use real vendor names (Anthropic, OpenAI, Claude, GPT, etc.) in dashboard code — use generic names:
example-provider,example-large-001, etc.
Files:
web/src/components/setup/SetupWelcome.vueweb/src/components/setup/SetupCompany.vueweb/src/components/setup/SetupComplete.vueweb/src/components/setup/SetupAdmin.vueweb/src/components/setup/SetupAgent.vueweb/src/components/setup/SetupProvider.vueweb/src/views/SetupPage.vue
web/**/*.{js,ts,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Run ESLint and vue-tsc type checking via
npm --prefix web run lintandnpm --prefix web run type-check
Files:
web/src/components/setup/SetupWelcome.vueweb/src/components/setup/SetupCompany.vueweb/src/router/guards.tsweb/src/components/setup/SetupComplete.vueweb/src/components/setup/SetupAdmin.vueweb/src/components/setup/SetupAgent.vueweb/src/__tests__/views/SetupPage.test.tsweb/src/api/endpoints/setup.tsweb/src/api/types.tsweb/src/components/setup/SetupProvider.vueweb/src/views/SetupPage.vueweb/src/stores/setup.ts
docs/design/**/*.md
📄 CodeRabbit inference engine (CLAUDE.md)
docs/design/**/*.md: Always read the relevantdocs/design/page before implementing any feature; design spec is the starting point for architecture, data models, and behavior
If implementation deviates from spec (better approach, scope evolution), alert the user and explain why — user decides whether to proceed or update the spec; never silently diverge
When approved deviations occur, update the relevantdocs/design/page to reflect the new reality
Files:
docs/design/operations.md
web/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Use fast-check for property-based testing with
fc.assert+fc.property
Files:
web/src/router/guards.tsweb/src/__tests__/views/SetupPage.test.tsweb/src/api/endpoints/setup.tsweb/src/api/types.tsweb/src/stores/setup.ts
cli/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
cli/**/*.go: Use nativetesting.Ffuzz functions (Fuzz*) for fuzz testing Go code
Run Go lint viagolangci-lint run, vet viago vet, tests viago test ./..., and fuzz viago test -fuzz=FuzzTarget -fuzztime=30s
Never use real vendor names (Anthropic, OpenAI, Claude, GPT, etc.) in CLI code — use generic names:test-provider,test-small-001, etc.; vendor names only in third-party import paths
Files:
cli/cmd/setup.go
🧠 Learnings (18)
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to src/synthorg/**/*.py : Event names: always use constants from domain-specific modules under synthorg.observability.events (e.g., PROVIDER_CALL_START from events.provider, BUDGET_RECORD_ADDED from events.budget, etc.). Import directly: `from synthorg.observability.events.<domain> import EVENT_CONSTANT`.
Applied to files:
tests/unit/observability/test_events.pysrc/synthorg/observability/events/setup.pyCLAUDE.md
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to web/src/components/** : Vue components organized by feature (agents/, approvals/, budget/, common/, dashboard/, layout/, messages/, org-chart/, tasks/).
Applied to files:
web/src/components/setup/SetupWelcome.vueweb/src/components/setup/SetupCompany.vueweb/src/components/setup/SetupComplete.vueweb/src/components/setup/SetupAdmin.vueweb/src/components/setup/SetupAgent.vueweb/src/views/SetupPage.vueCLAUDE.md
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to web/** : Web dashboard: Node.js 20+, dependencies in web/package.json (Vue 3, PrimeVue, Tailwind CSS, Pinia, VueFlow, ECharts, Axios, vue-draggable-plus, Vitest, fast-check, ESLint, vue-tsc).
Applied to files:
web/src/components/setup/SetupWelcome.vueweb/src/components/setup/SetupComplete.vueCLAUDE.md
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to cli/** : CLI: Go 1.26+, dependencies in cli/go.mod (Cobra, charmbracelet/huh).
Applied to files:
docs/design/operations.mdcli/cmd/setup.go
📚 Learning: 2026-03-19T11:13:43.548Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T11:13:43.548Z
Learning: Applies to web/**/*.vue : Never use real vendor names (Anthropic, OpenAI, Claude, GPT, etc.) in dashboard code — use generic names: `example-provider`, `example-large-001`, etc.
Applied to files:
web/src/components/setup/SetupComplete.vue
📚 Learning: 2026-03-19T11:13:43.548Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T11:13:43.548Z
Learning: Applies to src/synthorg/**/*.py : Use event constants from `synthorg.observability.events.<domain>` (e.g., `API_REQUEST_STARTED` from `events.api`); import directly and log with structured kwargs: `logger.info(EVENT, key=value)`, never interpolated strings
Applied to files:
src/synthorg/observability/events/setup.pyCLAUDE.md
📚 Learning: 2026-03-19T11:13:43.548Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T11:13:43.548Z
Learning: Applies to src/synthorg/**/*.py : Every module with business logic must import logger via `from synthorg.observability import get_logger` and initialize with `logger = get_logger(__name__)`
Applied to files:
src/synthorg/observability/events/setup.pyCLAUDE.md
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to src/synthorg/**/*.py : All state transitions must log at INFO.
Applied to files:
src/synthorg/observability/events/setup.pyCLAUDE.md
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to src/synthorg/**/*.py : DEBUG logging for object creation, internal flow, entry/exit of key functions.
Applied to files:
src/synthorg/observability/events/setup.pyCLAUDE.md
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to src/synthorg/**/*.py : Every module with business logic MUST have: `from synthorg.observability import get_logger` then `logger = get_logger(__name__)`. Never use `import logging` / `logging.getLogger()` / `print()` in application code. Variable name: always `logger` (not `_logger`, not `log`).
Applied to files:
src/synthorg/observability/events/setup.pyCLAUDE.md
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to docker/{Dockerfile*,compose.yml} : Docker: Backend uses 3-stage build (builder → setup → distroless runtime), Chainguard Python, non-root (UID 65532), CIS-hardened. Web uses nginxinc/nginx-unprivileged, Vue 3 SPA with PrimeVue + Tailwind CSS, SPA routing, API/WebSocket proxy to backend.
Applied to files:
web/src/views/SetupPage.vueCLAUDE.md
📚 Learning: 2026-03-19T11:13:43.548Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T11:13:43.548Z
Learning: Applies to src/synthorg/**/*.py : Log all error paths at WARNING or ERROR with context before raising; log all state transitions at INFO; log object creation/internal flow/entry-exit at DEBUG
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-19T11:13:43.548Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T11:13:43.548Z
Learning: Applies to src/synthorg/**/*.py : Pure data models, enums, and re-exports do NOT need logging
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-19T11:13:43.548Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T11:13:43.548Z
Learning: Applies to docker/Dockerfile.web : `nginxinc/nginx-unprivileged` base with non-root user (UID 101), Vue 3 SPA (PrimeVue + Tailwind CSS), SPA routing, API/WebSocket proxy to backend
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-19T11:13:43.548Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T11:13:43.548Z
Learning: Applies to src/synthorg/**/*.py : Never use `import logging`, `logging.getLogger()`, or `print()` in application code; use the injected logger instead
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-19T11:13:43.548Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T11:13:43.548Z
Learning: Applies to src/synthorg/**/*.py : Always use variable name `logger` (not `_logger`, not `log`)
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to src/synthorg/**/*.py : Structured kwargs in logging: always `logger.info(EVENT, key=value)` — never `logger.info('msg %s', val)`.
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to src/synthorg/**/*.py : All error paths must log at WARNING or ERROR with context before raising.
Applied to files:
CLAUDE.md
🧬 Code graph analysis (8)
src/synthorg/api/controllers/__init__.py (1)
src/synthorg/api/controllers/setup.py (1)
SetupController(165-432)
src/synthorg/settings/definitions/api.py (3)
src/synthorg/settings/registry.py (1)
register(30-50)src/synthorg/settings/models.py (1)
SettingDefinition(34-140)src/synthorg/settings/enums.py (2)
SettingNamespace(6-21)SettingType(24-36)
web/src/router/guards.ts (2)
web/src/stores/auth.ts (1)
useAuthStore(8-191)web/src/stores/setup.ts (1)
useSetupStore(7-87)
web/src/api/endpoints/setup.ts (3)
src/synthorg/api/controllers/setup.py (6)
SetupStatusResponse(37-50)TemplateInfoResponse(53-68)SetupCompanyRequest(71-82)SetupCompanyResponse(85-98)SetupAgentRequest(101-139)SetupAgentResponse(142-159)web/src/api/types.ts (7)
SetupStatusResponse(824-828)ApiResponse(139-141)TemplateInfoResponse(830-835)SetupCompanyRequest(837-840)SetupCompanyResponse(853-857)SetupAgentRequest(842-851)SetupAgentResponse(859-865)web/src/api/client.ts (2)
apiClient(12-16)unwrap(70-80)
cli/cmd/setup.go (3)
cli/internal/config/state.go (2)
Load(53-91)State(16-28)cli/internal/ui/ui.go (1)
NewUI(58-70)cli/internal/docker/client.go (2)
Detect(34-66)ComposeExecOutput(100-107)
web/src/api/types.ts (1)
src/synthorg/api/controllers/setup.py (6)
SetupStatusResponse(37-50)TemplateInfoResponse(53-68)SetupCompanyRequest(71-82)SetupAgentRequest(101-139)SetupCompanyResponse(85-98)SetupAgentResponse(142-159)
web/src/stores/setup.ts (3)
src/synthorg/api/controllers/setup.py (2)
SetupStatusResponse(37-50)TemplateInfoResponse(53-68)web/src/api/types.ts (2)
SetupStatusResponse(824-828)TemplateInfoResponse(830-835)web/src/utils/errors.ts (1)
getErrorMessage(17-69)
tests/unit/api/controllers/test_setup.py (3)
tests/unit/api/conftest.py (2)
make_auth_headers(111-137)test_client(193-224)src/synthorg/api/controllers/setup.py (4)
SetupAgentRequest(101-139)SetupCompanyRequest(71-82)SetupStatusResponse(37-50)_extract_template_departments(438-480)src/synthorg/api/errors.py (1)
NotFoundError(167-175)
🪛 GitHub Check: CodeQL
cli/cmd/setup.go
[failure] 50-50: Uncontrolled data used in path expression
This path depends on a user-provided value.
This path depends on a user-provided value.
This path depends on a user-provided value.
This path depends on a user-provided value.
🪛 LanguageTool
CLAUDE.md
[style] ~202-~202: Since ownership is already implied, this phrasing may be redundant.
Context: ...fromevents.setup). Each domain has its own module -- see src/synthorg/observabili...
(PRP_OWN)
🔇 Additional comments (7)
CLAUDE.md (1)
117-154: Documentation updates correctly reflect the new setup surface.The new package-structure and logging guidance entries are coherent and match the setup wizard additions across backend, web, and CLI.
Also applies to: 202-202
tests/unit/observability/test_events.py (1)
232-232: Good test coverage update for observability domain discovery.Adding
setupto the expected module set keeps the discovery assertion accurate after introducing setup event constants.src/synthorg/api/openapi.py (1)
50-55: Public OpenAPI classification for setup status is correctly wired.This keeps response-injection behavior aligned with the intended unauthenticated setup status endpoint.
src/synthorg/api/controllers/__init__.py (1)
23-23: Controller registration/export changes are complete.
SetupControlleris consistently imported, included inALL_CONTROLLERS, and re-exported.Also applies to: 46-47, 71-71
web/src/components/setup/SetupComplete.vue (1)
1-47: Completion step component looks solid.Typed props, clear summary UI, and explicit dashboard navigation are all implemented cleanly.
web/src/components/setup/SetupWelcome.vue (1)
1-32: Welcome step implementation is clean and idiomatic.Typed emits and straightforward event wiring make this step easy to compose in the wizard flow.
docs/design/operations.md (1)
976-976: Operations design doc is correctly updated for the setup flow.The new CLI command and setup API surface entries are clear and aligned with the implemented feature.
As per coding guidelines: "When approved deviations occur, update the relevant
docs/design/page to reflect the new reality."Also applies to: 1000-1000
| psOut, err := docker.ComposeExecOutput(ctx, info, safeDir, "ps", "--format", "json") | ||
| if err != nil || psOut == "" { | ||
| return fmt.Errorf("no containers running -- run 'synthorg start' first") | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's look at the file and the specific lines in question
cat -n cli/cmd/setup.go | head -80Repository: Aureliolo/synthorg
Length of output: 2768
🏁 Script executed:
# Find the docker package definition
fd -t f "\.go$" | xargs grep -l "func.*ComposeExecOutput" | head -5Repository: Aureliolo/synthorg
Length of output: 126
🏁 Script executed:
# Check if there are any tests or documentation about Docker output
rg "docker.*ps.*format.*json" -A 3 -B 3Repository: Aureliolo/synthorg
Length of output: 1883
🏁 Script executed:
# Let's look at the docker.ComposeExecOutput function
cat -n cli/internal/docker/client.go | head -150Repository: Aureliolo/synthorg
Length of output: 6252
🏁 Script executed:
# Check the parseContainerDetails function to understand JSON parsing
cat -n cli/internal/diagnostics/collect.go | grep -A 30 "func parseContainerDetails"Repository: Aureliolo/synthorg
Length of output: 1053
🏁 Script executed:
# Let's also look at the test file to understand expected behavior
cat -n cli/internal/docker/client_test.goRepository: Aureliolo/synthorg
Length of output: 5864
Parse the JSON output instead of checking for an empty string.
When docker compose ps --format json has no containers running, it returns an empty JSON array ([]), which is non-empty. The current check psOut == "" does not catch this case and allows the setup flow to proceed incorrectly. Either parse the JSON (as done in diagnostics.parseContainerDetails) or check for the literal "[]" string, but string-equality checks alone are insufficient for JSON detection.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cli/cmd/setup.go` around lines 62 - 65, The current check treating psOut==""
is insufficient because docker.ComposeExecOutput returns "[]" when no containers
exist; update the logic around docker.ComposeExecOutput (psOut variable) in the
setup flow to detect an empty container list by parsing the JSON (reuse
diagnostics.parseContainerDetails or json.Unmarshal into a slice) and treat an
empty slice as no containers, or at minimum check for the literal "[]" string;
then return the same error when no containers are found.
| if not app_state.has_provider_registry or len(app_state.provider_registry) == 0: | ||
| msg = "At least one provider must be configured before completing setup" | ||
| raise ApiValidationError(msg) | ||
|
|
||
| settings_svc = app_state.settings_service | ||
| await settings_svc.set("api", "setup_complete", "true") |
There was a problem hiding this comment.
Don't allow /setup/complete before the wizard's required entities exist.
This only checks for configured providers. A direct call can mark setup complete before any company or agent has been created, which hides the setup flow even though the installation is still incomplete.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/synthorg/api/controllers/setup.py` around lines 423 - 428, The current
completion handler only checks app_state.provider_registry; update the setup
completion logic in the controller (where
settings_svc.set("api","setup_complete","true") is called) to also verify
required entities exist: use app_state.company_service and
app_state.agent_service to asynchronously confirm there is at least one company
and at least one agent (e.g., via count/list/get methods) and if either is
missing raise ApiValidationError with a clear message; ensure you await the
service calls and keep the provider check intact before calling
settings_svc.set.
| const error = ref<string | null>(null) | ||
| const creating = ref(false) | ||
| const testing = ref(false) | ||
| const createdProviderName = ref<string | null>(null) | ||
| const testPassed = ref(false) | ||
| const testResult = ref<TestConnectionResponse | null>(null) |
There was a problem hiding this comment.
Initial provider discovery can dead-end this step.
error is only surfaced for add/test actions, but onMounted() also depends on fetchPresets() / fetchProviders(). If those calls fail—or if there is no existing provider and store.presets resolves to []—this step renders no actionable path and the wizard stalls.
Also applies to: 128-136, 160-179, 227-233, 270-276
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/components/setup/SetupProvider.vue` around lines 20 - 25, The
component currently only sets the reactive error ref for add/test actions, but
onMounted() calls fetchPresets() and fetchProviders() (and relies on
store.presets) so failures or an empty presets array can leave the wizard stuck;
update the onMounted flow and related handlers (referencing onMounted,
fetchPresets, fetchProviders, and store.presets) to catch and set the error ref
on failure, and explicitly handle the empty-presets case by setting a
user-facing error or fallback state (e.g., set error.value to a descriptive
message and ensure createdProviderName/testResult fallbacks are set) so the UI
renders an actionable path instead of dead-ending; apply the same pattern to
other handlers that assume success (the add/test/result flows) so they all
surface errors via the error reactive.
web/src/stores/setup.ts
Outdated
| const MAX_STEPS = 10 | ||
|
|
There was a problem hiding this comment.
Clamp direct step changes to real wizard bounds.
MAX_STEPS is decoupled from the actual step list, and setStep() accepts any number, so callers can land on non-existent steps.
Also applies to: 40-54
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/stores/setup.ts` around lines 14 - 15, The wizard allows setting
arbitrary step numbers because MAX_STEPS is a static constant disconnected from
the actual steps and setStep() does not validate; update the implementation so
that step bounds are determined from the actual steps collection (e.g., derive
maxStep = steps.length - 1 or similar) and clamp any incoming value in
setStep(stepNumber) to the valid range (min 0, max maxStep); replace or remove
the hardcoded MAX_STEPS usage and ensure all callers and code paths referenced
around setStep and the MAX_STEPS constant (including the code around lines
40-54) use the clamped value.
Add a multi-step setup wizard that guides new users through initial configuration: admin account creation, LLM provider setup, company creation (with template support), and first agent creation. Backend: - SetupController with 5 endpoints: status (unauthenticated), templates, company, agent, complete - setup_complete boolean setting in the api namespace - Setup event constants for structured logging - Auth exclude path for /setup/status Frontend: - 5-step wizard: Welcome, Admin, Provider, Company, Agent + Complete - Setup Pinia store with status caching and step management - Router guard integration (redirect to /setup when incomplete) - Reuses existing provider form patterns and auth composables CLI: - synthorg setup command: resets setup_complete flag and opens browser Closes #452 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Pre-reviewed by 6 agents, 22 findings addressed: Backend (6 fixes): - Replace bare string event name with SETUP_AGENTS_READ_FALLBACK constant - Add warning log for settings service unavailability in status check - Add MemoryError/RecursionError guards on except Exception blocks - Wire budget_limit_monthly into agent config dict - Type _get_existing_agents parameter as SettingsService - Use spread operator instead of in-place list mutation Frontend (6 fixes): - Remove duplicate ProviderTestButton, use single test button with dedicated loading state - Restore requiresAuth: false on /setup route for fresh install flow - Replace @change with watch on role Select in SetupAgent - Await fetchTemplates in SetupCompany onMounted - Add named SetupCompanyResponse/SetupAgentResponse types - Add MAX_STEPS bounds check in setup store nextStep CLI (3 fixes): - Drain response body before close with io.LimitReader - Reap browser child process with goroutine Wait - Remove docker.Detect error double-wrapping Docs (7 fixes): - CLAUDE.md: add setup controller, setup/ components, setup store, setup CLI command, events.setup to logging examples - operations.md: add setup CLI command and /api/v1/setup API surface Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Backend (setup.py): - Add asyncio.Lock for agents list read-modify-write (race condition) - Add idempotency guard on mutating endpoints (ConflictError if already complete) - Fix NotBlankStr on template_name, personality_preset, response DTOs - Fix TemplateInfoResponse.source to Literal["builtin", "user"] - Add WARNING logs before all raise statements - Replace getattr with direct attribute access on typed Pydantic models - Extract _validate_provider_and_model and _build_agent_config helpers - Add SetupCompleteResponse typed DTO (replaces dict[str, bool]) - Fix _get_existing_agents: propagate JSONDecodeError, only catch missing entry - Move company name persistence after template validation - Always clear stale departments on company creation - Normalize personality preset to canonical lowercase in validator - Fix docstrings and comments for accuracy Backend (app.py): - Always ensure /setup/status in auth exclude paths even with custom config Backend (events/setup.py): - Add 7 new event constants for error/warning paths CLI (setup.go): - Replace http.DefaultClient with dedicated setupClient + redirect blocking - Add JWT authentication on DELETE request (buildLocalJWT) - Add errOut for stderr output - Fix psOut check for modern Compose JSON output - Return error on flag-reset failure instead of silent continue - Add URL scheme+host validation in openBrowser - Add directory path to "not initialized" error message - Wrap openBrowser error with context Frontend: - Add statusLoaded flag to setup store (fail-closed on fetch errors) - Make authGuard async, bootstrap fetchStatus before routing decisions - Fix nextStep to accept maxSteps parameter (was hardcoded MAX_STEPS=10) - Clamp setStep to valid range - Surface provider/template fetch errors in component error displays - Clear selectedModel when provider changes in SetupAgent - Separate createAgent from markComplete for retry safety - Redirect to dashboard in SetupPage if setup not needed after fetch Tests: - Add happy-path test for successful agent creation - Fix docstring "returns 400" -> "returns 422" - Rename test_requires_auth -> test_observer_can_read_templates - Remove duplicate imports, move import json to top - Update guards.test.ts for async authGuard Docs: - Update user_guide.md first-run section for setup wizard - Add synthorg setup to CLI commands lists - Fix settings docstring "10 settings" -> "11 settings" - Add setup wizard mention to README Quick Start Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
SettingEntry.value is typed as str (never None), so the if entry.value is None guard was flagged as unreachable by mypy. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
d9a27ee to
d1309ec
Compare
There was a problem hiding this comment.
Actionable comments posted: 10
♻️ Duplicate comments (3)
web/src/components/setup/SetupAdmin.vue (1)
7-7:⚠️ Potential issue | 🟠 MajorDon't hard-code the password minimum in the wizard.
api.auth.min_password_lengthis runtime-editable, but this form still validates and renders helper text from the staticMIN_PASSWORD_LENGTHconstant. Any override will make the client reject valid passwords or allow ones the API rejects. If the server-driven value is not available here yet, the safer fallback is to drop the client-side minimum-length check instead of enforcing the wrong number.Also applies to: 32-33, 78-79
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/setup/SetupAdmin.vue` at line 7, The component currently imports and uses the static MIN_PASSWORD_LENGTH constant in SetupAdmin.vue (remove the import and any references to MIN_PASSWORD_LENGTH) and instead read the runtime value api.auth.min_password_length (or the equivalent store/prop used elsewhere) for rendering helper text and performing validation in the password-related computed/validation functions; if api.auth.min_password_length is not yet available, do not enforce a client-side minimum length check (drop the min-length validation and helper enforcement) so the client won't reject passwords that the server accepts. Ensure all occurrences (helper text and validation logic that referenced MIN_PASSWORD_LENGTH) are updated to use api.auth.min_password_length or skipped when unset.src/synthorg/api/controllers/setup.py (2)
233-235:⚠️ Potential issue | 🟠 MajorAlign setup gating with configured providers and the wizard’s actual completion criteria.
has_providersand/setup/completeare both keyed offprovider_registry, even though the setup flow itself validates providers from the configured-provider source used bycreate_agent(). That can misclassify a non-fresh install as missing providers, andcomplete_setup()can still hide the wizard before any company or first agent exists. Gate both checks on configured providers, then require persisted company + agent state before settingsetup_complete.Also applies to: 423-443
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/synthorg/api/controllers/setup.py` around lines 233 - 235, has_providers is currently derived from app_state.provider_registry which mismatches the setup wizard's validation; change the gating logic in the setup controller to compute has_providers from the configured-provider source used by create_agent() (instead of app_state.provider_registry and app_state.has_provider_registry) and update the /setup/complete logic inside complete_setup() so it also checks persisted company and first-agent existence before setting setup_complete; specifically reference and modify the has_providers assignment, the code paths that call create_agent(), and the complete_setup()/setup_complete flow to require both a persisted Company and at least one persisted Agent before marking setup_complete.
622-628:⚠️ Potential issue | 🟠 MajorDon't collapse arbitrary settings read failures into an empty agent list.
except Exception:here still turns repository/service errors into[], andcreate_agent()will then rewritecompany.agentswith only the new entry. Restrict the fallback to the explicit unset-setting case and let other read failures abort the request.As per coding guidelines, "Handle errors explicitly; never silently swallow exceptions".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/synthorg/api/controllers/setup.py` around lines 622 - 628, The code is swallowing all exceptions when calling settings_svc.get_entry("company", "agents"), turning repository/service errors into an empty list; change the except block to only catch the explicit "entry not set" error (e.g., SettingsNotFoundError / EntryNotFound / KeyError that your settings service actually raises) and keep the current fallback (logger.debug(SETUP_AGENTS_READ_FALLBACK, reason="entry_not_found"); return []); for any other exception re-raise (or let it propagate) so service/repo errors abort the request; reference settings_svc.get_entry, SETUP_AGENTS_READ_FALLBACK and create_agent to locate and validate the behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/synthorg/api/controllers/setup.py`:
- Around line 82-109: Add a company description field to the setup
request/response and persistence: extend SetupCompanyRequest to include a
description: NotBlankStr (or NotBlankStr | None if optional) with appropriate
Field(max_length=...) and add the matching description attribute to
SetupCompanyResponse, then update the code path that saves the company (the
function handling the setup payload and any persistence method which reads
SetupCompanyRequest and constructs the company record) to persist the
description into the company entity; ensure validation and tests are updated to
reflect the new description attribute.
- Around line 217-218: The code currently sets needs_admin by checking
user_count = await persistence.users.count(), which only detects any users
instead of whether an admin exists; replace that logic to query for an admin
user explicitly (e.g., await persistence.users.find_one({ "role": "admin" }) or
await persistence.users.count({ "role": "admin" })) and set needs_admin = True
only when no admin is found (e.g., admin = await
persistence.users.find_one(...); needs_admin = admin is None, or admin_count =
await persistence.users.count({ "role": "admin" }); needs_admin = admin_count ==
0), keeping the rest of the setup flow unchanged and using the existing
persistence.users methods.
In `@src/synthorg/settings/definitions/api.py`:
- Around line 3-6: Update the module docstring that describes the settings split
to reflect the current counts: change the "Four are runtime-editable; seven are
bootstrap-only" wording to "Five are runtime-editable; six are bootstrap-only"
(since the new setting setup_complete makes runtime-editable settings = 5 and
bootstrap-only = 6); locate the docstring at the top of the module where the
settings are registered and update the numeric counts only, leaving the rest of
the description intact.
In `@web/src/__tests__/router/guards.test.ts`:
- Around line 23-34: The test suite currently hardcodes getSetupStatus to always
return needs_setup: false, so add focused tests covering the new setup-routing
branches: (1) mock getSetupStatus to return needs_setup: true and assert
navigation redirects to '/setup'; (2) mock needs_setup: true and assert that
navigating to '/setup' and '/login' is allowed (no redirect); and (3) mock
getSetupStatus to reject/throw and assert the guard fail-closes (navigation
blocked or redirected to an error route). Update the mock in the tests
referencing '@/api/endpoints/setup' and add specific test cases around the
router guard functions used in the file to cover these branches.
In `@web/src/components/setup/SetupAgent.vue`:
- Around line 64-65: The module-scoped savedAgent can leak across mounts; change
savedAgent to a component-scoped ref (e.g., const savedAgent =
ref<SetupAgentResponse | null>(null)) inside SetupAgent.vue and initialize/reset
it in onMounted (or use a Pinia store if you need cross-component persistence)
so markComplete and any retry logic read/write the component-local ref instead
of a module variable; update references to savedAgent in markComplete and
related functions to use savedAgent.value.
- Line 138: The current fragile cast for budget_limit_monthly
("budget_limit_monthly: undefined as unknown as null") should be replaced by
conditionally including the field in the request payload or explicitly setting
it to null based on intent; update the payload construction used before calling
setupApi.createAgent (and when using savedAgent) so that you either omit
budget_limit_monthly when the local budget variable is undefined/empty, or set
it to null only when you intentionally want a null value—use the
selectedRole/ROLE_DEPARTMENTS and agentName trimming logic already present to
build the payload and conditionally spread in budget_limit_monthly from the
component state instead of the current double-cast.
In `@web/src/components/setup/SetupCompany.vue`:
- Around line 26-39: handleCreate can be invoked multiple times while the first
request is pending; add a re-entrancy guard at the top of the function so if
creating.value is already true the handler returns immediately, and set
creating.value = true as early as possible (right after the guard and before
calling setupApi.createCompany) to prevent raced double submits; keep the
existing try/catch/finally logic to clear creating.value and set error, and
ensure emit('next', ...) remains in the try block so it only runs once after a
successful createCompany call.
- Around line 15-18: The setup step currently only collects companyName and
selectedTemplate (refs companyName, selectedTemplate, error, creating) and omits
the company description required by Issue `#452`; add a new reactive ref (e.g.,
companyDescription) and a corresponding input/textarea in the template bound to
it, validate it alongside companyName, and include its value as
company_description in the payload where the component currently sends
companyName and selectedTemplate to the backend (the same submit/create handler
that uses companyName and selectedTemplate). Ensure error handling/creating
state treat companyDescription the same as companyName and propagate any backend
validation errors to the existing error ref.
- Around line 43-49: The local try/catch around onMounted's call to
setup.fetchTemplates hides errors because fetchTemplates already catches and
stores failures on setup.error; remove the try/catch and instead call await
setup.fetchTemplates() and then set error.value = setup.error (or error.value =
getErrorMessage(setup.error) if you need the formatted string) so the component
surfaces store-side template-loading failures; update the onMounted block that
references setup.fetchTemplates, setup.error, and error.value accordingly.
In `@web/src/components/setup/SetupProvider.vue`:
- Around line 134-138: The code sets testPassed = true when hasProviders.value
is true without verifying connectivity; change this to run the existing provider
connection test instead of unconditionally marking success: locate the block in
SetupProvider.vue that references hasProviders, testPassed, createdProviderName
and replace the unconditional assignment with a call to the provider test
routine (the same logic used when a user clicks "Test connection" or the
function that validates providers) and only set testPassed.value = true if that
test resolves successfully (otherwise keep it false and surface the failure);
ensure createdProviderName.value is still populated from
Object.keys(store.providers) but gate progression on the test result.
---
Duplicate comments:
In `@src/synthorg/api/controllers/setup.py`:
- Around line 233-235: has_providers is currently derived from
app_state.provider_registry which mismatches the setup wizard's validation;
change the gating logic in the setup controller to compute has_providers from
the configured-provider source used by create_agent() (instead of
app_state.provider_registry and app_state.has_provider_registry) and update the
/setup/complete logic inside complete_setup() so it also checks persisted
company and first-agent existence before setting setup_complete; specifically
reference and modify the has_providers assignment, the code paths that call
create_agent(), and the complete_setup()/setup_complete flow to require both a
persisted Company and at least one persisted Agent before marking
setup_complete.
- Around line 622-628: The code is swallowing all exceptions when calling
settings_svc.get_entry("company", "agents"), turning repository/service errors
into an empty list; change the except block to only catch the explicit "entry
not set" error (e.g., SettingsNotFoundError / EntryNotFound / KeyError that your
settings service actually raises) and keep the current fallback
(logger.debug(SETUP_AGENTS_READ_FALLBACK, reason="entry_not_found"); return []);
for any other exception re-raise (or let it propagate) so service/repo errors
abort the request; reference settings_svc.get_entry, SETUP_AGENTS_READ_FALLBACK
and create_agent to locate and validate the behavior.
In `@web/src/components/setup/SetupAdmin.vue`:
- Line 7: The component currently imports and uses the static
MIN_PASSWORD_LENGTH constant in SetupAdmin.vue (remove the import and any
references to MIN_PASSWORD_LENGTH) and instead read the runtime value
api.auth.min_password_length (or the equivalent store/prop used elsewhere) for
rendering helper text and performing validation in the password-related
computed/validation functions; if api.auth.min_password_length is not yet
available, do not enforce a client-side minimum length check (drop the
min-length validation and helper enforcement) so the client won't reject
passwords that the server accepts. Ensure all occurrences (helper text and
validation logic that referenced MIN_PASSWORD_LENGTH) are updated to use
api.auth.min_password_length or skipped when unset.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 82d74db9-82b5-4c5f-b225-50b56a578cc6
📒 Files selected for processing (26)
CLAUDE.mdREADME.mdcli/cmd/setup.godocs/design/operations.mddocs/user_guide.mdsrc/synthorg/api/app.pysrc/synthorg/api/controllers/__init__.pysrc/synthorg/api/controllers/setup.pysrc/synthorg/api/openapi.pysrc/synthorg/observability/events/setup.pysrc/synthorg/settings/definitions/api.pytests/unit/api/controllers/test_setup.pytests/unit/observability/test_events.pyweb/src/__tests__/router/guards.test.tsweb/src/__tests__/views/SetupPage.test.tsweb/src/api/endpoints/setup.tsweb/src/api/types.tsweb/src/components/setup/SetupAdmin.vueweb/src/components/setup/SetupAgent.vueweb/src/components/setup/SetupCompany.vueweb/src/components/setup/SetupComplete.vueweb/src/components/setup/SetupProvider.vueweb/src/components/setup/SetupWelcome.vueweb/src/router/guards.tsweb/src/stores/setup.tsweb/src/views/SetupPage.vue
📜 Review details
⏰ 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 (Python 3.14)
- GitHub Check: CLI Test (windows-latest)
- GitHub Check: CLI Test (macos-latest)
- GitHub Check: Build Backend
- GitHub Check: Build Sandbox
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (9)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Use Pydantic v2 (BaseModel, model_validator, computed_field, ConfigDict) for all data models
Use PEP 758 except syntax:except A, B:(no parentheses) — not PEP 654 syntax
Do NOT usefrom __future__ import annotations— Python 3.14 has PEP 649 native lazy annotations
Include type hints on all public functions and classes; enforce with mypy strict mode
Use Google-style docstrings for all public classes and functions (enforced by ruff D rules)
Prefer immutability: create new objects rather than mutating existing ones; use copy.deepcopy() at construction and MappingProxyType for non-Pydantic collections
Use asyncio.TaskGroup for fan-out/fan-in parallel operations (structured concurrency) instead of bare create_task
Keep functions under 50 lines and files under 800 lines
Handle errors explicitly; never silently swallow exceptions
Validate at system boundaries (user input, external APIs, config files)
Set line length to 88 characters (enforced by ruff)
Files:
tests/unit/observability/test_events.pysrc/synthorg/api/controllers/__init__.pysrc/synthorg/api/app.pysrc/synthorg/api/openapi.pysrc/synthorg/settings/definitions/api.pysrc/synthorg/observability/events/setup.pytests/unit/api/controllers/test_setup.pysrc/synthorg/api/controllers/setup.py
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
tests/**/*.py: Mark all unit tests with@pytest.mark.unit, integration tests with@pytest.mark.integration, e2e tests with@pytest.mark.e2e, and slow tests with@pytest.mark.slow
Usepytest.mark.parametrizefor testing multiple similar cases instead of writing separate test functions
Use Hypothesis (@given+@settings) for property-based testing in Python unit tests
Never use real vendor names (Anthropic, OpenAI, Claude, GPT, etc.) in tests; use generic names like test-provider, test-small-001, or test-medium-001
Set test timeout to 30 seconds per test; use pytest-xdist with-n autofor parallel execution (ALWAYS include this flag, never run tests sequentially)
Maintain minimum 80% code coverage (enforced in CI); usepytest --cov=synthorg --cov-fail-under=80to verify locally
Mock time.monotonic() and asyncio.sleep() in timing-sensitive tests to make them deterministic instead of widening timing margins
NEVER skip, dismiss, or ignore flaky tests — always fix them fully and fundamentally
Files:
tests/unit/observability/test_events.pytests/unit/api/controllers/test_setup.py
**/*.md
📄 CodeRabbit inference engine (CLAUDE.md)
ALWAYS use
uv sync(notcd ...commands) when referencing command execution in documentation and examples
Files:
README.mddocs/user_guide.mdCLAUDE.mddocs/design/operations.md
src/synthorg/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/synthorg/**/*.py: Every module with business logic MUST import logger viafrom synthorg.observability import get_loggerthen uselogger = get_logger(__name__)
Never useimport logging,logging.getLogger(), orprint()in application code; only use the centralized get_logger function
Always use structured logging with keyword arguments:logger.info(EVENT, key=value)— never use formatted strings likelogger.info('msg %s', val)
Use event name constants from synthorg.observability.events. (e.g., API_REQUEST_STARTED from events.api, TOOL_INVOKE_START from events.tool) for all log events
Log all error paths at WARNING or ERROR level with context before raising exceptions
Log all state transitions at INFO level
Use DEBUG level logging for object creation, internal flow, and entry/exit of key functions
Use NotBlankStr (from core.types) for all identifier/name fields in models instead of manual whitespace validators
Never implement retry logic in provider driver subclasses or calling code — it is handled automatically by BaseCompletionProvider base class
Never use real vendor names (Anthropic, OpenAI, Claude, GPT, etc.) in project-owned code, docstrings, or comments; use generic names like example-provider or example-large-001
Use frozen Pydantic models for config/identity; use separate mutable-via-copy models (model_copy(update=...)) for runtime state that evolves
Use@computed_fieldfor derived values instead of storing and validating redundant fields
Files:
src/synthorg/api/controllers/__init__.pysrc/synthorg/api/app.pysrc/synthorg/api/openapi.pysrc/synthorg/settings/definitions/api.pysrc/synthorg/observability/events/setup.pysrc/synthorg/api/controllers/setup.py
**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,vue}: Use Vue 3 with PrimeVue components and Tailwind CSS for the dashboard frontend
Use Pinia stores for state management in the Vue dashboard
Use Axios client (from api/) for all backend API calls in the dashboard
Files:
web/src/components/setup/SetupWelcome.vueweb/src/components/setup/SetupComplete.vueweb/src/components/setup/SetupAdmin.vueweb/src/components/setup/SetupCompany.vueweb/src/views/SetupPage.vueweb/src/__tests__/router/guards.test.tsweb/src/api/types.tsweb/src/__tests__/views/SetupPage.test.tsweb/src/router/guards.tsweb/src/api/endpoints/setup.tsweb/src/components/setup/SetupAgent.vueweb/src/components/setup/SetupProvider.vueweb/src/stores/setup.ts
web/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
web/**/*.{ts,tsx,vue}: Run ESLint linting vianpm --prefix web run linton all dashboard code
Run vue-tsc type checking on all dashboard TypeScript and Vue files
Files:
web/src/components/setup/SetupWelcome.vueweb/src/components/setup/SetupComplete.vueweb/src/components/setup/SetupAdmin.vueweb/src/components/setup/SetupCompany.vueweb/src/views/SetupPage.vueweb/src/__tests__/router/guards.test.tsweb/src/api/types.tsweb/src/__tests__/views/SetupPage.test.tsweb/src/router/guards.tsweb/src/api/endpoints/setup.tsweb/src/components/setup/SetupAgent.vueweb/src/components/setup/SetupProvider.vueweb/src/stores/setup.ts
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.test.{ts,tsx}: Use Vitest for unit testing Vue components and TypeScript code in the dashboard
Use fast-check for property-based testing in Vue dashboard tests
Files:
web/src/__tests__/router/guards.test.tsweb/src/__tests__/views/SetupPage.test.ts
cli/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
cli/**/*.go: Use Cobra framework for CLI commands in the Go CLI binary
Use charmbracelet/lipgloss for styled CLI output and charmbracelet/huh for interactive forms in the Go CLI
Run golangci-lint, go vet, and go test for linting and testing all Go CLI code
Files:
cli/cmd/setup.go
docs/design/**/*.md
📄 CodeRabbit inference engine (CLAUDE.md)
docs/design/**/*.md: ALWAYS read the relevant docs/design/ page before implementing any feature or planning any issue; design spec is the starting point for architecture and behavior
If implementation deviates from the spec, alert the user and explain why; user decides whether to proceed or update the spec; then update the relevant design page to reflect approved deviations
Files:
docs/design/operations.md
🧠 Learnings (21)
📓 Common learnings
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to docker/{Dockerfile*,compose.yml} : Docker: Backend uses 3-stage build (builder → setup → distroless runtime), Chainguard Python, non-root (UID 65532), CIS-hardened. Web uses nginxinc/nginx-unprivileged, Vue 3 SPA with PrimeVue + Tailwind CSS, SPA routing, API/WebSocket proxy to backend.
📚 Learning: 2026-03-19T11:29:37.522Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T11:29:37.522Z
Learning: Applies to src/synthorg/**/*.py : Use event name constants from synthorg.observability.events.<domain> (e.g., API_REQUEST_STARTED from events.api, TOOL_INVOKE_START from events.tool) for all log events
Applied to files:
tests/unit/observability/test_events.pyCLAUDE.mdsrc/synthorg/observability/events/setup.py
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to src/synthorg/**/*.py : Event names: always use constants from domain-specific modules under synthorg.observability.events (e.g., PROVIDER_CALL_START from events.provider, BUDGET_RECORD_ADDED from events.budget, etc.). Import directly: `from synthorg.observability.events.<domain> import EVENT_CONSTANT`.
Applied to files:
tests/unit/observability/test_events.pyCLAUDE.mdsrc/synthorg/observability/events/setup.py
📚 Learning: 2026-03-19T11:29:37.522Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T11:29:37.522Z
Learning: Applies to **/*.{ts,tsx,vue} : Use Vue 3 with PrimeVue components and Tailwind CSS for the dashboard frontend
Applied to files:
web/src/components/setup/SetupWelcome.vueweb/src/components/setup/SetupComplete.vueweb/src/components/setup/SetupAdmin.vueweb/src/components/setup/SetupCompany.vueCLAUDE.mdweb/src/__tests__/views/SetupPage.test.ts
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to web/** : Web dashboard: Node.js 20+, dependencies in web/package.json (Vue 3, PrimeVue, Tailwind CSS, Pinia, VueFlow, ECharts, Axios, vue-draggable-plus, Vitest, fast-check, ESLint, vue-tsc).
Applied to files:
web/src/components/setup/SetupWelcome.vueweb/src/components/setup/SetupComplete.vueCLAUDE.md
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to web/src/components/** : Vue components organized by feature (agents/, approvals/, budget/, common/, dashboard/, layout/, messages/, org-chart/, tasks/).
Applied to files:
web/src/components/setup/SetupComplete.vueweb/src/components/setup/SetupCompany.vueweb/src/views/SetupPage.vueCLAUDE.mdweb/src/components/setup/SetupAgent.vue
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to docker/{Dockerfile*,compose.yml} : Docker: Backend uses 3-stage build (builder → setup → distroless runtime), Chainguard Python, non-root (UID 65532), CIS-hardened. Web uses nginxinc/nginx-unprivileged, Vue 3 SPA with PrimeVue + Tailwind CSS, SPA routing, API/WebSocket proxy to backend.
Applied to files:
docs/user_guide.mdCLAUDE.md
📚 Learning: 2026-03-19T11:29:37.522Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T11:29:37.522Z
Learning: Applies to src/synthorg/**/*.py : Every module with business logic MUST import logger via `from synthorg.observability import get_logger` then use `logger = get_logger(__name__)`
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to src/synthorg/**/*.py : Every module with business logic MUST have: `from synthorg.observability import get_logger` then `logger = get_logger(__name__)`. Never use `import logging` / `logging.getLogger()` / `print()` in application code. Variable name: always `logger` (not `_logger`, not `log`).
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to src/synthorg/**/*.py : DEBUG logging for object creation, internal flow, entry/exit of key functions.
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-19T11:29:37.522Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T11:29:37.522Z
Learning: Applies to **/*.{ts,tsx,vue} : Use Pinia stores for state management in the Vue dashboard
Applied to files:
CLAUDE.mdweb/src/__tests__/router/guards.test.tsweb/src/__tests__/views/SetupPage.test.tsweb/src/stores/setup.ts
📚 Learning: 2026-03-19T11:29:37.522Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T11:29:37.522Z
Learning: Applies to src/synthorg/**/*.py : Always use structured logging with keyword arguments: `logger.info(EVENT, key=value)` — never use formatted strings like `logger.info('msg %s', val)`
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to src/synthorg/**/*.py : Structured kwargs in logging: always `logger.info(EVENT, key=value)` — never `logger.info('msg %s', val)`.
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-19T11:29:37.522Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T11:29:37.522Z
Learning: Applies to src/synthorg/**/*.py : Never use `import logging`, `logging.getLogger()`, or `print()` in application code; only use the centralized get_logger function
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to src/synthorg/**/*.py : All error paths must log at WARNING or ERROR with context before raising.
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to src/synthorg/**/*.py : All state transitions must log at INFO.
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-19T11:29:37.522Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T11:29:37.522Z
Learning: Applies to **/*.test.{ts,tsx} : Use fast-check for property-based testing in Vue dashboard tests
Applied to files:
web/src/__tests__/views/SetupPage.test.ts
📚 Learning: 2026-03-19T11:29:37.522Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T11:29:37.522Z
Learning: Applies to **/*.test.{ts,tsx} : Use Vitest for unit testing Vue components and TypeScript code in the dashboard
Applied to files:
web/src/__tests__/views/SetupPage.test.ts
📚 Learning: 2026-03-19T11:29:37.522Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T11:29:37.522Z
Learning: Applies to cli/**/*.go : Use Cobra framework for CLI commands in the Go CLI binary
Applied to files:
cli/cmd/setup.go
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to cli/** : CLI: Go 1.26+, dependencies in cli/go.mod (Cobra, charmbracelet/huh).
Applied to files:
cli/cmd/setup.godocs/design/operations.md
📚 Learning: 2026-03-19T11:29:37.522Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T11:29:37.522Z
Learning: Applies to src/synthorg/**/*.py : Use NotBlankStr (from core.types) for all identifier/name fields in models instead of manual whitespace validators
Applied to files:
src/synthorg/api/controllers/setup.py
🧬 Code graph analysis (9)
src/synthorg/api/controllers/__init__.py (1)
src/synthorg/api/controllers/setup.py (1)
SetupController(191-447)
src/synthorg/settings/definitions/api.py (2)
src/synthorg/settings/registry.py (1)
register(30-50)src/synthorg/settings/models.py (1)
SettingDefinition(34-140)
web/src/__tests__/router/guards.test.ts (2)
web/src/stores/setup.ts (1)
useSetupStore(7-97)web/src/router/guards.ts (1)
authGuard(18-79)
web/src/api/types.ts (1)
src/synthorg/api/controllers/setup.py (6)
SetupStatusResponse(48-61)TemplateInfoResponse(64-79)SetupCompanyRequest(82-93)SetupAgentRequest(112-153)SetupCompanyResponse(96-109)SetupAgentResponse(156-173)
cli/cmd/setup.go (3)
cli/internal/config/state.go (2)
Load(53-91)State(16-28)cli/internal/ui/ui.go (1)
NewUI(58-70)cli/internal/docker/client.go (2)
Detect(34-66)ComposeExecOutput(100-107)
web/src/api/endpoints/setup.ts (3)
src/synthorg/api/controllers/setup.py (6)
SetupStatusResponse(48-61)TemplateInfoResponse(64-79)SetupCompanyRequest(82-93)SetupCompanyResponse(96-109)SetupAgentRequest(112-153)SetupAgentResponse(156-173)web/src/api/types.ts (7)
SetupStatusResponse(824-828)ApiResponse(139-141)TemplateInfoResponse(830-835)SetupCompanyRequest(837-840)SetupCompanyResponse(853-857)SetupAgentRequest(842-851)SetupAgentResponse(859-865)web/src/api/client.ts (2)
apiClient(12-16)unwrap(70-80)
tests/unit/api/controllers/test_setup.py (2)
src/synthorg/api/controllers/setup.py (4)
SetupAgentRequest(112-153)SetupCompanyRequest(82-93)SetupStatusResponse(48-61)_extract_template_departments(553-600)src/synthorg/api/errors.py (1)
NotFoundError(167-175)
web/src/stores/setup.ts (3)
src/synthorg/api/controllers/setup.py (2)
SetupStatusResponse(48-61)TemplateInfoResponse(64-79)web/src/api/types.ts (2)
SetupStatusResponse(824-828)TemplateInfoResponse(830-835)web/src/utils/errors.ts (1)
getErrorMessage(17-69)
src/synthorg/api/controllers/setup.py (4)
src/synthorg/api/errors.py (3)
ApiValidationError(178-186)ConflictError(189-197)NotFoundError(167-175)src/synthorg/api/state.py (4)
AppState(40-460)has_provider_registry(323-325)provider_registry(328-333)provider_management(281-286)src/synthorg/settings/service.py (2)
SettingsService(152-637)get_entry(275-298)src/synthorg/templates/presets.py (1)
get_personality_preset(406-428)
🪛 GitHub Check: CodeQL
cli/cmd/setup.go
[failure] 59-59: Uncontrolled data used in path expression
This path depends on a user-provided value.
This path depends on a user-provided value.
This path depends on a user-provided value.
This path depends on a user-provided value.
🪛 LanguageTool
CLAUDE.md
[style] ~202-~202: Since ownership is already implied, this phrasing may be redundant.
Context: ...fromevents.setup). Each domain has its own module -- see src/synthorg/observabili...
(PRP_OWN)
🔇 Additional comments (24)
web/src/api/types.ts (1)
821-865: LGTM!The Setup-related TypeScript interfaces correctly mirror their backend Pydantic counterparts from
src/synthorg/api/controllers/setup.py. The field names, types, and nullable annotations are properly aligned.tests/unit/observability/test_events.py (1)
232-232: LGTM!The test correctly expects the new
setupdomain module, aligning with the newly addedsrc/synthorg/observability/events/setup.pyfor first-run setup event constants.src/synthorg/api/openapi.py (1)
50-55: LGTM!Adding
/setup/statusto_PUBLIC_PATH_SUFFIXEScorrectly marks this endpoint as unauthenticated in the OpenAPI spec, consistent with the controller implementation whereget_statushas no guards and the middleware exclusion inapp.py.src/synthorg/api/controllers/__init__.py (1)
23-23: LGTM!The
SetupControlleris correctly imported, added toALL_CONTROLLERS, and exported via__all__, following the established pattern for controller registration.Also applies to: 46-46, 71-71
src/synthorg/api/app.py (1)
647-668: LGTM!The middleware exclusion logic correctly ensures
/setup/statusis always publicly accessible, handling both default and customexclude_pathsconfigurations. The defensive check on lines 662-663 prevents accidental lockout when custom paths are provided via config.README.md (1)
99-103: LGTM!The README clearly documents the new
synthorg setupcommand and provides helpful guidance for fresh installs, directing users to the web-based setup wizard.web/src/components/setup/SetupWelcome.vue (1)
1-31: LGTM!The welcome step component follows Vue 3 Composition API patterns with proper TypeScript typing for emits. PrimeVue
Buttonis correctly configured, and Tailwind CSS classes are used appropriately for styling. As per coding guidelines, this uses Vue 3 with PrimeVue components and Tailwind CSS for the dashboard frontend.docs/design/operations.md (1)
977-977: LGTM!The documentation accurately reflects the new CLI
setupcommand and the/api/v1/setupAPI surface, including the public status check, template listing, company/agent creation, and completion gating functionality.Also applies to: 1001-1001
web/src/__tests__/views/SetupPage.test.ts (2)
140-165: Use stable selectors for wizard chrome elements.The
.rounded-fullselector (Line 144) is coupled to Tailwind internals, andtoContain('S')(Line 164) is too loose—it will pass if the logo disappears since other text likely contains the letter 'S'. Consider usingdata-testidattributes or accessible labels for more reliable assertions.
108-158: LGTM - test structure covers core wizard flow.The test suite appropriately validates the multi-step wizard rendering and navigation. The async handling with
flushPromises()is correct, and the mocks are well-structured for the setup API surface.web/src/api/endpoints/setup.ts (1)
1-37: LGTM - clean API wrapper implementation.The endpoint wrappers follow a consistent pattern, use proper TypeScript typing, and correctly leverage the shared
apiClientandunwraputilities. The comment aboutgetSetupStatusbeing public is helpful for understanding the auth behavior.web/src/router/guards.ts (2)
31-33: Good fix - guard now eagerly fetches setup status.The guard correctly fetches status when
nulland not already loading, addressing the previous concern about stale/missing data. The fail-closed design (defaultingisSetupNeededtotruewhen fetch fails) is appropriate for security.
35-52: LGTM - setup routing logic is correct.The priority ordering is clear: setup-needed users are funneled to
/setup(allowing/loginfor auth), and post-setup visitors to/setupare redirected to/. This aligns with the PR objectives.cli/cmd/setup.go (4)
72-75: Good fix - JSON output detection now handles empty arrays.The check
psOut == "" || psOut == "[]" || psOut == "[]\n"correctly handles the case wheredocker compose ps --format jsonreturns an empty JSON array when no containers are running.
103-121: Good fix - DELETE request now uses JWT authentication.The
resetSetupFlagfunction now includes a Bearer token viabuildLocalJWT(state.JWTSecret), addressing the previous concern about unauthenticated DELETE requests to a protected endpoint.
124-153: LGTM - openBrowser has appropriate security controls.The URL validation correctly restricts to
localhost/127.0.0.1hosts andhttp/httpsschemes only, preventing arbitrary command execution. The goroutine to reap the child process prevents zombies.
54-61: Path sanitization is properly implemented and no action is required.The
safeStateDir()function callsconfig.SecurePath()which validates the path usingfilepath.Clean()(removes traversal sequences like..) andfilepath.IsAbs()(enforces absolute paths). This prevents path traversal attacks. The CodeQL warning on line 59 is a false positive.src/synthorg/observability/events/setup.py (1)
1-53: LGTM - well-structured event constants.The event naming follows the established
<domain>.<entity>.<action>convention, and the module provides comprehensive coverage for setup flow observability including success paths, error conditions, and edge cases (corrupted data, missing providers, etc.).web/src/components/setup/SetupAgent.vue (2)
110-114: Good fix - model selection cleared on provider change.The watcher correctly clears
selectedModelwhenselectedProviderchanges, preventing submission of a model that's no longer visible in the filtered dropdown.
127-143: Good fix - agent creation separated from markComplete retries.The
savedAgent ?? awaitpattern correctly prevents duplicate agent creation on retry. However, verify the type coercion on line 138.web/src/components/setup/SetupProvider.vue (1)
128-139: Good fix - error handling added for initial provider/preset loading.The
onMountedhook now catches and surfaces errors fromfetchPresets()andfetchProviders(), addressing the previous concern about dead-ending the wizard step.web/src/stores/setup.ts (3)
16-22: Good fix - fail-closed design for setup status.The computed properties correctly return
true(setup needed) whenstatusLoadedisfalse, ensuring the guard fails closed on fetch errors rather than bypassing the setup flow.
60-63: Good fix - step bounds are now clamped.The
setStepfunction properly clamps the step number to valid bounds[0, maxSteps-1], preventing navigation to non-existent wizard steps.
1-97: LGTM - well-structured Pinia store.The store correctly manages setup wizard state with proper fail-closed semantics, step bounds validation, and error handling. The separation of
statusandstatusLoadedcleanly handles the "not yet fetched" vs "fetch failed" vs "fetch succeeded" states.
| class SetupCompanyRequest(BaseModel): | ||
| """Company creation payload for first-run setup. | ||
|
|
||
| Attributes: | ||
| company_name: Company display name. | ||
| template_name: Optional template to apply (None = blank company). | ||
| """ | ||
|
|
||
| model_config = ConfigDict(frozen=True) | ||
|
|
||
| company_name: NotBlankStr = Field(max_length=200) | ||
| template_name: NotBlankStr | None = Field(default=None, max_length=100) | ||
|
|
||
|
|
||
| class SetupCompanyResponse(BaseModel): | ||
| """Company creation result. | ||
|
|
||
| Attributes: | ||
| company_name: The company name that was set. | ||
| template_applied: Name of the template that was applied, if any. | ||
| department_count: Number of departments created. | ||
| """ | ||
|
|
||
| model_config = ConfigDict(frozen=True) | ||
|
|
||
| company_name: NotBlankStr | ||
| template_applied: NotBlankStr | None | ||
| department_count: int = Field(ge=0) |
There was a problem hiding this comment.
The company step still has no way to save a description.
The linked setup flow requires company name and description, but this request/response contract and persistence path only handle company_name. As shipped, the wizard cannot persist that required field at all.
Also applies to: 325-346
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/synthorg/api/controllers/setup.py` around lines 82 - 109, Add a company
description field to the setup request/response and persistence: extend
SetupCompanyRequest to include a description: NotBlankStr (or NotBlankStr | None
if optional) with appropriate Field(max_length=...) and add the matching
description attribute to SetupCompanyResponse, then update the code path that
saves the company (the function handling the setup payload and any persistence
method which reads SetupCompanyRequest and constructs the company record) to
persist the description into the company entity; ensure validation and tests are
updated to reflect the new description attribute.
| user_count = await persistence.users.count() | ||
| needs_admin = user_count == 0 |
There was a problem hiding this comment.
Derive needs_admin from admin presence, not total user count.
The response contract says this flag is about whether an admin exists, but users.count() only tells you whether any user exists. An instance with observer/service users and no admin will skip the admin step and leave the installation without an administrator.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/synthorg/api/controllers/setup.py` around lines 217 - 218, The code
currently sets needs_admin by checking user_count = await
persistence.users.count(), which only detects any users instead of whether an
admin exists; replace that logic to query for an admin user explicitly (e.g.,
await persistence.users.find_one({ "role": "admin" }) or await
persistence.users.count({ "role": "admin" })) and set needs_admin = True only
when no admin is found (e.g., admin = await persistence.users.find_one(...);
needs_admin = admin is None, or admin_count = await persistence.users.count({
"role": "admin" }); needs_admin = admin_count == 0), keeping the rest of the
setup flow unchanged and using the existing persistence.users methods.
| // Mock setup API -- guard now calls fetchStatus on first navigation | ||
| vi.mock('@/api/endpoints/setup', () => ({ | ||
| getSetupStatus: vi.fn().mockResolvedValue({ | ||
| needs_admin: false, | ||
| needs_setup: false, | ||
| has_providers: true, | ||
| }), | ||
| listTemplates: vi.fn().mockResolvedValue([]), | ||
| createCompany: vi.fn(), | ||
| createAgent: vi.fn(), | ||
| completeSetup: vi.fn(), | ||
| })) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Add direct coverage for the new setup-routing branches.
This suite hard-codes needs_setup = false in every case, so the new guard behavior that redirects fresh installs to /setup, allows /setup and /login, and fail-closes when fetchStatus() errors is never asserted here. Those are the highest-risk branches introduced by the guard change.
Also applies to: 54-66
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/__tests__/router/guards.test.ts` around lines 23 - 34, The test suite
currently hardcodes getSetupStatus to always return needs_setup: false, so add
focused tests covering the new setup-routing branches: (1) mock getSetupStatus
to return needs_setup: true and assert navigation redirects to '/setup'; (2)
mock needs_setup: true and assert that navigating to '/setup' and '/login' is
allowed (no redirect); and (3) mock getSetupStatus to reject/throw and assert
the guard fail-closes (navigation blocked or redirected to an error route).
Update the mock in the tests referencing '@/api/endpoints/setup' and add
specific test cases around the router guard functions used in the file to cover
these branches.
| model_provider: provider, | ||
| model_id: modelId, | ||
| department: ROLE_DEPARTMENTS[selectedRole.value] ?? 'engineering', | ||
| budget_limit_monthly: undefined as unknown as null, |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Type coercion for budget_limit_monthly is fragile.
The cast undefined as unknown as null works around TypeScript but may cause issues if the API strictly validates null vs undefined. Consider either omitting the field when undefined, or using a proper conditional spread.
♻️ Proposed fix
department: ROLE_DEPARTMENTS[selectedRole.value] ?? 'engineering',
- budget_limit_monthly: undefined as unknown as null,
+ budget_limit_monthly: null,
})Or, if the intent is to omit the field entirely:
const payload = {
name: agentName.value.trim(),
// ... other fields
department: ROLE_DEPARTMENTS[selectedRole.value] ?? 'engineering',
}
// Only include budget if set
const result = savedAgent.value ?? await setupApi.createAgent(payload)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/components/setup/SetupAgent.vue` at line 138, The current fragile
cast for budget_limit_monthly ("budget_limit_monthly: undefined as unknown as
null") should be replaced by conditionally including the field in the request
payload or explicitly setting it to null based on intent; update the payload
construction used before calling setupApi.createAgent (and when using
savedAgent) so that you either omit budget_limit_monthly when the local budget
variable is undefined/empty, or set it to null only when you intentionally want
a null value—use the selectedRole/ROLE_DEPARTMENTS and agentName trimming logic
already present to build the payload and conditionally spread in
budget_limit_monthly from the component state instead of the current
double-cast.
| const companyName = ref('') | ||
| const selectedTemplate = ref<string | null>(null) | ||
| const error = ref<string | null>(null) | ||
| const creating = ref(false) |
There was a problem hiding this comment.
This step drops the company description from the setup flow.
Issue #452 requires company creation to capture name/description, but this component only collects company_name and template_name. Users currently have no way to provide the initial description, and the backend never receives it during setup.
Also applies to: 31-34, 61-69
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/components/setup/SetupCompany.vue` around lines 15 - 18, The setup
step currently only collects companyName and selectedTemplate (refs companyName,
selectedTemplate, error, creating) and omits the company description required by
Issue `#452`; add a new reactive ref (e.g., companyDescription) and a
corresponding input/textarea in the template bound to it, validate it alongside
companyName, and include its value as company_description in the payload where
the component currently sends companyName and selectedTemplate to the backend
(the same submit/create handler that uses companyName and selectedTemplate).
Ensure error handling/creating state treat companyDescription the same as
companyName and propagate any backend validation errors to the existing error
ref.
| if (hasProviders.value) { | ||
| testPassed.value = true | ||
| const names = Object.keys(store.providers) | ||
| createdProviderName.value = names[0] ?? null | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider whether existing providers should still require a connection test.
When an existing provider is detected, testPassed is set to true without actually testing the connection. This allows users to proceed, but a broken provider configuration could go undetected until the agent creation step fails.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/components/setup/SetupProvider.vue` around lines 134 - 138, The code
sets testPassed = true when hasProviders.value is true without verifying
connectivity; change this to run the existing provider connection test instead
of unconditionally marking success: locate the block in SetupProvider.vue that
references hasProviders, testPassed, createdProviderName and replace the
unconditional assignment with a call to the provider test routine (the same
logic used when a user clicks "Test connection" or the function that validates
providers) and only set testPassed.value = true if that test resolves
successfully (otherwise keep it false and surface the failure); ensure
createdProviderName.value is still populated from Object.keys(store.providers)
but gate progression on the test result.
- Fix docstring counts: 5 runtime-editable, 6 bootstrap-only (was wrong) - Use SettingNotFoundError instead of broad Exception in _get_existing_agents - Make savedAgent a component-scoped ref (prevents cross-mount leak) - Fix budget_limit_monthly: use null directly instead of fragile cast - Add double-submit guard to SetupCompany.handleCreate - Simplify fetchTemplates onMounted: surface store errors without dead try/catch - Add 3 setup-routing guard tests (setup-needed redirect, allow /setup+/login, redirect /setup to / when complete) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (5)
web/src/__tests__/router/guards.test.ts (1)
54-65:⚠️ Potential issue | 🟡 MinorExercise the real
/loginsetup-needed path.Every test forces
statusLoaded = true, soauthGuard()never hits the first-navigationgetSetupStatus()branch, and the case at Line 204 only proves/setup. Add onestatusLoaded = falsecase that actually navigates to/loginwhileneeds_setupis true.Also applies to: 204-216
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/__tests__/router/guards.test.ts` around lines 54 - 65, Test suite currently forces setup.statusLoaded = true so authGuard() never exercises the first-navigation branch that calls getSetupStatus(); add one test case that sets setup.statusLoaded = false, sets setup.status.needs_setup = true, invokes authGuard() with a navigation to '/login' and asserts next was called with expected redirect behavior so the real `/login` setup-needed path is exercised; locate setup store via useSetupStore() and the guard via authGuard to implement this single-case addition and ensure it does not alter the other pre-populated setup cases.src/synthorg/api/controllers/setup.py (2)
418-445:⚠️ Potential issue | 🟠 MajorDon't finalize setup before the wizard prerequisites exist.
This endpoint only checks for providers. A direct call can set
setup_completebefore the company and first agent are created, which hides the wizard while the installation is still incomplete.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/synthorg/api/controllers/setup.py` around lines 418 - 445, complete_setup currently only checks providers and marks setup_complete, allowing direct calls to hide the wizard before required resources exist; update complete_setup to also verify that the initial prerequisites exist by checking app_state for a company and for at least one agent (use app_state.company_service / app_state.agent_service or equivalent services on AppState) before calling settings_service.set("api","setup_complete","true"), and if either the company or an agent is missing log a warning and raise ApiValidationError with a clear message preventing completion.
218-219:⚠️ Potential issue | 🟠 Major
needs_adminis checking for any user, not an admin.
users.count()only answers “is there a user?”. If a seeded/service user exists but no administrator, the wizard will skip the admin step and leave the install without one.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/synthorg/api/controllers/setup.py` around lines 218 - 219, The code sets needs_admin by checking all users via persistence.users.count(), which only detects any user, not admin users; change it to count only admin users (e.g., call persistence.users.count(filter={"role":"admin"}) or the appropriate admin predicate used elsewhere) and set needs_admin = (admin_count == 0) so the setup wizard requires an administrator only when no admin accounts exist; update the variable name (admin_count) and reference to persistence.users.count(...) accordingly.web/src/components/setup/SetupCompany.vue (1)
15-20:⚠️ Potential issue | 🟠 MajorThis step still has no description field.
The local state, form, and
createCompany()payload only carrycompanyName, so the wizard still can't collect or save the initial company description.Also applies to: 31-35, 61-69
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/setup/SetupCompany.vue` around lines 15 - 20, The setup step is missing a company description field and it isn't included in the component state or payload; add a new ref like description = ref<string | null>('') alongside companyName/selectedTemplate/error/creating, bind that ref to the form input in SetupCompany.vue, include description in the payload sent by createCompany(), and update the isValid computed (or validation logic used before submit) if you want description to be required; ensure the symbols companyName, description, isValid, and createCompany are updated consistently.web/src/components/setup/SetupAgent.vue (1)
57-63:⚠️ Potential issue | 🟠 MajorThis step still can't collect the initial agent budget.
The form has no budget control, and Line 139 hard-codes
budget_limit_monthly: null, so the wizard cannot satisfy the required “basic budget” input.Also applies to: 131-140, 170-249
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/setup/SetupAgent.vue` around lines 57 - 63, The form is missing a budget field and the submit payload currently hard-codes budget_limit_monthly: null, so add a reactive budget ref (e.g., budgetLimitMonthly = ref<number | null>(null)), wire it into the template as a numeric input/control alongside agentName/selectedRole/etc., validate it in the submit flow (ensure a basic budget is provided) and replace the hard-coded budget_limit_monthly: null in the create/submit handler (the function that builds the agent payload) with budgetLimitMonthly.value so the wizard sends the actual initial budget; also update any related validation/error ref (error) and the creating flag use as needed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/synthorg/api/controllers/setup.py`:
- Around line 479-487: The current catch-all except Exception around
settings_svc.get_entry("api", "setup_complete") converts any failure into "setup
incomplete" which masks backend failures; change the except block to catch only
the expected "missing setting" error type from settings_svc (e.g.,
SettingsNotFoundError or LookupError/KeyError depending on the service) and
return False in that branch, while letting all other exceptions propagate (or
log and re-raise) so that unexpected errors are surfaced; keep the rest of the
logic that returns entry.value == "true" unchanged.
In `@web/src/components/setup/SetupAgent.vue`:
- Around line 117-149: handleCreate can be re-entered concurrently causing
duplicate POSTs; add an early re-entrancy guard by checking creating.value at
the start of handleCreate and returning immediately if true, ensuring the
function sets creating.value = true synchronously before any awaits and before
calling setupApi.createAgent; reference handleCreate, creating, savedAgent, and
setupApi.createAgent and apply the same guard to the other handler mentioned
(lines 242-248) so subsequent clicks/Enter presses are ignored while a create is
in progress.
---
Duplicate comments:
In `@src/synthorg/api/controllers/setup.py`:
- Around line 418-445: complete_setup currently only checks providers and marks
setup_complete, allowing direct calls to hide the wizard before required
resources exist; update complete_setup to also verify that the initial
prerequisites exist by checking app_state for a company and for at least one
agent (use app_state.company_service / app_state.agent_service or equivalent
services on AppState) before calling
settings_service.set("api","setup_complete","true"), and if either the company
or an agent is missing log a warning and raise ApiValidationError with a clear
message preventing completion.
- Around line 218-219: The code sets needs_admin by checking all users via
persistence.users.count(), which only detects any user, not admin users; change
it to count only admin users (e.g., call
persistence.users.count(filter={"role":"admin"}) or the appropriate admin
predicate used elsewhere) and set needs_admin = (admin_count == 0) so the setup
wizard requires an administrator only when no admin accounts exist; update the
variable name (admin_count) and reference to persistence.users.count(...)
accordingly.
In `@web/src/__tests__/router/guards.test.ts`:
- Around line 54-65: Test suite currently forces setup.statusLoaded = true so
authGuard() never exercises the first-navigation branch that calls
getSetupStatus(); add one test case that sets setup.statusLoaded = false, sets
setup.status.needs_setup = true, invokes authGuard() with a navigation to
'/login' and asserts next was called with expected redirect behavior so the real
`/login` setup-needed path is exercised; locate setup store via useSetupStore()
and the guard via authGuard to implement this single-case addition and ensure it
does not alter the other pre-populated setup cases.
In `@web/src/components/setup/SetupAgent.vue`:
- Around line 57-63: The form is missing a budget field and the submit payload
currently hard-codes budget_limit_monthly: null, so add a reactive budget ref
(e.g., budgetLimitMonthly = ref<number | null>(null)), wire it into the template
as a numeric input/control alongside agentName/selectedRole/etc., validate it in
the submit flow (ensure a basic budget is provided) and replace the hard-coded
budget_limit_monthly: null in the create/submit handler (the function that
builds the agent payload) with budgetLimitMonthly.value so the wizard sends the
actual initial budget; also update any related validation/error ref (error) and
the creating flag use as needed.
In `@web/src/components/setup/SetupCompany.vue`:
- Around line 15-20: The setup step is missing a company description field and
it isn't included in the component state or payload; add a new ref like
description = ref<string | null>('') alongside
companyName/selectedTemplate/error/creating, bind that ref to the form input in
SetupCompany.vue, include description in the payload sent by createCompany(),
and update the isValid computed (or validation logic used before submit) if you
want description to be required; ensure the symbols companyName, description,
isValid, and createCompany are updated consistently.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: d57c6b14-acfb-436d-9d40-13befe51bb24
📒 Files selected for processing (5)
src/synthorg/api/controllers/setup.pysrc/synthorg/settings/definitions/api.pyweb/src/__tests__/router/guards.test.tsweb/src/components/setup/SetupAgent.vueweb/src/components/setup/SetupCompany.vue
📜 Review details
⏰ 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: Build Web
- GitHub Check: Build Sandbox
- GitHub Check: Build Backend
- GitHub Check: CLI Test (windows-latest)
- GitHub Check: Test (Python 3.14)
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Use Pydantic v2 (BaseModel, model_validator, computed_field, ConfigDict) for all data models
Use PEP 758 except syntax:except A, B:(no parentheses) — not PEP 654 syntax
Do NOT usefrom __future__ import annotations— Python 3.14 has PEP 649 native lazy annotations
Include type hints on all public functions and classes; enforce with mypy strict mode
Use Google-style docstrings for all public classes and functions (enforced by ruff D rules)
Prefer immutability: create new objects rather than mutating existing ones; use copy.deepcopy() at construction and MappingProxyType for non-Pydantic collections
Use asyncio.TaskGroup for fan-out/fan-in parallel operations (structured concurrency) instead of bare create_task
Keep functions under 50 lines and files under 800 lines
Handle errors explicitly; never silently swallow exceptions
Validate at system boundaries (user input, external APIs, config files)
Set line length to 88 characters (enforced by ruff)
Files:
src/synthorg/settings/definitions/api.pysrc/synthorg/api/controllers/setup.py
src/synthorg/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/synthorg/**/*.py: Every module with business logic MUST import logger viafrom synthorg.observability import get_loggerthen uselogger = get_logger(__name__)
Never useimport logging,logging.getLogger(), orprint()in application code; only use the centralized get_logger function
Always use structured logging with keyword arguments:logger.info(EVENT, key=value)— never use formatted strings likelogger.info('msg %s', val)
Use event name constants from synthorg.observability.events. (e.g., API_REQUEST_STARTED from events.api, TOOL_INVOKE_START from events.tool) for all log events
Log all error paths at WARNING or ERROR level with context before raising exceptions
Log all state transitions at INFO level
Use DEBUG level logging for object creation, internal flow, and entry/exit of key functions
Use NotBlankStr (from core.types) for all identifier/name fields in models instead of manual whitespace validators
Never implement retry logic in provider driver subclasses or calling code — it is handled automatically by BaseCompletionProvider base class
Never use real vendor names (Anthropic, OpenAI, Claude, GPT, etc.) in project-owned code, docstrings, or comments; use generic names like example-provider or example-large-001
Use frozen Pydantic models for config/identity; use separate mutable-via-copy models (model_copy(update=...)) for runtime state that evolves
Use@computed_fieldfor derived values instead of storing and validating redundant fields
Files:
src/synthorg/settings/definitions/api.pysrc/synthorg/api/controllers/setup.py
**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,vue}: Use Vue 3 with PrimeVue components and Tailwind CSS for the dashboard frontend
Use Pinia stores for state management in the Vue dashboard
Use Axios client (from api/) for all backend API calls in the dashboard
Files:
web/src/components/setup/SetupCompany.vueweb/src/components/setup/SetupAgent.vueweb/src/__tests__/router/guards.test.ts
web/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
web/**/*.{ts,tsx,vue}: Run ESLint linting vianpm --prefix web run linton all dashboard code
Run vue-tsc type checking on all dashboard TypeScript and Vue files
Files:
web/src/components/setup/SetupCompany.vueweb/src/components/setup/SetupAgent.vueweb/src/__tests__/router/guards.test.ts
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.test.{ts,tsx}: Use Vitest for unit testing Vue components and TypeScript code in the dashboard
Use fast-check for property-based testing in Vue dashboard tests
Files:
web/src/__tests__/router/guards.test.ts
🧠 Learnings (5)
📚 Learning: 2026-03-19T11:29:37.522Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T11:29:37.522Z
Learning: Applies to **/*.{ts,tsx,vue} : Use Vue 3 with PrimeVue components and Tailwind CSS for the dashboard frontend
Applied to files:
web/src/components/setup/SetupCompany.vue
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to web/src/components/** : Vue components organized by feature (agents/, approvals/, budget/, common/, dashboard/, layout/, messages/, org-chart/, tasks/).
Applied to files:
web/src/components/setup/SetupCompany.vueweb/src/components/setup/SetupAgent.vue
📚 Learning: 2026-03-19T11:29:37.522Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T11:29:37.522Z
Learning: When review agents find valid issues (including pre-existing issues, suggestions, and adjacent findings), fix them all — no deferring, no 'out of scope' skipping
Applied to files:
web/src/components/setup/SetupAgent.vue
📚 Learning: 2026-03-19T11:29:37.522Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T11:29:37.522Z
Learning: Applies to **/*.{ts,tsx,vue} : Use Pinia stores for state management in the Vue dashboard
Applied to files:
web/src/__tests__/router/guards.test.ts
📚 Learning: 2026-03-19T11:29:37.522Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T11:29:37.522Z
Learning: Applies to src/synthorg/**/*.py : Use NotBlankStr (from core.types) for all identifier/name fields in models instead of manual whitespace validators
Applied to files:
src/synthorg/api/controllers/setup.py
… guard - _is_setup_complete: catch SettingNotFoundError instead of broad Exception (matches the fix already applied to _get_existing_agents) - SetupAgent handleCreate: add re-entrancy guard (return if creating.value) - Add guard test for statusLoaded=false branch (fetches before routing) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
♻️ Duplicate comments (2)
web/src/__tests__/router/guards.test.ts (1)
204-251:⚠️ Potential issue | 🟡 MinorAdd the still-missing setup guard branches (
/login+ fetch failure).At Line 204, the test name says “allows
/setupand/login” but only/setupis asserted.
At Line 235, only the successful fetch path is covered; there’s still no explicit rejectedgetSetupStatus()case.Suggested test additions
+import * as setupApi from '@/api/endpoints/setup' @@ it('allows /setup and /login when setup is needed', async () => { @@ await authGuard(toSetup, from, next) expect(next).toHaveBeenCalledWith() + + next = vi.fn() + const toLogin = createRoute({ path: '/login', name: 'login' as never, meta: { requiresAuth: false } }) + await authGuard(toLogin, from, next) + expect(next).toHaveBeenCalledWith() }) @@ + it('handles setup status fetch errors deterministically', async () => { + vi.mocked(setupApi.getSetupStatus).mockRejectedValueOnce(new Error('setup status failed')) + + const setup = useSetupStore() + setup.statusLoaded = false + setup.$patch({ status: null }) + + const to = createRoute({ path: '/dashboard', fullPath: '/dashboard', name: 'dashboard' as never, meta: {} }) + const from = createRoute() + + await authGuard(to, from, next) + + // Assert the intended fail behavior explicitly (setup/error/login), + // so regressions are caught. + expect(next).toHaveBeenCalled() + })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/__tests__/router/guards.test.ts` around lines 204 - 251, Add two tests: one that mirrors the existing "/setup" test but asserts that navigating to "/login" is allowed when setup.status.needs_setup is true (use useSetupStore(), set statusLoaded=true, createRoute({ path: '/login', name: 'login' as never, meta: { requiresAuth: false } }), call authGuard and expect next() called with no args); and one that simulates getSetupStatus() rejecting when setup.statusLoaded is false (spy/mock the module that exports getSetupStatus to return a rejected Promise), set setup.statusLoaded=false and status=null, call authGuard with a protected route (e.g., '/dashboard') and assert that setup.statusLoaded becomes true and auth routing handles the error (expect next called with redirect to '/login' with query.redirect set to the original fullPath) so the guard's fetch-failure branch is covered.web/src/components/setup/SetupAgent.vue (1)
243-250: 🧹 Nitpick | 🔵 TrivialConsider disabling the button while
creatingfor better UX.The function-level guard at line 118 prevents duplicate submissions, but the button remains visually enabled during the async operation. While
:loading="creating"provides feedback, addingcreatingto the disabled condition would provide clearer visual cues.♻️ Optional UX improvement
<Button type="submit" label="Create Agent & Finish" icon="pi pi-user-plus" class="w-full" :loading="creating" - :disabled="!isValid" + :disabled="!isValid || creating" />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/setup/SetupAgent.vue` around lines 243 - 250, The submit Button in SetupAgent.vue remains enabled during the async create process because its :disabled binding only checks isValid; update the Button's :disabled binding to also consider the creating flag (e.g., :disabled="!isValid || creating") so the control is truly disabled while creating is true; ensure you only change the Button component props (the Button instance with label "Create Agent & Finish") and not the existing function-level guard that prevents duplicate submissions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@web/src/__tests__/router/guards.test.ts`:
- Around line 204-251: Add two tests: one that mirrors the existing "/setup"
test but asserts that navigating to "/login" is allowed when
setup.status.needs_setup is true (use useSetupStore(), set statusLoaded=true,
createRoute({ path: '/login', name: 'login' as never, meta: { requiresAuth:
false } }), call authGuard and expect next() called with no args); and one that
simulates getSetupStatus() rejecting when setup.statusLoaded is false (spy/mock
the module that exports getSetupStatus to return a rejected Promise), set
setup.statusLoaded=false and status=null, call authGuard with a protected route
(e.g., '/dashboard') and assert that setup.statusLoaded becomes true and auth
routing handles the error (expect next called with redirect to '/login' with
query.redirect set to the original fullPath) so the guard's fetch-failure branch
is covered.
In `@web/src/components/setup/SetupAgent.vue`:
- Around line 243-250: The submit Button in SetupAgent.vue remains enabled
during the async create process because its :disabled binding only checks
isValid; update the Button's :disabled binding to also consider the creating
flag (e.g., :disabled="!isValid || creating") so the control is truly disabled
while creating is true; ensure you only change the Button component props (the
Button instance with label "Create Agent & Finish") and not the existing
function-level guard that prevents duplicate submissions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: e8074fc2-6833-4c22-b29c-4994801781e9
📒 Files selected for processing (3)
src/synthorg/api/controllers/setup.pyweb/src/__tests__/router/guards.test.tsweb/src/components/setup/SetupAgent.vue
📜 Review details
⏰ 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). (8)
- GitHub Check: Deploy Preview
- GitHub Check: CLI Test (macos-latest)
- GitHub Check: CLI Test (windows-latest)
- GitHub Check: Test (Python 3.14)
- GitHub Check: Build Sandbox
- GitHub Check: Build Backend
- GitHub Check: Build Web
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,vue}: Use Vue 3 with PrimeVue components and Tailwind CSS for the dashboard frontend
Use Pinia stores for state management in the Vue dashboard
Use Axios client (from api/) for all backend API calls in the dashboard
Files:
web/src/__tests__/router/guards.test.tsweb/src/components/setup/SetupAgent.vue
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.test.{ts,tsx}: Use Vitest for unit testing Vue components and TypeScript code in the dashboard
Use fast-check for property-based testing in Vue dashboard tests
Files:
web/src/__tests__/router/guards.test.ts
web/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
web/**/*.{ts,tsx,vue}: Run ESLint linting vianpm --prefix web run linton all dashboard code
Run vue-tsc type checking on all dashboard TypeScript and Vue files
Files:
web/src/__tests__/router/guards.test.tsweb/src/components/setup/SetupAgent.vue
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Use Pydantic v2 (BaseModel, model_validator, computed_field, ConfigDict) for all data models
Use PEP 758 except syntax:except A, B:(no parentheses) — not PEP 654 syntax
Do NOT usefrom __future__ import annotations— Python 3.14 has PEP 649 native lazy annotations
Include type hints on all public functions and classes; enforce with mypy strict mode
Use Google-style docstrings for all public classes and functions (enforced by ruff D rules)
Prefer immutability: create new objects rather than mutating existing ones; use copy.deepcopy() at construction and MappingProxyType for non-Pydantic collections
Use asyncio.TaskGroup for fan-out/fan-in parallel operations (structured concurrency) instead of bare create_task
Keep functions under 50 lines and files under 800 lines
Handle errors explicitly; never silently swallow exceptions
Validate at system boundaries (user input, external APIs, config files)
Set line length to 88 characters (enforced by ruff)
Files:
src/synthorg/api/controllers/setup.py
src/synthorg/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/synthorg/**/*.py: Every module with business logic MUST import logger viafrom synthorg.observability import get_loggerthen uselogger = get_logger(__name__)
Never useimport logging,logging.getLogger(), orprint()in application code; only use the centralized get_logger function
Always use structured logging with keyword arguments:logger.info(EVENT, key=value)— never use formatted strings likelogger.info('msg %s', val)
Use event name constants from synthorg.observability.events. (e.g., API_REQUEST_STARTED from events.api, TOOL_INVOKE_START from events.tool) for all log events
Log all error paths at WARNING or ERROR level with context before raising exceptions
Log all state transitions at INFO level
Use DEBUG level logging for object creation, internal flow, and entry/exit of key functions
Use NotBlankStr (from core.types) for all identifier/name fields in models instead of manual whitespace validators
Never implement retry logic in provider driver subclasses or calling code — it is handled automatically by BaseCompletionProvider base class
Never use real vendor names (Anthropic, OpenAI, Claude, GPT, etc.) in project-owned code, docstrings, or comments; use generic names like example-provider or example-large-001
Use frozen Pydantic models for config/identity; use separate mutable-via-copy models (model_copy(update=...)) for runtime state that evolves
Use@computed_fieldfor derived values instead of storing and validating redundant fields
Files:
src/synthorg/api/controllers/setup.py
🧠 Learnings (5)
📓 Common learnings
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T11:29:37.522Z
Learning: Applies to **/*.{ts,tsx,vue} : Use Pinia stores for state management in the Vue dashboard
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to docker/{Dockerfile*,compose.yml} : Docker: Backend uses 3-stage build (builder → setup → distroless runtime), Chainguard Python, non-root (UID 65532), CIS-hardened. Web uses nginxinc/nginx-unprivileged, Vue 3 SPA with PrimeVue + Tailwind CSS, SPA routing, API/WebSocket proxy to backend.
📚 Learning: 2026-03-19T11:29:37.522Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T11:29:37.522Z
Learning: Applies to **/*.{ts,tsx,vue} : Use Pinia stores for state management in the Vue dashboard
Applied to files:
web/src/__tests__/router/guards.test.ts
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to web/src/components/** : Vue components organized by feature (agents/, approvals/, budget/, common/, dashboard/, layout/, messages/, org-chart/, tasks/).
Applied to files:
web/src/components/setup/SetupAgent.vue
📚 Learning: 2026-03-19T11:29:37.522Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T11:29:37.522Z
Learning: When review agents find valid issues (including pre-existing issues, suggestions, and adjacent findings), fix them all — no deferring, no 'out of scope' skipping
Applied to files:
web/src/components/setup/SetupAgent.vue
📚 Learning: 2026-03-19T11:29:37.522Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T11:29:37.522Z
Learning: Applies to src/synthorg/**/*.py : Use NotBlankStr (from core.types) for all identifier/name fields in models instead of manual whitespace validators
Applied to files:
src/synthorg/api/controllers/setup.py
🧬 Code graph analysis (2)
web/src/__tests__/router/guards.test.ts (3)
web/src/stores/setup.ts (1)
useSetupStore(7-97)web/src/router/guards.ts (1)
authGuard(18-79)web/src/stores/auth.ts (1)
useAuthStore(8-191)
src/synthorg/api/controllers/setup.py (5)
src/synthorg/api/errors.py (3)
ApiValidationError(178-186)ConflictError(189-197)NotFoundError(167-175)src/synthorg/api/state.py (4)
AppState(40-460)has_provider_registry(323-325)provider_registry(328-333)provider_management(281-286)src/synthorg/settings/errors.py (1)
SettingNotFoundError(8-9)src/synthorg/settings/service.py (2)
SettingsService(152-637)get_entry(275-298)src/synthorg/templates/presets.py (1)
get_personality_preset(406-428)
🔇 Additional comments (10)
web/src/__tests__/router/guards.test.ts (1)
54-186: Nice update to align tests with async guard + setup store state.Switching these cases to
await authGuard(...)and preloadinguseSetupStorestate makes the suite consistent with the new guard control flow.web/src/components/setup/SetupAgent.vue (2)
140-140: The agent creation step still sendsbudget_limit_monthly: null.The PR objectives (from issue
#452) require "Add first agent (role, model from configured provider, basic budget)". The API supportsbudget_limit_monthly, but there's no UI input for it and the value is hardcoded tonull. This means the wizard cannot satisfy the basic budget requirement.Consider adding a budget input field or making this field optional in the requirements if a default budget is acceptable.
1-66: LGTM: Component structure and state management.Good use of Pinia stores, component-scoped
savedAgentref to prevent cross-mount leaks, and proper TypeScript typing. The re-entrancy guard and provider-change model clearing are correctly implemented. Based on learnings, "Use Pinia stores for state management in the Vue dashboard".src/synthorg/api/controllers/setup.py (7)
218-219: Deriveneeds_adminfrom admin user presence, not total user count.
persistence.users.count()only tells whether any user exists. An instance with observer/service users but no admin will skip the admin step and leave the installation without an administrator. The response contract (needs_admin: True if no admin user exists yet) requires checking for admin role specifically.
83-95: The company step has no way to save a description.The PR objectives (from issue
#452) specify "Create company (template or blank; name, description)".SetupCompanyRequestandSetupCompanyResponseonly handlecompany_name; there's nodescriptionfield. The wizard cannot persist the required company description.Also applies to lines 97-111 (response DTO) and 325-333 (persistence).
438-441: Don't allow/setup/completebefore required entities exist.This only validates that providers are configured. A direct call can mark setup complete before any company or agent has been created, which hides the setup flow even though the installation is incomplete. Consider also checking that at least one company and one agent exist via the settings service before allowing completion.
1-44: LGTM: Module setup, imports, and observability.Proper use of centralized logging via
get_logger(__name__), event constants fromsynthorg.observability.events.setup, and module-level_AGENT_LOCKfor serializing agent writes. As per coding guidelines, "Every module with business logic MUST import logger viafrom synthorg.observability import get_logger".
46-187: LGTM: DTOs are well-defined with proper validation.All DTOs use
ConfigDict(frozen=True),NotBlankStrfor identifier fields, and appropriateFieldconstraints. TheSetupAgentRequest._validate_preset_existsmodel validator correctly normalizes the personality preset key and usesobject.__setattr__to mutate the frozen model during validation. As per coding guidelines, "UseNotBlankStr(fromcore.types) for all identifier/name fields in models".
253-284: LGTM: Endpoint implementations and helper functions.The template listing, company creation, and agent creation endpoints are well-structured with proper guards, validation-before-persistence ordering, atomic department clearing, and structured error handling. The
_extract_template_departmentshelper correctly maps template errors to appropriate API errors. The_AGENT_LOCKproperly serializes concurrent agent appends.Also applies to: 285-348, 350-412, 414-448, 451-602
631-640: The review comment is incorrect.SettingEntry.valueis defined as a requiredstrfield (notOptional[str]) insrc/synthorg/settings/models.pyline 251. Pydantic validation enforces this constraint, so ifget_entry()returns successfully,entry.valueis guaranteed to be a non-None string. The docstring's mention of "if entry is absent or None" refers to the setting not being found (handled by theSettingNotFoundErrorexception), not toNonevalues within a returned entry. Therefore, the proposedNonecheck is unnecessary.
- SetupAgent: add creating to Button :disabled binding - Guard tests: add /login allowed when setup needed, fetchStatus rejection fallback to auth routing Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@web/src/__tests__/router/guards.test.ts`:
- Around line 204-216: The test name "allows /setup and /login when setup is
needed" is inaccurate because the body only exercises the /setup route; either
rename the test to "allows /setup when setup is needed" or add a second
assertion that calls authGuard with a createRoute for path '/login' and asserts
next() was called; locate the test block using the spec invoking
useSetupStore(), createRoute(), authGuard and next and update the it(...)
description or add the extra route/assertion accordingly.
- Around line 267-283: The test comments that "statusLoaded stays false" but
doesn't assert it; update the test in guards.test.ts to explicitly verify the
behavior by adding an assertion after calling authGuard that
useSetupStore().statusLoaded is false (or, if the intended behavior is
different, change the expectation accordingly), referencing the mocked
getSetupStatus, useSetupStore, authGuard and next so the test fails if error
handling changes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 030e99a4-f30d-4eb9-9807-25e66e5c5388
📒 Files selected for processing (2)
web/src/__tests__/router/guards.test.tsweb/src/components/setup/SetupAgent.vue
📜 Review details
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,vue}: Use Vue 3 with PrimeVue components and Tailwind CSS for the dashboard frontend
Use Pinia stores for state management in the Vue dashboard
Use Axios client (from api/) for all backend API calls in the dashboard
Files:
web/src/__tests__/router/guards.test.tsweb/src/components/setup/SetupAgent.vue
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.test.{ts,tsx}: Use Vitest for unit testing Vue components and TypeScript code in the dashboard
Use fast-check for property-based testing in Vue dashboard tests
Files:
web/src/__tests__/router/guards.test.ts
web/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
web/**/*.{ts,tsx,vue}: Run ESLint linting vianpm --prefix web run linton all dashboard code
Run vue-tsc type checking on all dashboard TypeScript and Vue files
Files:
web/src/__tests__/router/guards.test.tsweb/src/components/setup/SetupAgent.vue
🧠 Learnings (4)
📓 Common learnings
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T11:29:37.522Z
Learning: Applies to site/**/*.{astro,tsx,ts,css} : Use Astro for the landing page at synthorg.io with Tailwind CSS; include SEO infrastructure (sitemap, OG/Twitter meta, JSON-LD structured data), contact form (Formcarry), and CLI installation page (/get/)
📚 Learning: 2026-03-19T11:29:37.522Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T11:29:37.522Z
Learning: Applies to **/*.{ts,tsx,vue} : Use Pinia stores for state management in the Vue dashboard
Applied to files:
web/src/__tests__/router/guards.test.ts
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to web/src/components/** : Vue components organized by feature (agents/, approvals/, budget/, common/, dashboard/, layout/, messages/, org-chart/, tasks/).
Applied to files:
web/src/components/setup/SetupAgent.vue
📚 Learning: 2026-03-19T11:29:37.522Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T11:29:37.522Z
Learning: When review agents find valid issues (including pre-existing issues, suggestions, and adjacent findings), fix them all — no deferring, no 'out of scope' skipping
Applied to files:
web/src/components/setup/SetupAgent.vue
🔇 Additional comments (8)
web/src/components/setup/SetupAgent.vue (5)
140-140: Budget field is still hardcoded tonull.The
budget_limit_monthlyis always sent asnull, so the wizard cannot satisfy the "basic budget" requirement from issue#452even though the API supports this field. Consider adding a budget input (e.g., a numeric field with a sensible default) to allow users to set an initial monthly budget for the agent.
1-66: LGTM on imports, constants, and state setup.The component-scoped
savedAgentref correctly prevents state leakage across component mounts. Constants are well-organized with clear mappings between roles, levels, and departments.
68-115: Well-structured computed properties and watchers.The model encoding scheme (
provider::model_id) is correctly parsed withsplit('::')andjoin('::')to handle model IDs that may contain the delimiter. The provider change watcher properly clears the model selection to prevent form/payload mismatch.
117-151: Agent creation and retry logic is sound.The re-entrancy guard at line 118, combined with the
savedAgentcaching pattern, correctly prevents duplicate agent creation when retrying after amarkCompletefailure. The separation of the non-idempotentcreateAgentcall from the idempotent completion step is well implemented.
162-253: Template is clean and accessible.Form inputs are properly labeled with matching
for/idattributes, the error alert has the correctrole="alert", and the submit button correctly disables during submission with both visual feedback (:loading) and interaction prevention (:disabled). Theshow-clearon the provider dropdown enables a good "All providers" UX.web/src/__tests__/router/guards.test.ts (3)
23-34: LGTM!The setup API mock is well-structured with sensible defaults. Using
mockResolvedValuefor the happy path and allowing individual tests to override via store patching ormockRejectedValueOnceis a clean approach.
54-66: LGTM!Good test hygiene: pre-populating the setup store with
statusLoaded = trueprevents unintended network fetches and gives each test a known baseline to override as needed.
84-104: LGTM!The pattern of re-patching the setup store after
setActivePinia(createPinia())is correctly applied throughout. This ensures setup-dependent routing remains stable when the auth store needs a fresh Pinia instance with token from localStorage.
- Rename "allows /setup and /login" to "allows /setup" (body only tests /setup) - Add explicit statusLoaded assertion in fetchStatus rejection test Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
🤖 I have created a release *beep* *boop* --- ## [0.3.7](v0.3.6...v0.3.7) (2026-03-19) ### Features * **engine:** implement Hybrid Plan + ReAct execution loop ([#582](#582)) ([008147c](008147c)) * implement first-run setup wizard ([#584](#584)) ([dfed931](dfed931)) ### Bug Fixes * **api:** address ZAP DAST scan findings ([#579](#579)) ([ce9a3e0](ce9a3e0)) * **cli:** regenerate compose and re-exec binary on update ([#576](#576)) ([3f226eb](3f226eb)) ### CI/CD * add SBOM generation to Docker and CLI releases ([#580](#580)) ([db459cf](db459cf)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
🤖 I have created a release *beep* *boop* --- ## [0.3.7](v0.3.6...v0.3.7) (2026-03-19) ### Features * **engine:** implement Hybrid Plan + ReAct execution loop ([#582](#582)) ([008147c](008147c)) * implement first-run setup wizard ([#584](#584)) ([dfed931](dfed931)) ### Bug Fixes * **api:** address ZAP DAST scan findings ([#579](#579)) ([ce9a3e0](ce9a3e0)) * **cli:** auto-delete binary on Windows, prune images, fix GoReleaser ([#590](#590)) ([eb7c691](eb7c691)) * **cli:** regenerate compose and re-exec binary on update ([#576](#576)) ([3f226eb](3f226eb)) ### CI/CD * add SBOM generation to Docker and CLI releases ([#580](#580)) ([db459cf](db459cf)) ### Maintenance * **main:** release 0.3.7 ([#583](#583)) ([bf58779](bf58779)) * reset failed v0.3.7 release ([#591](#591)) ([b69000d](b69000d)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added engine Hybrid Plan + ReAct execution loop * Added first-run setup wizard * **Bug Fixes** * Addressed ZAP DAST scan issues * Fixed CLI Windows/image/update issues * **Maintenance** * Added SBOM generation for Docker/CLI releases * General maintenance updates <!-- end of auto-generated comment: release notes by coderabbit.ai -->
🤖 I have created a release *beep* *boop* --- ## [0.3.7](v0.3.6...v0.3.7) (2026-03-19) ### Features * **engine:** implement Hybrid Plan + ReAct execution loop ([#582](#582)) ([008147c](008147c)) * implement first-run setup wizard ([#584](#584)) ([dfed931](dfed931)) ### Bug Fixes * **api:** address ZAP DAST scan findings ([#579](#579)) ([ce9a3e0](ce9a3e0)) * **ci:** reset failed v0.3.7 release and fix syft SBOM scan ([#593](#593)) ([d1508c2](d1508c2)) * **cli:** auto-delete binary on Windows, prune images, fix GoReleaser ([#590](#590)) ([eb7c691](eb7c691)) * **cli:** regenerate compose and re-exec binary on update ([#576](#576)) ([3f226eb](3f226eb)) ### CI/CD * add SBOM generation to Docker and CLI releases ([#580](#580)) ([db459cf](db459cf)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
🤖 I have created a release *beep* *boop* --- ## [0.3.7](v0.3.6...v0.3.7) (2026-03-19) ### Features * **engine:** implement Hybrid Plan + ReAct execution loop ([#582](#582)) ([008147c](008147c)) * implement first-run setup wizard ([#584](#584)) ([dfed931](dfed931)) ### Bug Fixes * **api:** address ZAP DAST scan findings ([#579](#579)) ([ce9a3e0](ce9a3e0)) * **ci:** remove CLI SBOM generation, reset failed v0.3.7 ([#595](#595)) ([d0f4992](d0f4992)) * **ci:** reset failed v0.3.7 release and fix syft SBOM scan ([#593](#593)) ([d1508c2](d1508c2)) * **cli:** auto-delete binary on Windows, prune images, fix GoReleaser ([#590](#590)) ([eb7c691](eb7c691)) * **cli:** regenerate compose and re-exec binary on update ([#576](#576)) ([3f226eb](3f226eb)) ### CI/CD * add SBOM generation to Docker and CLI releases ([#580](#580)) ([db459cf](db459cf)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
Three setup wizard hardening fixes from PR #584 review: - #588: needs_admin now checks for CEO role explicitly via new count_by_role(HumanRole) method on UserRepository protocol, SQLite implementation, and fake test implementation - #587: complete_setup verifies company name and at least one agent exist (via settings) before marking setup complete, in addition to the existing provider check - #589: SetupStatusResponse now includes min_password_length from backend settings; SetupAdmin.vue reads it from the setup store instead of the hardcoded constant Closes #587 Closes #588 Closes #589 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Three setup wizard hardening fixes from PR #584 review: - #588: needs_admin now checks for CEO role explicitly via new count_by_role(HumanRole) method on UserRepository protocol, SQLite implementation, and fake test implementation - #587: complete_setup verifies company name and at least one agent exist (via settings) before marking setup complete, in addition to the existing provider check - #589: SetupStatusResponse now includes min_password_length from backend settings; SetupAdmin.vue reads it from the setup store instead of the hardcoded constant Closes #587 Closes #588 Closes #589 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
## Summary Three setup wizard hardening fixes from PR #584 review: - **#588**: `needs_admin` now checks for CEO role explicitly via new `count_by_role(HumanRole)` method on `UserRepository` protocol, SQLite implementation, and fake test implementation -- instead of just checking `user_count == 0` - **#587**: `complete_setup` verifies company name and at least one agent exist (via settings) before marking setup complete, in addition to the existing provider check. Check order: company -> agents -> providers - **#589**: `SetupStatusResponse` now includes `min_password_length` from backend settings; `SetupAdmin.vue` reads it from the setup store instead of the hardcoded `MIN_PASSWORD_LENGTH` constant, with fallback to the constant if the server value is unavailable ## Changes ### Backend - `UserRepository` protocol: added `count_by_role(role: HumanRole) -> int` - `SQLiteUserRepository`: parameterized `SELECT COUNT(*) WHERE role = ?` implementation - `setup.py` controller: `get_setup_status` uses `count_by_role(CEO)` for `needs_admin`; `complete_setup` validates company + agents + providers; status response includes `min_password_length` - New observability events: `SETUP_NO_COMPANY`, `SETUP_NO_AGENTS`, `PERSISTENCE_USER_COUNTED_BY_ROLE`, `PERSISTENCE_USER_COUNT_BY_ROLE_FAILED` ### Frontend - `SetupStatusResponse` type: added `min_password_length: number` - `setup.ts` store: added `minPasswordLength` computed with `MIN_PASSWORD_LENGTH` fallback - `SetupAdmin.vue`: reads password length from store instead of hardcoded constant ### Docs - `CLAUDE.md` and `docs/design/operations.md`: clarified completion gate prerequisites ## Test plan - [x] `test_needs_admin_true_when_only_non_admin_exists` -- verifies CEO-specific check - [x] `test_needs_admin_false_when_ceo_exists` -- verifies default fixture passes - [x] `test_status_includes_min_password_length` -- verifies new field in response - [x] `test_complete_validates_all_prerequisites` -- walks through agents -> providers -> success checks - [x] All 9501 existing unit tests pass - [x] mypy strict, ruff lint+format, vue-tsc, ESLint all pass Pre-reviewed by 4 agents (code-reviewer, frontend+API-contract, docs-consistency, issue-resolution-verifier). 2 doc findings addressed. Closes #587 Closes #588 Closes #589 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Setup wizard now requires company info, at least one agent, and a provider before completion. * Setup status now exposes the system minimum password length; the admin setup UI uses this value for validation and messaging. * **Documentation** * User and developer docs updated to reflect the new setup prerequisites and displayed minimum password length. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary
SetupControllerwith 5 endpoints (status, templates, company, agent, complete),setup_completeboolean setting, structured logging eventssynthorg setupcommand to re-trigger the wizard (resets flag + opens browser)Test plan
go vet,go test,go buildall passCloses #452
🤖 Generated with Claude Code