tm0/flow
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix all issues from spec review

Browse Source 
- Add TargetConfig model to remote domain with normalization rules
- Add SetupModuleDef model to bootstrap domain
- Fix domain purity: Package carries pre-walked file lists, parse_module_ref
  takes parsed dict not file path, discover_packages moved to service layer
- Clarify conflict detection: cross-package collisions (pure) vs filesystem
  conflicts (injected callback in plan_link)
- Add dry_run to init, repos_pull, repos_push, stop, remove, respawn
- Document interactive commands (edit, attach, exec) as dry_run exceptions
- Document ProjectService as read-only (no dry_run needed)
- Fix ContainerState -> ContainerInfo naming consistency
- Add post-install and allow-sudo fields to config YAML example
- Document core/paths.py constants including MODULES_DIR
- Add target config normalization rules
- Clarify validate_env as eager precondition check, not a plan action
- Clarify setup show as effectively setup run --dry-run

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
Tomas Mirchev
2026-03-16 04:18:51 +02:00
parent 5222acc233
commit 93929c67d4
1 changed files with 210 additions and 63 deletions
Download Patch File Download Diff File Expand all files Collapse all files

273
docs/superpowers/specs/2026-03-16-flow-architecture-redesign.md
View File

@@ -30,8 +30,8 @@ core/ Layer 1: Runtime primitives + config loading
### Rules
- `domain/` never imports `core/runtime`, `services/`, or `commands/`. It only imports `core/template.py`, `core/errors.py`, `core/paths.py` (constants only), and stdlib.
- `services/` receives all dependencies via constructor (runtime, config, console). No module-level singletons.
- `domain/` never imports `core/runtime`, `services/`, or `commands/`. It only imports `core/template.py`, `core/errors.py`, `core/paths.py` (constants only), and stdlib. Domain functions that need filesystem information (directory listings, file existence) receive it as pre-built data or injected callbacks -- never by reading the filesystem directly.
- `services/` receives all dependencies via constructor (runtime, config, console). No module-level singletons. Cross-service calls use the shared `FlowContext` (e.g. `BootstrapService` constructs `DotfilesService(self.ctx)` internally). This is the intended pattern since all services share the same runtime context.
- `commands/` are trivial: parse args into a typed namespace, call one service method.
- Every plan is a frozen dataclass returned by domain functions. Services decide whether to execute or dry-run print them.
@@ -72,11 +72,11 @@ src/flow/
modules.py # Built-in setup module definitions (hostname, locale, shell, ssh-keygen, runcmd)
remote/
models.py # Target, SSHCommand
models.py # Target, TargetConfig, SSHCommand
resolution.py # Target parsing, host template expansion
containers/
models.py # ContainerSpec, ContainerState
models.py # ContainerSpec, ContainerInfo, ImageRef
resolution.py # Image ref parsing, name normalization, mount computation
services/
@@ -278,8 +278,11 @@ class Package:
"""A dotfiles package: a named set of files mapping to home-relative targets."""
name: str # e.g. "zsh", "nvim"
layer: str # "_shared" or profile name
package_id: str # Qualified: "layer/name" (e.g. "_shared/nvim")
source_dir: Path # Absolute path in dotfiles repo
module: ModuleRef | None # If backed by external repo
local_files: list[tuple[Path, Path]] # (absolute_source, relative_to_package_root)
# Pre-walked by the service layer (no I/O in domain)
@dataclass(frozen=True)
class ModuleRef:
@@ -289,7 +292,9 @@ class ModuleRef:
ref_value: str # e.g. "main", "v1.0", "abc123"
mount_path: Path # Relative path within package to _module.yaml parent
# e.g. Path(".config/nvim")
cache_dir: Path # ~/.local/share/flow/modules/<name>
cache_dir: Path # ~/.local/share/flow/modules/<package_id>
module_files: list[tuple[Path, Path]] # (absolute_source, relative_to_cache_root)
# Pre-walked by the service layer
@dataclass(frozen=True)
class LinkTarget:
@@ -370,15 +375,19 @@ Resolution steps:
```python
# domain/dotfiles/resolution.py
def discover_packages(dotfiles_dir: Path, profile: str | None) -> list[Package]: ...
# All functions are PURE: they operate on pre-built Package data (including
# pre-walked file lists), never touching the filesystem directly.
def resolve_package_targets(
package: Package,
home: Path,
skip: set[str],
) -> list[LinkTarget]:
"""Resolve all LinkTargets for a package, handling modules correctly."""
"""Resolve all LinkTargets for a package, handling modules correctly.
Uses package.local_files and package.module.module_files (pre-walked
by service layer) to compute targets. No filesystem I/O.
"""
...
def resolve_all_targets(
@@ -386,28 +395,62 @@ def resolve_all_targets(
home: Path,
skip: set[str],
) -> list[LinkTarget]:
"""Resolve targets for all packages. Raises on conflicts."""
"""Resolve targets for all packages. Raises PlanConflict on duplicate targets
across packages (e.g. _shared/zsh and linux-work/zsh both targeting ~/.zshrc).
This is a pure cross-package collision check, not a filesystem conflict check.
"""
...
```
```python
# domain/dotfiles/modules.py
# Pure functions for module metadata. I/O (reading YAML, walking dirs) is done
# by the service layer, which passes parsed data into these functions.
def parse_module_yaml(path: Path) -> ModuleRef: ...
def parse_module_ref(
raw: dict, # Pre-loaded YAML content (dict, not file path)
package_id: str,
mount_path: Path,
modules_base: Path,
) -> ModuleRef:
"""Build a ModuleRef from parsed _module.yaml content.
Args:
raw: The parsed YAML dict (service reads the file and passes content in).
package_id: Qualified name like "_shared/nvim".
mount_path: Relative path from package root to _module.yaml parent.
modules_base: Base directory for module caches (from core/paths.py).
"""
...
def compute_mount_path(module_yaml: Path, package_dir: Path) -> Path:
"""The key function: relative path from package root to _module.yaml parent."""
return module_yaml.parent.relative_to(package_dir)
def module_cache_dir(package_name: str, modules_base: Path) -> Path:
return modules_base / package_name.replace("/", "--")
def module_cache_dir(package_id: str, modules_base: Path) -> Path:
"""Compute cache dir for a module. Uses package_id (e.g. '_shared/nvim')
with '/' replaced by '--' to avoid collisions between same-named packages
in different layers."""
return modules_base / package_id.replace("/", "--")
def normalize_source(source: str) -> str:
"""github:org/repo -> https://github.com/org/repo.git"""
...
```
### 4.3 Planning
**I/O boundary note:** The service layer (`DotfilesService`) is responsible for:
1. Walking the dotfiles dir to find packages and `_module.yaml` files
2. Reading `_module.yaml` files and passing parsed dicts to `parse_module_ref`
3. Walking module cache dirs to build file lists
4. Constructing `Package` objects with pre-populated `local_files` and `ModuleRef.module_files`
5. Passing these fully-built objects to pure domain functions
### 4.3 Planning and Conflict Detection
Conflict detection and planning are integrated into a single flow. There are two kinds of conflicts:
1. **Cross-package collisions** (pure): two packages want the same target path. Detected by `resolve_all_targets` which raises `PlanConflict`.
2. **Filesystem conflicts** (requires I/O): a target path already exists on disk and is not managed by flow. Detected by `plan_link` via an injected callback.
```python
# domain/dotfiles/planning.py
@@ -415,13 +458,21 @@ def normalize_source(source: str) -> str:
def plan_link(
desired: list[LinkTarget],
current: LinkedState,
filesystem_check: Callable[[Path], str | None],
# ^^ Injected by service layer. Returns "file", "dir", "symlink", or None.
# This is the ONLY I/O dependency in the planning layer.
# Tests provide a fake (e.g., lambda p: None).
) -> LinkPlan:
"""Compare desired targets with current state, produce reconciliation plan.
- New targets: create_link ops
- Removed targets: remove_link ops
- Removed targets (in current but not desired): remove_link ops
- Existing correct symlinks: unchanged (no op)
- Conflicts (target exists, not managed): listed in plan.conflicts
- Filesystem conflicts: listed in plan.conflicts (target exists, not managed)
- Directory conflicts: listed in plan.conflicts (target is a dir, cannot overwrite)
The service checks plan.conflicts and raises PlanConflict if non-empty and
--force was not passed.
"""
...
@@ -433,24 +484,6 @@ def plan_unlink(
...
```
### 4.4 Conflict Detection
```python
# domain/dotfiles/conflicts.py
def detect_conflicts(
desired: list[LinkTarget],
current: LinkedState,
filesystem_check: Callable[[Path], str | None],
# Returns "file", "dir", "symlink", or None
# This is the ONLY I/O dependency, injected from service layer
) -> list[str]:
"""Return human-readable conflict descriptions."""
...
```
The `filesystem_check` callback is the boundary between pure domain and I/O. The service injects it. Tests provide a fake.
---
## 5. Packages Domain
@@ -564,22 +597,35 @@ class Profile:
os: str # "linux" | "macos"
package_manager: str | None # None = auto-detect
packages: list[Any] # Raw entries, normalized via packages domain
modules: list[dict] # Setup module definitions
setup_modules: list[SetupModuleDef] # Parsed setup module definitions
requires: list[str] # Required env vars
shell: str | None # e.g. "zsh"
dotfiles_profile: str | None # Profile name for dotfiles linking
@dataclass(frozen=True)
class SetupModuleDef:
"""Configuration for a single setup module step, parsed from YAML.
Examples:
SetupModuleDef(type="hostname", config={"value": "my-host"})
SetupModuleDef(type="ssh-keygen", config={"keys": [{"type": "ed25519", ...}]})
SetupModuleDef(type="runcmd", config={"commands": ["sudo groupadd docker || true"]})
"""
type: str # "hostname" | "locale" | "shell" | "ssh-keygen" | "runcmd"
config: dict # Type-specific configuration from YAML
@dataclass(frozen=True)
class BootstrapAction:
"""A single step in the bootstrap plan."""
type: str # "validate_env" | "install_packages" | "run_module"
type: str # "install_packages" | "run_setup_module"
# | "link_dotfiles" | "set_shell"
description: str
payload: Any # Type-specific data:
# install_packages -> PackagePlan
# run_module -> SetupModuleDef
# link_dotfiles -> profile name
# set_shell -> shell name
# run_setup_module -> tuple[SetupModuleDef, list[str]]
# (the module def + pre-computed shell commands)
# link_dotfiles -> profile name (str)
# set_shell -> shell name (str)
critical: bool # Stop on failure?
@dataclass(frozen=True)
@@ -589,6 +635,8 @@ class BootstrapPlan:
summary: str
```
**Note on env var validation:** `plan_bootstrap` validates required env vars eagerly and raises `ConfigError` if any are missing. This is a pure check (comparing `profile.requires` against the provided `env` dict) and happens before any actions are generated. It is not an action in the plan -- missing env vars are a precondition failure that prevents plan creation.
### 6.2 Setup Modules
Each module type is a small class with a plan method (pure) that returns shell commands to execute.
@@ -673,9 +721,24 @@ profiles:
### 7.1 Models
```python
@dataclass(frozen=True)
class TargetConfig:
"""A target as defined in config YAML. Parsed from the 'targets' section.
Supports two YAML formats:
personal@orb: personal.orb # shorthand: key is namespace@platform, value is host
work@ec2: # dict form
host: work.ec2.internal
identity: ~/.ssh/id_work
"""
namespace: str
platform: str
host: str
identity: str | None = None
@dataclass(frozen=True)
class Target:
"""A resolved SSH target."""
"""A fully resolved SSH target (after merging CLI args + config + templates)."""
user: str
namespace: str
platform: str
@@ -797,8 +860,9 @@ Each service is a class receiving dependencies via constructor. All mutating met
class DotfilesService:
def __init__(self, ctx: FlowContext): ...
def init(self, repo_url: str) -> None:
"""Clone dotfiles repo + discover and clone all module repos."""
def init(self, repo_url: str, *, dry_run: bool) -> None:
"""Clone dotfiles repo + discover and clone all module repos.
dry_run: prints repos that would be cloned without cloning."""
def link(self, *, profile: str | None, packages: list[str] | None,
force: bool, dry_run: bool) -> None:
@@ -808,25 +872,44 @@ class DotfilesService:
"""Remove managed links."""
def status(self, packages: list[str] | None) -> None:
"""Show package list, link health, module info."""
"""Show package list, link health, module info. Read-only."""
def edit(self, target: str) -> None:
"""Pull relevant repo -> open editor -> commit + push."""
def edit(self, target: str, *, no_commit: bool) -> None:
"""Pull relevant repo -> open editor -> commit + push.
Interactive command -- dry_run not applicable (editor is the point).
no_commit: skip the auto-commit/push after editing."""
def repos_list(self) -> None: ...
def repos_status(self, repo_filter: str | None) -> None: ...
def repos_pull(self, repo_filter: str | None) -> None: ...
def repos_push(self, repo_filter: str | None) -> None: ...
def repos_list(self) -> None:
"""List all managed repos. Read-only."""
def repos_status(self, repo_filter: str | None) -> None:
"""Git status for repos. Read-only."""
def repos_pull(self, repo_filter: str | None, *, dry_run: bool) -> None:
"""Pull one or all repos. dry_run: shows what would be pulled."""
def repos_push(self, repo_filter: str | None, *, dry_run: bool) -> None:
"""Push one or all repos. dry_run: shows what would be pushed."""
```
#### Link flow detail
The service layer performs all I/O (directory walking, YAML reading, filesystem checks) and passes pre-built data structures to pure domain functions:
```python
def link(self, ...):
packages = discover_packages(dotfiles_dir, profile) # domain
targets = resolve_all_targets(packages, home, skip) # domain
current = self._load_linked_state() # I/O
plan = plan_link(targets, current) # domain
# I/O: walk dirs, read _module.yaml files, build Package objects
packages = self._discover_packages(profile)
# Pure: resolve file paths to LinkTargets
targets = resolve_all_targets(packages, home, skip)
# I/O: load persisted state
current = self._load_linked_state()
# Pure (with injected callback): build plan with filesystem checks
plan = plan_link(targets, current,
filesystem_check=self._check_path_on_disk)
if plan.conflicts and not force:
raise PlanConflict(...)
@@ -860,8 +943,13 @@ class BootstrapService:
dry_run: bool) -> None:
"""Parse profile -> build plan -> execute actions sequentially."""
def list(self) -> None: ...
def show(self, profile: str) -> None: ...
def list(self) -> None:
"""List available profiles with summary info. Read-only."""
def show(self, profile: str, *, dry_run: bool) -> None:
"""Show profile details. When dry_run=True (or always, effectively),
builds and prints the full BootstrapPlan without executing.
'setup show' is conceptually 'setup run --dry-run' scoped to one profile."""
```
#### Run flow detail
@@ -909,12 +997,15 @@ class ContainerService:
def create(self, name: str, image: str, project: str | None, *,
dry_run: bool) -> None: ...
def attach(self, name: str) -> None: ...
def exec(self, name: str, cmd: list[str] | None) -> None: ...
def list(self) -> None: ...
def stop(self, name: str, *, kill: bool) -> None: ...
def remove(self, name: str, *, force: bool) -> None: ...
def respawn(self, name: str) -> None: ...
def attach(self, name: str) -> None:
"""Interactive -- attach to tmux session. No dry_run."""
def exec(self, name: str, cmd: list[str] | None) -> None:
"""Interactive -- exec into container. No dry_run."""
def list(self) -> None:
"""Read-only."""
def stop(self, name: str, *, kill: bool, dry_run: bool) -> None: ...
def remove(self, name: str, *, force: bool, dry_run: bool) -> None: ...
def respawn(self, name: str, *, dry_run: bool) -> None: ...
```
### 9.6 ProjectService
@@ -923,9 +1014,14 @@ class ContainerService:
class ProjectService:
def __init__(self, ctx: FlowContext): ...
def check(self, *, fetch: bool) -> None: ...
def fetch(self) -> None: ...
def summary(self) -> None: ...
def check(self, *, fetch: bool) -> None:
"""Read-only (fetch is network I/O but does not mutate local state
beyond FETCH_HEAD -- acceptable for a check command)."""
def fetch(self) -> None:
"""Fetch all remotes. Network I/O only, no local mutations beyond
remote tracking refs. dry_run not applicable."""
def summary(self) -> None:
"""Read-only."""
```
---
@@ -1011,6 +1107,9 @@ defaults:
container-tag: latest
tmux-session: default
# Targets: two formats supported
# Shorthand: "namespace@platform: host [identity]"
# Dict form: "namespace@platform: {host: ..., identity: ...}"
targets:
personal@orb: personal.orb
work@ec2:
@@ -1037,6 +1136,15 @@ packages:
bin: [bin/nvim]
share: [share/nvim]
- name: docker
type: pkg
sources:
apt: docker-ce
allow-sudo: true # Allow sudo in post-install hook
post-install: | # Shell script, runs after install
sudo groupadd docker || true
sudo usermod -aG docker $USER
profiles:
linux-work:
os: linux
@@ -1052,10 +1160,49 @@ profiles:
keys:
- type: ed25519
comment: "{{ env.USER_EMAIL }}"
- type: runcmd
commands:
- "sudo groupadd docker || true"
packages:
- git
- fd
- binary/neovim
- name: docker
allow-sudo: true
```
### Path constants (`core/paths.py`)
All XDG-compliant, hardcoded in `core/paths.py`:
```python
CONFIG_DIR = XDG_CONFIG_HOME / "flow" # ~/.config/flow/
DATA_DIR = XDG_DATA_HOME / "flow" # ~/.local/share/flow/
STATE_DIR = XDG_STATE_HOME / "flow" # ~/.local/state/flow/
DOTFILES_DIR = DATA_DIR / "dotfiles" # Cloned dotfiles repo
MODULES_DIR = DATA_DIR / "modules" # Module cache (each module cloned here)
PACKAGES_DIR = DATA_DIR / "packages" # Binary package scratch
LINKED_STATE = STATE_DIR / "linked.json" # Current link state
INSTALLED_STATE = STATE_DIR / "installed.json" # Installed packages state
# Self-hosted config (from dotfiles repo, takes priority over CONFIG_DIR)
DOTFILES_FLOW_CONFIG = DOTFILES_DIR / "_shared" / "flow" / ".config" / "flow"
```
`MODULES_DIR` is the `modules_base` passed to `module_cache_dir()` and other domain functions.
### Target config normalization
The config parser in `core/config.py` normalizes both target formats into `TargetConfig` objects:
```python
# Shorthand: "personal@orb: personal.orb"
# -> TargetConfig(namespace="personal", platform="orb", host="personal.orb", identity=None)
# Dict: "work@ec2: {host: ..., identity: ...}"
# -> TargetConfig(namespace="work", platform="ec2", host="work.ec2.internal", identity="~/.ssh/id_work")
```
---