83 lines
16 KiB
Markdown
83 lines
16 KiB
Markdown
## Context
|
|
|
|
`apex-agent-mvp` is code-complete (35/38 tasks; the remaining 3 require a physical iPhone and are unaffected by this change) but was never archived — `openspec/specs/` is still empty, so its capability specs (`device-management`, `scene-perception`, `agent-runtime`, `task-memory`, `mcp-tool-server`) exist only as pending deltas under `openspec/changes/apex-agent-mvp/specs/`. The working codebase already reflects a mostly driver-agnostic design: `Driver` is an ABC with no framework types leaking through, `WDADriver` is the only implementation, `DeviceManager` tracks devices by an opaque `driver_type` string, and `Scene` (not raw XML/OCR boxes) is the only thing the LLM-facing tools ever touch. The gaps are cosmetic-but-real: the package holding `Driver`+`DeviceManager` is called `core/` (a grab-bag name), the perception package is called `vision/` (names the technique, not the capability), the shared error base is literally `ApexAgentError`, and the one true "pick a driver by type" extension point (`SUPPORTED_DRIVER_TYPES` in `api/console.py`) lives in the API layer instead of the driver layer. Two other changes (`skill-catalog-subscription`, `web-console`) are proposed but unapplied and untouched by this change.
|
|
|
|
Two stakeholders: the LLM/agent runtime consuming the driver/device/tools/perception layers (must see zero behavior change), and future contributors/AI coding agents who will read `docs/CONSTITUTION.md` and `docs/ROADMAP.md` before adding the next driver or milestone.
|
|
|
|
## Goals / Non-Goals
|
|
|
|
**Goals:**
|
|
- Zero behavior change: every existing test passes after the move with only import-path edits, not logic edits.
|
|
- Make the physical package layout match the mental model: `driver/` (interface + implementations), `device/` (lifecycle/registry of *devices*, not drivers), `core/` (truly shared models/errors only).
|
|
- Give driver-type registration a home in `driver/` so a second driver (Android, browser, ...) never requires touching `api/console.py`.
|
|
- Give perception-technique registration the same treatment via a `PerceptionProvider` port, so OCR is one swappable implementation, not the pipeline's identity.
|
|
- Produce durable planning artifacts (`ROADMAP.md`, two ADRs, `CONSTITUTION.md`, `research/` scaffold) that later milestone changes can be scoped against, instead of re-deriving architecture context from `apex-agent-mvp/design.md` each time.
|
|
- Codify a **Hexagonal (Ports-and-Adapters) + DDD layering** as the project's governing architecture, with an explicit dependency direction (domain has zero framework/LLM/HTTP dependencies; adapters depend on domain, never the reverse) and a bottom-up build order that future milestone changes are expected to respect: `core` (domain) → `driver`/`device` (adapters) → `tools` (capability/ports layer) → `perception` (Scene, mockable) → `storage` → `runtime` (application/orchestration) → real perception techniques → LLM-backed planning → `api` (outermost adapter).
|
|
|
|
**Non-Goals:**
|
|
- No second driver implementation (Android/browser/Windows) — only the extension point moves.
|
|
- No changes to `skill-catalog-subscription` or `web-console` proposals/specs.
|
|
- No change to `agent-runtime`, `task-memory`, or `mcp-tool-server` behavior/specs — only their import paths shift if they reference `core.driver`/`core.device_manager`/`vision.*`.
|
|
- No archiving of `apex-agent-mvp` as part of this change — that remains a separate decision for the user (real-device verification tasks 7.1–7.3 are still outstanding). Note `apex-agent-mvp` already bundled Perception (real OCR), Planning (LLM), and API/MCP into one MVP change, ahead of the layering this change now codifies — that is accepted as-is (not rebuilt or reordered); the layering/build-order constraint below governs *future* milestone changes (Semantic, World, Skill, Workflow, Agent, Cloud Runtime), not a retroactive rewrite of already-shipped code.
|
|
- No CI/lint pipeline changes beyond what's needed to keep `pytest` green — mechanical enforcement of the layering (e.g. import-linter contracts) is explicitly deferred, not done now.
|
|
|
|
## Decisions
|
|
|
|
### D1: Split `core/` into `driver/` + `device/`, keep `core/` as a shared-models-only package
|
|
`driver/base.py` gets the `Driver` ABC (from `core/driver.py`); `driver/wda_driver.py` gets `WDADriver` (from `core/wda_driver.py`); `device/manager.py` gets `DeviceManager`/`DriverFactory`/`DEFAULT_MANAGER` (from `core/device_manager.py`). `core/models.py` (Bounds, Device, SceneElement, Scene, Task, Step) and `core/errors.py` stay in `core/`, since they're shared across driver, device, perception, runtime, and storage — not device- or driver-specific.
|
|
- **Alternative considered**: Eliminate `core/` entirely and push models/errors into whichever package "owns" them most (e.g. `Scene`/`SceneElement` → `perception/`, `Task`/`Step` → `runtime/`). Rejected for this change — it would force `device/` to import from `perception/` and `runtime/` to import from `perception/`, creating cross-package coupling that doesn't exist today; splitting shared models is a legitimate follow-up but is a behavior-neutral naming change's job to avoid, not force.
|
|
|
|
### D2: Rename `vision/` → `perception/`
|
|
Straight package rename, same file names inside (`ocr.py`, `ui_parser.py`, `scene_builder.py`, `icon_detector.py`). No merging or splitting.
|
|
- **Alternative considered**: Keep `vision/` and only rename the *pipeline* concept in docs. Rejected — the proposal's own roadmap explicitly calls this stage "Perception," and leaving the package named after one input signal (OCR/vision) while `scene_builder.py` already fuses tree+OCR+vision is the exact naming drift this change exists to fix.
|
|
|
|
### D3: Driver Registry moves into `driver/registry.py`, `api/console.py` imports from it
|
|
New `driver/registry.py` holds `SUPPORTED_DRIVER_TYPES: dict[str, DriverFactoryBuilder]` and `build_driver_factory(driver_type, connection_info)`, moved verbatim from `api/console.py` (currently lines ~17-50). `api/console.py` imports `build_driver_factory` from `driver.registry` instead of defining it. Adding `AndroidDriver` later means adding one entry to `driver/registry.py`, not touching `api/console.py`.
|
|
- **Alternative considered**: Leave the registry in `api/console.py` and only document the intent to move it later. Rejected — this is precisely the kind of small, cheap, high-leverage structural fix Milestone 0 exists to make before a second driver shows up and someone has to thread a device-agnostic factory builder through an API-layer dict under time pressure.
|
|
|
|
### D4: Rename `ApexAgentError` → `DeviceRuntimeError`
|
|
`core/errors.py`'s base class and its subclasses (`DriverError`, `DeviceNotFoundError`, `DeviceOfflineError`, `DeviceBusyError`, `ElementNotFoundError`, `TaskFailedError`) keep their names; only the base changes from `ApexAgentError` to `DeviceRuntimeError`. All `except ApexAgentError` / `raise ApexAgentError` call sites across `api/`, `tools/`, `runtime/`, `tests/` update accordingly.
|
|
- **Alternative considered**: Keep `ApexAgentError` as a deprecated alias (`ApexAgentError = DeviceRuntimeError`) for backward compatibility. Rejected — there are no external consumers of this exception type yet (nothing has shipped), so a compatibility shim adds dead code for a compatibility need that doesn't exist.
|
|
|
|
### D5: Rebrand via `pyproject.toml` + root `README.md`, not a source-wide string sweep
|
|
`pyproject.toml`'s `name` becomes `device-agent-runtime` and a `description` field is added; a root `README.md` (currently absent) states the new positioning and links to `docs/ROADMAP.md`. `apex-agent-mvp/proposal.md` and `design.md` are left untouched as a historical record of the original framing — they describe a change that already happened, and rewriting history in a planning artifact adds no value.
|
|
- **Alternative considered**: Grep-and-replace every "Apex Agent" / "IPA" string across existing `openspec/changes/apex-agent-mvp/**` docs. Rejected per the chosen change scope (foundation docs + restructure only, not reconciling other pending changes) — those files are a record of a past decision, not living documentation.
|
|
|
|
### D6: Roadmap, ADR, Constitution, and research/ track are pure additions, no code coupling
|
|
`docs/ROADMAP.md`, `docs/adr/0001-device-agnostic-runtime.md`, `docs/CONSTITUTION.md`, and `research/00N-*/README.md` are net-new Markdown files with no imports from or references into runtime code, so they carry zero risk to the passing test suite.
|
|
- **Alternative considered**: Encode `docs/CONSTITUTION.md`'s invariants as enforced lint rules (e.g. import-linter contracts forbidding `tools/` from importing `driver/` directly). Deferred, not rejected — worth doing once there's a second driver to actually violate the boundary against; adding enforcement machinery for a boundary nothing has crossed yet is premature for a foundation change.
|
|
|
|
### D7: Adopt Hexagonal + DDD layering as the governing architecture, recorded in a second ADR
|
|
`docs/adr/0002-layered-hexagonal-architecture.md` records the dependency direction and build order as project law: `core` (domain: `Bounds`/`Device`/`Scene`/`Task`/`Step`/errors) has zero framework, LLM, or transport dependencies; `driver`/`device` are adapters translating external device SDKs (Appium/WDA today) into the domain's `Driver` contract; `tools` is the capability/port layer the Agent Runtime calls; `perception` produces `Scene` behind the `PerceptionProvider` port (D8); `storage` persists timeline/task state; `runtime` (Planner/Executor) is the application layer that orchestrates the above; only at `runtime`'s Planner does an LLM enter the picture, and only `api` (REST/MCP) is HTTP/MCP-transport-aware. This is recorded as a standing constraint for *future* milestone changes (Semantic, World, Skill, Workflow, Agent, Cloud Runtime): none of them may introduce LLM or transport dependencies into `core`, `driver`, `device`, or `tools`.
|
|
- **Alternative considered**: Leave this as implicit convention (as it already mostly was) rather than a written ADR. Rejected — the explicit trigger for writing it down now is that `apex-agent-mvp` already shipped Perception+Planning+API bundled in one MVP change, i.e. the layering was followed loosely, not strictly by construction; upcoming AI-heavy milestones (Semantic Scene's LLM call, Skill embedding retrieval, Multi-Agent roles) are exactly where it's cheapest to keep LLM/transport concerns out of the domain/adapter layers *before* they're written, not after.
|
|
|
|
### D8: `PerceptionProvider` port mirrors the Driver Registry pattern (D3)
|
|
`perception/provider.py` defines a `PerceptionProvider` ABC with one method, `build_scene(screenshot, tree) -> Scene`. The existing OCR+tree fusion in `scene_builder.py` is wrapped as the default implementation (registered, not rewritten — same behavior, same `Scene` output). A `NullPerceptionProvider` returning an empty `Scene` (correct `width`/`height`, no elements) is added for tests and any environment without OCR dependencies installed. `tools/describe_screen.py` and `runtime/` depend on the port type, not on `scene_builder` directly.
|
|
- **Alternative considered**: Skip the port and let callers import `scene_builder.build_scene()` directly (as today), documenting only that *conceptually* it's swappable. Rejected — the whole point of D7's constraint is that swapping/mocking perception must be a real, exercised code path (useful today for fast tests without OCR installed) rather than an aspiration that nothing currently proves; this mirrors why the Driver Registry (D3) was made a real registry instead of a documented convention.
|
|
|
|
## Risks / Trade-offs
|
|
|
|
- **[Risk]** Renaming `core.driver`/`core.device_manager`/`vision.*` import paths across 37 files by hand risks missing one and breaking an import at runtime rather than at test time → **Mitigation**: run the full `pytest` suite (all of `tests/`) after the move as the acceptance gate in `tasks.md`; a missed import surfaces immediately as a collection error, not a silent behavior change.
|
|
- **[Risk]** Moving `SUPPORTED_DRIVER_TYPES`/`build_driver_factory` out of `api/console.py` could break `tests/test_console_api.py` if it patches/imports those names directly → **Mitigation**: check `test_console_api.py`'s imports as part of the move and update them alongside the production code, in the same task.
|
|
- **[Risk]** Wrapping `scene_builder.py` behind `PerceptionProvider` could subtly change its return value if the wrapper reshapes data → **Mitigation**: the default provider must call the existing `scene_builder` function unmodified and return its result as-is; `tests/test_scene_builder.py` passing unchanged is the acceptance check, not a new test suite.
|
|
- **[Trade-off]** Codifying D7's layering as an ADR/constitution now, without lint enforcement, means it's a convention future contributors (human or AI) must read and follow, not something that fails a build if violated → acceptable per D6's reasoning: enforcement machinery is worth adding once there's a concrete violation to enforce against (e.g. when a Semantic Scene change is tempted to call an LLM SDK from within `core`), not preemptively.
|
|
- **[Trade-off]** Keeping `core/` alive (rather than eliminating it) means the package boundary story is "driver / device / core (shared) / perception / runtime / storage / api" — one more package than the roadmap's illustrative "driver/device/runtime" — acceptable because this codebase already has more layers than the roadmap's Milestone-1-only sketch assumed, and forcing models into whichever package "feels right" would create the cross-package coupling described in D1.
|
|
- **[Trade-off]** Not archiving `apex-agent-mvp` as part of this change means `openspec/specs/` stays empty and this change's own capability (`driver-registry`) has no prior baseline to diff against — acceptable since `driver-registry` is genuinely new (ADDED, not MODIFIED) and archival ordering is the user's call, not a blocker for this change's own correctness.
|
|
|
|
## Migration Plan
|
|
|
|
1. Create new packages/files (`driver/`, `device/`, `perception/`) via `git mv` (preserves history) rather than copy+delete.
|
|
2. Update every import site (37 files) from `core.driver`/`core.device_manager`/`vision.*` to the new paths; rename `ApexAgentError` → `DeviceRuntimeError` at all call sites.
|
|
3. Add `perception/provider.py` (`PerceptionProvider`, `NullPerceptionProvider`, default provider wrapping `scene_builder.py`); point `tools/describe_screen.py`/`runtime/` at the port.
|
|
4. Move `SUPPORTED_DRIVER_TYPES`/`build_driver_factory` into `driver/registry.py`; update `api/console.py`'s imports.
|
|
5. Update `pyproject.toml` (`name`, `description`, `packages.find.include`).
|
|
6. Run `pytest` — must be 100% green with no test-content changes (only import updates inside `tests/` itself where needed).
|
|
7. Add `README.md`, `docs/ROADMAP.md`, `docs/adr/0001-device-agnostic-runtime.md`, `docs/adr/0002-layered-hexagonal-architecture.md`, `docs/CONSTITUTION.md`, `research/00N-*/README.md`.
|
|
8. Rollback: since every step is a rename/move plus additive docs with no data migration, reverting is `git revert` of the commit(s); no runtime state or external system is touched.
|
|
|
|
## Open Questions
|
|
|
|
- Whether `core/` should eventually be renamed too (e.g. `shared/` or `domain/`) once it's clearer which models belong to which future layer (World Model in Milestone 6 will likely want to own more of `core/models.py`) — left open, not blocking this change.
|
|
- Whether `docs/CONSTITUTION.md` should later be enforced mechanically (import-linter, custom lint rule) once a second driver exists to validate the boundary against — deferred to a future change per D6.
|
|
- Whether the Semantic Scene (Milestone 5), World Model (Milestone 6), and Skill Learning (Milestone 7) changes — currently being drafted in parallel — correctly keep their LLM/embedding calls confined to a `runtime`-level (or new dedicated) layer rather than `perception`/`core`, per D7's constraint; worth a follow-up read once those changes are drafted.
|