Clean action runtime project state

This commit is contained in:
2026-05-14 13:58:45 +03:00
parent b05d3589b7
commit 4ce98d0ff1
27 changed files with 711 additions and 4772 deletions

124
docs/architecture.md Normal file
View File

@@ -0,0 +1,124 @@
# Flow Architecture
This document describes the current action-runtime architecture.
## Runtime Shape
Flow is organized around a canonical action plan:
```text
cli -> app -> domain -> actions -> adapters
```
The important boundary is `ActionPlan -> ActionExecutor -> ActionResult`.
- `cli`: Typer command declarations and argument parsing in `src/flow/cli.py`.
- `app`: command use-cases in `src/flow/app/`. These load config/state, compose
domain planners, and submit action plans.
- `domain`: pure models, parsers, resolvers, and planners in `src/flow/domain/`.
- `actions`: canonical action models, expansion, execution, dry-run rendering,
JSONL audit logging, and rollback in `src/flow/actions/`.
- `adapters`: filesystem, process, git, package-manager, download, archive,
container, and tmux implementations in `src/flow/adapters/`.
The filesystem is not the core abstraction. It is one adapter used by the
executor. The core abstraction is the action plan.
## Actions
`DomainAction` captures user intent:
- `kind`: `dotfiles`, `package`, `project`, `repo`, `container`, `remote`,
`setup`, or `completion`
- `action`: `link`, `unlink`, `install`, `remove`, `update`, `pull`, `push`,
`create`, `stop`, and related verbs
- `payload`: typed-by-domain data or already expanded primitive actions
- `rollback_policy`: `rollbackable`, `barrier`, or `none`
`PrimitiveAction` is the executor-owned operation set:
- filesystem: create/remove symlink, write, write JSON, copy, copy tree, remove,
chmod
- git: clone, pull, push, fetch, checkout, status
- process: argv command or explicit user shell hook
- packages/assets: download and archive extraction
- containers: run, start, stop, kill, remove, exec
- tmux: new session, set option, respawn pane
Domain actions are expanded before execution. Dry-run output and audit logging
come from the same primitive list that real execution uses.
## Rollback
Rollback is explicit and best effort.
Supported file/state primitives record reverse actions before mutation. On
failure, the executor rolls back in reverse order until it reaches a barrier.
External boundaries such as package-manager commands, user shell hooks, git
pushes, container lifecycle operations, and interactive command handoffs are not
guaranteed to roll back.
## Command Surface
Implemented commands:
```text
flow dotfiles init|link|unlink|status|edit
flow dotfiles repos list|status|pull|push
flow packages install|remove|list
flow setup run|show|list
flow remote enter|list
flow enter
flow dev create|attach|connect|exec|enter|stop|remove|rm|respawn|list
flow projects check|fetch|summary
flow sync
flow completion zsh|install-zsh
```
Global flags:
```text
flow --version
flow --quiet
flow --no-color
```
Mutating non-interactive commands expose `--dry-run` where a preview is useful:
dotfiles link/unlink, dotfiles repo pull/push, package install/remove, setup
run, remote enter, and dev create.
## Dotfiles And Modules
The dotfiles repo and `_module.yaml` repos share one abstraction: managed repos.
`flow dotfiles repos` operates on both. Modules are regular git clones cached
under the flow data directory; they are not git submodules.
Link planning treats conflicts and missing modules as pre-execution failures.
State is written through actions, and broken tracked symlinks are repaired by
the same link reconciliation path.
## Packages
Package manifests are parsed into typed dataclasses. Package-manager commands
are built as argv lists by the package adapter. Binary and AppImage installs
expand into download, archive, copy/chmod, cleanup, post-install barrier, and
state-write primitives.
`packages remove` is strict: it refuses unknown packages, removes tracked files,
and writes updated state through the executor.
## Tests And Guards
The test suite covers:
- action executor dry-run, audit logs, rollback, barriers, domain expansion, and
primitive dispatch
- dotfiles discovery, module handling, conflict behavior, rollback, and repo
operations
- package manifest parsing, package-manager argv, binary installs, archive
safety, strict removal, and post-install barriers
- Typer CLI invocation, help output, completion, and error reporting
- Docker/Podman e2e for the example dotfiles repo when `FLOW_RUN_E2E=1`
Static guard tests reject direct mutating filesystem and command APIs outside
`actions`, `adapters`, and tests.

