0127d283a6
Enforces max 5-10 files per directory using proper subpackages: - api/tool/, api/config/, api/workspace/, api/user/, api/project/, api/system/ - services/docker/, services/instance/, services/config/, services/git/, services/build/, services/terminal/, services/shared/ - models/tool/, models/config/, models/user/, models/project/, models/system/ - schemas/tool/, schemas/config/, schemas/user/, schemas/project/, schemas/system/ Updated design.md module map and tasks.md with 9 phases.
320 lines
11 KiB
Markdown
320 lines
11 KiB
Markdown
## Context
|
||
|
||
Current `dev` has all behavioral features from the overwritten `main` merge, but the code structure is pre-refactor:
|
||
- Monolithic `api/tool_instances.py` (~3000 lines)
|
||
- Monolithic `api/config_profiles.py` (~1000 lines)
|
||
- Monolithic `services/docker.py`
|
||
- No `schemas/` directory
|
||
- Flat frontend component structure with inconsistent naming
|
||
|
||
The `b6f89f9` merge from `main` had a clean refactoring that we need to redo, but adapted to our current reality.
|
||
|
||
## Goals / Non-Goals
|
||
|
||
**Goals:**
|
||
- Extract Pydantic schemas from API routers into `src/schemas/`
|
||
- Split `services/docker.py` into `services/docker/` package
|
||
- Extract instance lifecycle logic from `api/tool_instances.py` into `services/instance_lifecycle.py`
|
||
- Extract config profile business logic from `api/config_profiles.py` into `services/config_profiles.py`
|
||
- Add `get_current_user` auth dependency and migrate routers that need the full user object
|
||
- Reorganize frontend components into `features/` directories
|
||
- Standardize frontend API file naming to kebab-case
|
||
- Standardize frontend page naming to `*Page.tsx`
|
||
|
||
**Non-Goals:**
|
||
- Changing any API request/response shapes
|
||
- Changing any database schemas
|
||
- Adding new features
|
||
- Modifying frontend component behavior or styling
|
||
- Converting `ConfigProfile` mounts from JSON to relation tables (out of scope — would require migration)
|
||
|
||
## Decisions
|
||
|
||
### 0. Submodule Rule: Max 5–10 Files Per Directory
|
||
**Decision:** Every directory that functions as a Python module must contain at most 5–10 `.py` files. When a module grows beyond this, split it into a package with submodules.
|
||
**Rationale:** Prevents monolithic directories, makes navigation predictable, and keeps cognitive load bounded.
|
||
|
||
### 1. Schema Extraction: Domain Subpackages
|
||
**Decision:** Extract Pydantic models into `schemas/` subpackages by domain:
|
||
- `schemas/tool/` — tool_type.py, tool_instance.py
|
||
- `schemas/config/` — config_profile.py
|
||
- `schemas/user/` — user.py, user_config.py
|
||
- `schemas/project/` — project.py, git_repository.py, ssh_key.py
|
||
- `schemas/system/` — health.py
|
||
**Rationale:** Keeps schemas close to their domain. Each subpackage has ≤5 files.
|
||
|
||
### 2. API Router Subpackages
|
||
**Decision:** Split `api/` into domain subpackages:
|
||
- `api/tool/` — tool_instances.py, tool_types.py, tool_definitions.py, tool_types_validation.py, sessions.py
|
||
- `api/config/` — config_profiles.py, user_config.py
|
||
- `api/workspace/` — workspaces.py, workspace_files.py, workspace_git.py, workspace_instances.py
|
||
- `api/user/` — users.py, auth.py, ssh_keys.py
|
||
- `api/project/` — projects.py, git_repositories.py
|
||
- `api/system/` — health.py, events.py, notifications.py, dashboard.py, terminal.py, instance_proxy.py
|
||
**Rationale:** `api/` currently has ~22 files. Splitting into 6 subpackages keeps each at 2–6 files.
|
||
|
||
### 3. Service Subpackages
|
||
**Decision:** Split `services/` into subpackages:
|
||
- `services/docker/` — compose.py, container.py, config_staging.py, tunnel.py, __init__.py
|
||
- `services/instance/` — instance_lifecycle.py, lifecycle_hooks.py, health_monitor.py, event_bus.py
|
||
- `services/config/` — config_profile_resolver.py, config_profiles.py
|
||
- `services/git/` — clone.py, git_operations.py, git_service.py
|
||
- `services/build/` — docker_build.py, manifest_compiler.py
|
||
- `services/terminal/` — terminal_manager.py, terminal_session.py
|
||
- `services/shared/` — tunnel.py, notification_service.py, file_service.py, permission_fixer.py, readiness_probe.py, ssh_keys.py, workspace_manager.py, correlation.py
|
||
**Rationale:** `services/` currently has ~20 files. Subpackages keep each at ≤8 files.
|
||
|
||
### 4. Model Subpackages
|
||
**Decision:** Split `models/` into subpackages:
|
||
- `models/tool/` — tool_type.py, tool_instance.py, tool_definition_manifest.py
|
||
- `models/config/` — config_profile.py, config_include.py, config_mount.py
|
||
- `models/user/` — user.py, user_config.py, ssh_key.py
|
||
- `models/project/` — project.py, git_repository.py, workspace.py
|
||
- `models/system/` — health_check.py, notification.py, instance_event.py, terminal_session.py
|
||
- `models/base.py` stays at root
|
||
**Rationale:** `models/` currently has ~15 files. Subpackages keep each at ≤4 files.
|
||
|
||
### 5. Docker Service Split: Functional Boundaries
|
||
**Decision:** Split by responsibility:
|
||
- `compose.py` — compose file generation, modification, port injection, network injection
|
||
- `container.py` — container status, IP lookup, network connect, logs
|
||
- `config_staging.py` — staging config files into instance directories
|
||
- `tunnel.py` — extracting tunnel URLs from cloudflared output
|
||
**Rationale:** Each module has a single reason to change. `docker.py` mixed compose logic with container runtime queries.
|
||
|
||
### 6. Instance Lifecycle: Service Receives Raw Params, Not Request Objects
|
||
**Decision:** Service functions receive model instances and primitive parameters, not FastAPI request objects.
|
||
**Example:** `create_instance(session, user, project, repo, tool_type, data: CreateInstanceRequest)` → service extracts fields.
|
||
**Rationale:** Keeps service layer independent of HTTP framework. Easier to test.
|
||
|
||
### 7. Auth Pattern: Gradual Migration, Not Big Bang
|
||
**Decision:** Add `get_current_user` alongside existing `get_current_user_id`. Migrate routers incrementally.
|
||
**Rationale:** Reduces risk. Endpoints that only need the ID can keep the old pattern.
|
||
|
||
### 8. Frontend Naming: Align with `b6f89f9` Conventions
|
||
**Decision:** Use kebab-case for API files, PascalCase for page files with `Page` suffix, `features/` for component directories.
|
||
**Rationale:** Matches the `b6f89f9` structure that was already reviewed and accepted.
|
||
|
||
## Module Map
|
||
|
||
### Backend — Before
|
||
```
|
||
api/ (~22 .py files)
|
||
tool_instances.py (~3000 lines) — HTTP + Docker + Git + Lifecycle
|
||
config_profiles.py (~1000 lines) — HTTP + Validation + Defaults
|
||
tool_types.py (~500 lines) — HTTP + Schemas
|
||
health.py (~150 lines) — HTTP + Schemas
|
||
users.py (~100 lines) — HTTP + Schemas
|
||
...
|
||
services/ (~20 .py files)
|
||
docker.py (~600 lines) — Compose + Container + Tunnel
|
||
models/ (~15 .py files)
|
||
config_profile.py
|
||
tool_instance.py
|
||
...
|
||
```
|
||
|
||
### Backend — After
|
||
```
|
||
schemas/ (5 subpackages, ≤5 files each)
|
||
tool/
|
||
__init__.py
|
||
tool_type.py
|
||
tool_instance.py
|
||
config/
|
||
__init__.py
|
||
config_profile.py
|
||
user/
|
||
__init__.py
|
||
user.py
|
||
user_config.py
|
||
project/
|
||
__init__.py
|
||
project.py
|
||
git_repository.py
|
||
ssh_key.py
|
||
system/
|
||
__init__.py
|
||
health.py
|
||
|
||
api/ (6 subpackages, 2–6 files each)
|
||
tool/
|
||
__init__.py
|
||
tool_instances.py (~300 lines) — HTTP routing only
|
||
tool_types.py (~250 lines) — HTTP + validation
|
||
tool_definitions.py
|
||
tool_types_validation.py
|
||
sessions.py
|
||
config/
|
||
__init__.py
|
||
config_profiles.py (~200 lines) — HTTP routing only
|
||
user_config.py
|
||
workspace/
|
||
__init__.py
|
||
workspaces.py
|
||
workspace_files.py
|
||
workspace_git.py
|
||
workspace_instances.py
|
||
user/
|
||
__init__.py
|
||
users.py (~60 lines) — HTTP only
|
||
auth.py
|
||
ssh_keys.py
|
||
project/
|
||
__init__.py
|
||
projects.py
|
||
git_repositories.py
|
||
system/
|
||
__init__.py
|
||
health.py (~80 lines) — HTTP only
|
||
events.py
|
||
notifications.py
|
||
dashboard.py
|
||
terminal.py
|
||
instance_proxy.py
|
||
|
||
services/ (7 subpackages, ≤8 files each)
|
||
docker/
|
||
__init__.py (~40 lines) — Re-exports
|
||
compose.py (~240 lines) — Compose generation
|
||
container.py (~120 lines) — Container queries
|
||
config_staging.py (~80 lines) — File staging
|
||
tunnel.py (~150 lines) — Tunnel URL extraction
|
||
instance/
|
||
__init__.py
|
||
instance_lifecycle.py (~420 lines) — Create/Start/Stop/Restart/Delete
|
||
lifecycle_hooks.py
|
||
health_monitor.py
|
||
event_bus.py
|
||
config/
|
||
__init__.py
|
||
config_profile_resolver.py
|
||
config_profiles.py (~300 lines) — CRUD + Defaults
|
||
git/
|
||
__init__.py
|
||
clone.py
|
||
git_operations.py
|
||
git_service.py
|
||
build/
|
||
__init__.py
|
||
docker_build.py
|
||
manifest_compiler.py
|
||
terminal/
|
||
__init__.py
|
||
terminal_manager.py
|
||
terminal_session.py
|
||
shared/
|
||
__init__.py
|
||
tunnel.py
|
||
notification_service.py
|
||
file_service.py
|
||
permission_fixer.py
|
||
readiness_probe.py
|
||
ssh_keys.py
|
||
workspace_manager.py
|
||
correlation.py
|
||
|
||
models/ (5 subpackages + base.py)
|
||
tool/
|
||
__init__.py
|
||
tool_type.py
|
||
tool_instance.py
|
||
tool_definition_manifest.py
|
||
config/
|
||
__init__.py
|
||
config_profile.py
|
||
config_include.py
|
||
config_mount.py
|
||
user/
|
||
__init__.py
|
||
user.py
|
||
user_config.py
|
||
ssh_key.py
|
||
project/
|
||
__init__.py
|
||
project.py
|
||
git_repository.py
|
||
workspace.py
|
||
system/
|
||
__init__.py
|
||
health_check.py
|
||
notification.py
|
||
instance_event.py
|
||
terminal_session.py
|
||
base.py (stays at root)
|
||
```
|
||
|
||
### Frontend — Before
|
||
```
|
||
src/
|
||
api/
|
||
tool_types.ts
|
||
ssh_keys.ts
|
||
git_repositories.ts
|
||
sessions.ts
|
||
components/
|
||
git-toolbar.tsx
|
||
file-editor.tsx
|
||
commit-dialog.tsx
|
||
...
|
||
pages/
|
||
dashboard.tsx
|
||
projects.tsx
|
||
sessions.tsx
|
||
...
|
||
```
|
||
|
||
### Frontend — After
|
||
```
|
||
src/
|
||
api/
|
||
tool-types.ts
|
||
ssh-keys.ts
|
||
git-repositories.ts
|
||
sessions.ts
|
||
components/
|
||
features/
|
||
git/
|
||
GitToolbar.tsx
|
||
FileBrowser.tsx
|
||
FileEditor.tsx
|
||
CommitDialog.tsx
|
||
MergeDialog.tsx
|
||
WorkspaceSidebar.tsx
|
||
dashboard/
|
||
DashboardSummary.tsx
|
||
ActiveSessionsList.tsx
|
||
ProjectsSection.tsx
|
||
QuickCreateForm.tsx
|
||
RecentSessionsSection.tsx
|
||
project/
|
||
RepositoriesSettingsTab.tsx
|
||
tool-workshop/
|
||
ToolTypesTab.tsx
|
||
ProtectedRoute.tsx
|
||
AppShell.tsx
|
||
pages/
|
||
DashboardPage.tsx
|
||
ProjectsPage.tsx
|
||
SessionsPage.tsx
|
||
...
|
||
```
|
||
|
||
## Risks / Trade-offs
|
||
|
||
**[Risk] Import cycles during extraction** → **Mitigation:** Extract schemas first (no service dependencies), then services, then thin routers last. Use TYPE_CHECKING guards.
|
||
|
||
**[Risk] Merge conflicts with in-flight features** → **Mitigation:** Coordinate timing. This refactor should be the only large change on `dev` while it's in progress. Freeze other backend work.
|
||
|
||
**[Risk] Frontend renaming breaks imports** → **Mitigation:** Use `git mv` for renames so git tracks history. Update all imports in a single commit.
|
||
|
||
**[Risk] Missing re-export in docker/__init__.py breaks consumers** → **Mitigation:** After splitting, run a full import test across all backend files. Add any missing re-exports.
|
||
|
||
## Migration Plan
|
||
|
||
1. **Phase 1: Schemas** — Extract all Pydantic models into `schemas/`. Update imports in API routers. No logic changes.
|
||
2. **Phase 2: Services** — Split `docker.py`, extract `instance_lifecycle.py`, extract `config_profiles.py`. Update imports.
|
||
3. **Phase 3: Auth** — Add `get_current_user`, migrate routers that need the full user object.
|
||
4. **Phase 4: Frontend** — Rename files, move components, update imports.
|
||
5. **Phase 5: Verification** — Run full test suite, typecheck, build.
|