View File

@@ -1,271 +0,0 @@
# Flow CLI -- Code Review
> Reviewed 2026-03-15 against commit `24d682a` (main). Source: ~6,900 LOC across 21 modules. Tests:
> ~2,600 LOC, 167 pass / 6 skip.
---
## 1. Reason to Exist
Flow solves a real problem: managing a personal dev environment across machines. It unifies dotfiles
linking, machine bootstrapping, container management, SSH entry, and binary package installs into
one CLI. The alternative is a pile of shell scripts or Ansible for what is essentially
personal-workstation setup. Flow is lighter, more opinionated, and self-hosted (config lives in your
dotfiles repo). This is a reasonable project to maintain.
---
## 2. Features Supported
| Area | Implemented | Notes |
| ------------ | --------------------------------------------------------------------------------------------- | ----------------------------------------- |
| `enter` | SSH handoff with tmux, terminfo warnings, target config | Works |
| `dev` | Container create/exec/connect/list/stop/remove/respawn | Docker & Podman |
| `dotfiles` | init/link/unlink/undo/status/sync/relink/clean/edit, modules, repo ops | Most complex subsystem |
| `bootstrap` | Profile-driven provisioning: packages, shell, locale, hostname, ssh-keygen, runcmd, post-link | Works |
| `package` | Binary package install/list/remove from manifest | Real install; "remove" only forgets state |
| `sync` | Git health check across `~/projects` | Works |
| `completion` | Dynamic zsh completion with context-aware candidates | Thorough |
---
## 3. Mismatches Between Docs/README and Source Code
### 3.1 Architecture doc describes layers that don't fully exist yet
`docs/architecture.md` describes a clean adapter/service/runtime split:
> `flow.commands.*` — argparse registration and compatibility wrappers only `flow.services.*` —
> domain behavior
In practice:
- **`commands/sync.py`** still contains ~170 lines of its own git logic (`_git`, `_check_repo`,
`run_check`, `run_fetch`, `run_summary`) that duplicate `services/projects.py` almost
line-for-line. The service exists but the command module **never imports or uses it**.
- **`commands/package.py`** does its own JSON state loading and binary install calls instead of
delegating to `services/packages.py`. The service class `PackageService` exists but is **never
used anywhere**.
- **`commands/dotfiles.py`** is a 193-line shim that re-exports ~20 private functions from
`services/dotfiles.py`, each wrapped in `_sync_service_module()`. The command module is not
"argparse registration only" -- it's a compatibility layer that monkeypatches module-level
constants into the service at runtime.
**Verdict:** The architecture doc describes the target state, not the actual state. Two service
modules (`projects`, `packages`) are dead code.
### 3.2 README says "`environments` is not supported" but code still checks for it
`services/bootstrap.py:63` raises `RuntimeError` if `environments` key is found. This is actually
correct defensive code, but the README makes it sound like the key is simply ignored. Minor.
### 3.3 README documents `flow dev create api -i tm0/node -p ~/projects/api`
The code works, but the container is always `sleep infinity` + exec-in. README doesn't mention this
is the execution model (not a standard container entry point).
### 3.4 `docs/flows.md` lists "Keep But Treat As Convenience Aliases" section
These aliases aren't flagged in code or docs as aliases. `dotfiles sync` is described as
"`repo pull` + `modules sync`", but in code it's `_pull_dotfiles` + `_sync_modules` -- not actually
calling the repo/modules commands. Functionally equivalent but not aliased.
---
## 4. Code Quality Assessment
### 4.1 Bugs
#### Duplicate method definition in `system.py`
`FileSystem.write_bytes` is defined **twice** (lines 259-261 and 263-265). The second definition
silently shadows the first. They're identical, so no behavioral bug, but this is a clear copy-paste
error.
#### `commands/container.py` has unused legacy functions
`_container_exists`, `_container_running`, and `_tmux_fallback` are defined at module level (lines
61-122) but never called -- the `ContainerService` class has its own versions. Dead code.
#### `commands/bootstrap.py` monkeypatches the service module
```python
def _sync_service_module() -> None:
_service.shutil = shutil
_service.urllib = urllib
_service._copy_install_item = _copy_install_item
```
This replaces `shutil` and `urllib` on the service module object at runtime. The service already
imports these -- this patching has no real effect (the service's own imports win).
`_copy_install_item` patch is circular: it reassigns the service function to a wrapper that calls
the original. This is effectively a no-op.
#### `dotfiles.py` command module monkeypatches module-level constants
```python
def _sync_service_module() -> None:
_service.DOTFILES_DIR = DOTFILES_DIR
_service.MODULES_DIR = MODULES_DIR
_service.LINKED_STATE = LINKED_STATE
...
```
The service already imports these from `flow.core.paths`. This override only matters if the command
module's own constants diverge from the service's -- but they're imported from the same source. The
whole pattern is a code smell from an incomplete refactoring.
### 4.2 Massive Code Duplication
This is the single biggest quality issue. The bootstrap service duplicates nearly every function
from `package_defs.py`:
| `services/package_defs.py` | `services/bootstrap.py` |
| ------------------------------------- | ------------------------------------ |
| `linux_detect_package_manager()` | `_linux_detect_package_manager()` |
| `resolve_package_manager()` | `_resolve_package_manager()` |
| `get_package_catalog()` | `_get_package_catalog()` |
| `normalize_profile_package_entry()` | `_normalize_profile_package_entry()` |
| `resolve_package_spec()` | `_resolve_package_spec()` |
| `resolve_pkg_source_name()` | `_resolve_pkg_source_name()` |
| `platform_lookup_keys()` | `_platform_lookup_keys()` |
| `resolve_binary_platform_vars()` | `_resolve_binary_platform_vars()` |
| `resolve_binary_asset()` | `_resolve_binary_asset()` |
| `resolve_binary_download_url()` | `_resolve_binary_download_url()` |
| `validate_declared_install_path()` | `_validate_declared_install_path()` |
| `install_destination()` | `_install_destination()` |
| `install_strip_prefix()` | `_install_strip_prefix()` |
| `strip_prefix()` | `_strip_prefix()` |
| `BinaryInstaller.install()` | `_install_binary_package()` |
| `BinaryInstaller.copy_install_item()` | `_copy_install_item()` |
| `profile_template_context()` | `_profile_template_context()` |
| `render_template_value()` | `_render_template_value()` |
That's **18 duplicate function pairs** with near-identical implementations. `package_defs.py` was
clearly extracted as the canonical version, but `bootstrap.py` was never updated to import from it.
Similarly, `services/projects.py` duplicates `commands/sync.py`.
Total duplicate code: roughly 500-600 lines.
### 4.3 Module-Level Singletons
`services/dotfiles.py` and `services/bootstrap.py` use module-level singletons:
```python
_RUNNER = CommandRunner()
_FS = FileSystem()
```
These are then patched by the command modules. This makes the service modules hard to test in
isolation and creates hidden mutable global state. The `FlowContext` already carries a
`SystemRuntime` with `runner`, `fs`, and `git` -- but the service code ignores it and uses the
module-level singletons instead.
### 4.4 Tests Reach Into Private APIs
Tests import private (underscore-prefixed) functions directly:
```python
from flow.commands.bootstrap import (
_ensure_required_variables,
_get_profiles,
_install_binary_package,
...
)
```
This couples tests to internal implementation details. Combined with the command-module shimming
pattern, it means the test suite is testing the wrong layer (command shims instead of service logic
directly).
### 4.5 Inconsistent Error Handling
Two exception hierarchies coexist:
- `FlowError(RuntimeError)` -- defined in `core/errors.py`, used by `system.py` and services
- Plain `RuntimeError` -- used throughout `bootstrap.py`, `dotfiles.py`, and command modules
The CLI entry point catches `RuntimeError` as the generic error type, so both work. But `FlowError`
was clearly intended to be the project-wide error type and is underused. The `bootstrap.py` service
(1001 lines) never imports or uses `FlowError`.
### 4.6 `stow.py` is Unused
`core/stow.py` (358 lines) implements a GNU Stow-style tree folding/unfolding algorithm with
`LinkTree`, `TreeFolder`, `LinkOperation`. It has 310 lines of tests. But **nothing in the
application imports or uses it**. The dotfiles service has its own `LinkSpec`-based approach. This
is dead code (or a planned feature that never landed).
### 4.7 `action.py` is Unused
`core/action.py` (120 lines) defines `Action` and `ActionExecutor` for plan-then-execute workflows.
It has 115 lines of tests. But **no command or service uses it**. The bootstrap command does its own
sequential execution. Dead code.
### 4.8 `process.py` is a Thin Wrapper With Odd Restrictions
```python
def run_command(command, console, *, check=True, shell=True, capture=False):
if not shell:
raise RuntimeError("run_command only supports shell commands")
```
It rejects `shell=False` even though the parameter exists in the signature. This wrapper creates a
new `CommandRunner()` on every call instead of using the one from context.
### 4.9 Minor Issues
- **No type checking enforcement.** `pyproject.toml` has no mypy/pyright configuration. Type
annotations exist but are never validated.
- **`ConsoleLogger` hardcodes ANSI codes.** No TTY detection, no `--no-color` flag. Piping output to
a file produces escape sequences.
- **`flow package remove` only forgets state.** README and `docs/flows.md` both acknowledge this,
but the CLI help text says "Remove installed packages" without qualification. Users will expect
files to be deleted.
- **SSH keygen quoting.** `bootstrap.py:747` passes `shlex.quote` results into a string that is
later shell-executed. Double quoting could be an issue with values containing spaces, though
unlikely in practice for key filenames.
---
## 5. Summary
### What's Good
- **Clear domain model.** The manifest format is well-designed: profiles, packages, dotfiles layout,
modules, templates. It's genuinely useful.
- **Transactional dotfiles linking.** The undo system with snapshots and backup directories is
well-thought-out and more robust than most dotfile managers.
- **Defensive input validation.** Config parsing handles multiple formats (dict keys, lists, string
shorthand), missing values, and type mismatches gracefully.
- **Test coverage exists.** 167 tests pass. The dotfiles folding and e2e container tests show real
investment in correctness.
- **Clean CLI surface.** Argparse registration is consistent, aliases work, help text is clear.
- **Zsh completion is thorough.** Context-aware dynamic completion is a nice touch.
### What Needs Work
| Priority | Issue | Effort |
| -------- | ------------------------------------------------------------------------------------------------------------------- | ----------------------------------------------------- |
| High | ~600 lines of duplicated code between bootstrap and package_defs | Delete duplicates, import from package_defs |
| High | Two dead modules (stow.py, action.py) + dead service modules (projects.py, packages.py) = ~900 lines of unused code | Delete or wire up |
| High | Command modules bypass their own service layer | Finish the refactoring described in architecture.md |
| Medium | Module-level singletons vs FlowContext.runtime | Use runtime from context consistently |
| Medium | `_sync_service_module()` monkeypatching pattern | Remove once services use their own imports |
| Medium | Inconsistent error types (RuntimeError vs FlowError) | Standardize on FlowError |
| Low | No color/TTY detection | Add `--no-color` or detect `isatty` |
| Low | `package remove` semantics | Either implement real uninstall or rename to `forget` |
| Low | Duplicate `write_bytes` method | Delete the duplicate |
### Overall Assessment
This is a **functional but mid-refactoring codebase**. The core functionality works -- dotfiles
linking, bootstrapping, container management, and SSH entry all do what they claim. The manifest
model and transactional undo are genuinely well-designed.
The main debt is architectural: a service-layer extraction was started but left incomplete,
resulting in large amounts of duplicated code, dead modules, and a monkeypatching compatibility
layer. The tests pass but are coupled to the wrong abstraction layer.
For a personal "vibe-coded" tool, this is solid. For production or team use, the duplication and
dead code need cleanup before adding more features.

View File

@@ -1,180 +0,0 @@
# Flow CLI Refactor Status
> Based on code review (2026-03-22), architecture discussion, and the current implementation.
> Spec: `docs/superpowers/specs/2026-03-16-flow-architecture-redesign.md`
## Current State
The action-runtime rewrite is implemented. `cli.py` is a thin Typer adapter, `flow.app` owns
application orchestration, domain modules keep pure planning and resolution logic, and
executor-managed mutations are represented as action plans before they reach runtime adapters.
The old structural problems from the original codebase (duplicated flows, monkeypatching, dead
modules, singleton-style runtime access) have been removed from the active command paths. The
remaining refactor work is deferred cleanup, not a blocker for the action-centered architecture.
---
## Command Surface
This is the implemented command surface.
```
flow remote enter <target> # Host only. SSH+tmux into VM.
flow remote list # List configured targets.
flow dev create <name> -i <image> # VM only. Create+start container.
flow dev attach <name> # Attach to container tmux session.
flow dev exec <name> [cmd...] # Run command in container.
flow dev enter <name> # Interactive shell in container.
flow dev list # List dev containers.
flow dev stop <name> # Stop container.
flow dev rm <name> # Remove container.
flow dev respawn <name> # Respawn tmux panes.
flow dotfiles init --repo <url> # Clone dotfiles repo + all module repos.
flow dotfiles link [--profile p] # Reconcile symlinks (creates, fixes broken, removes stale).
flow dotfiles unlink [packages...] # Remove managed symlinks.
flow dotfiles status [packages...] # Show packages, link health, module info.
flow dotfiles edit <package> # Pull -> $EDITOR -> commit+push.
flow dotfiles repos list # List ALL managed repos (dotfiles + modules).
flow dotfiles repos status [--repo=x] # Git status for one or all repos.
flow dotfiles repos pull [--repo=x] [--dry-run]
flow dotfiles repos push [--repo=x] [--dry-run]
flow setup run [profile|--profile p] [--dry-run] [--var KEY=VALUE]
flow setup list # List profiles.
flow setup show <profile> # Show profile plan.
flow packages install [name...] [--profile p] [--dry-run]
flow packages list [--all] # List packages.
flow packages remove <name...> [--dry-run]
flow projects check [--fetch] # VM only. Git health across ~/projects.
flow projects fetch # Fetch all project remotes.
flow projects summary # Quick status overview.
```
### Aliases
- `dotfiles` -> `dot`
- `packages` -> `package`, `pkg`
- `projects` -> `project`; `flow sync` -> `flow projects check --fetch`
- `setup` -> `bootstrap`, `provision`
- `remote enter` -> `enter`
- `dev attach` -> `dev connect`
- `dev rm` -> `dev remove`
- `dotfiles repos` -> `dotfiles repo`
### Global flags
- `--version`
- `--quiet` / `-q`
- `--no-color`
### Commands Removed During Refactor
| Removed | Reason |
|---------|--------|
| `dotfiles sync` | Redundant: `repos pull` + `link` |
| `dotfiles relink` | Redundant: `link` is idempotent |
| `dotfiles undo` | Redundant: `unlink` is the inverse of `link` |
| `dotfiles clean` | Folded into `link` (reconciliation handles broken symlinks) |
| `dotfiles modules list` | Replaced by `dotfiles repos list` |
| `dotfiles modules sync` | Replaced by `dotfiles repos pull` |
## Action-Centered Architecture
The runtime boundary is `flow.actions`.
- `ActionPlan` is the unit of execution. It can contain high-level `DomainAction` entries and direct
`PrimitiveAction` entries.
- `DomainAction` records intent from a domain such as dotfiles, packages, repos, remote targets,
containers, completion, or setup.
- `expand_actions()` converts domain actions into primitive actions. Some domains supply already
expanded primitive plans when the service has concrete runtime arguments.
- `PrimitiveAction` is the canonical executor input for filesystem, process, git, download,
archive, container, and tmux operations.
- `ActionExecutor` owns dry-run output, append-only JSONL audit logging, rollback stack management,
rollback barriers, and dispatch into `SystemRuntime`.
App use-cases construct plans and pass them to the executor for action-backed commands. Direct
runtime calls are limited to explicit interactive boundaries such as attaching to tmux or entering a
container shell. Domain modules stay free of I/O where the current implementation has pure
resolution/planning functions.
Rollback is best-effort and explicit. Actions default to `rollbackable`; external boundaries such
as shell commands, remote sessions, and non-reversible git/container operations use `barrier` or
`none` policies.
## Key Design Decision: Modules Are Repos
The `_module.yaml` files define external git repos that provide content for dotfiles packages.
These module repos are **not** git submodules -- they are regular git clones managed by flow.
The dotfiles repo itself is also a git clone managed by flow. So **all managed git repos** (the
dotfiles repo + every module repo) share the same abstraction and the same commands.
`dotfiles repos` is the single entry point for all repo operations. There is no separate
`dotfiles modules` subcommand group. The `--repo=<name>` flag filters to a specific repo when
needed (dotfiles repo is named `dotfiles`, module repos are named by their package, e.g. `nvim`).
---
## Completed Work
### Unified Repos Abstraction
`RepoInfo` with `module_ref` is the canonical repo model. `_discover_repos()` finds the dotfiles
repo and module repos, and `repos list/status/pull/push` iterate that single collection with an
optional `--repo` filter. `dotfiles init` uses the same pull-or-clone flow.
### Command Trimming
Removed redundant dotfiles commands:
- `dotfiles sync`: use `dotfiles repos pull` plus `dotfiles link`
- `dotfiles relink`: `dotfiles link` is idempotent
- `dotfiles undo`: use `dotfiles unlink`
- `dotfiles clean`: broken symlink repair is part of link planning
- `dotfiles modules list/sync`: use `dotfiles repos list/pull`
### Feature Completion
- `dotfiles edit`: pull -> `$EDITOR`/`$VISUAL` -> scoped `git add` -> commit+push, with
`--no-commit` to skip auto-commit/push.
- `dotfiles status`: module info, link health, and package filtering.
- `dotfiles repos list`: all managed repos with name, type, local path, and clone status.
- `--no-color`: global flag added to `cli.py`.
- `--dry-run`: supported by dotfiles link/unlink, repos pull/push, packages install, setup run,
remote enter, and dev create.
### Improvements Over Spec
These are correct deviations -- the implementation improved on the spec:
- `adapters/containers.py` + `adapters/tmux.py` extracted as adapters (spec had them inline)
- `core/config_parse.py` + `core/yaml.py` extracted for config parsing
- `SystemRuntime` extended with containers, tmux, download, and archive runtime fields
- `flow.actions` extracted as the canonical execution layer instead of leaving mutation dispatch in
individual app use-cases
---
## CI
The GitHub Actions workflow is split into two jobs:
- `unit`: installs dependencies and runs `pytest tests/ -v --ignore=tests/e2e`
- `e2e`: verifies Docker is available, sets `FLOW_RUN_E2E=1`, and runs `pytest tests/e2e/ -v`
---
## Optional Future Work
These are optional refinements, not blockers for the action-centered rewrite.
### Global `--dry-run`
If per-command `--dry-run` becomes a maintenance burden, promote to a global flag in `cli.py`.

File diff suppressed because it is too large Load Diff

File diff suppressed because it is too large Load Diff