diff --git a/.gitignore b/.gitignore index 4cc2457..a74c7b6 100644 --- a/.gitignore +++ b/.gitignore @@ -13,3 +13,4 @@ Thumbs.db *.swo /vendor/ +/tapper.sock diff --git a/AGENTS.md b/AGENTS.md new file mode 100644 index 0000000..f1451e0 --- /dev/null +++ b/AGENTS.md @@ -0,0 +1,95 @@ +# AGENTS.md + +Guidance for AI coding agents (and humans) working in this repository. Read this before making changes. Deeper topics are split into `docs/` — see the index at [`docs/README.md`](docs/README.md). + +## What this project is + +Tapper is a PHP debugging tool in the shape of `Ray`/`Clockwork`: you call `tp($value)` anywhere in a PHP application, and the payload shows up live in a terminal UI (TUI) running as a separate process. + +**Two things are true about this repo at once, and both should shape how you work in it:** + +1. It is a real, shippable debugger with its own README and CI. +2. It is deliberately being used as a **research vehicle**. The longer-term goal (not yet started) is a general PHP desktop-app framework with a swappable rendering backend — this TUI today, a GLFW-based GUI later. Tapper exists to discover, under real pressure, what that framework's abstractions should be, before anything is extracted into a separate package. + +Decision from 2026-08-15: **do not extract a framework yet.** Keep building Tapper as an application first. Extracting now, from a single example, would mean guessing at the TUI/GLFW boundary rather than discovering it — see [`docs/known-issues.md`](docs/known-issues.md#framework-extraction-readiness) for what "ready to extract" will look like and why it isn't yet (the widget/layout layer is currently *not* abstracted — components build `php-tui` widgets directly). + +## Repository layout + +``` +bin/tapper CLI entrypoint — boots the DI container and runs the TUI Application +src/ + helpers.php Global tp() function (the public API surface for debuggee apps) + Runtime/Tapper.php Client side: collects debug info, sends it over the wire, blocks for the reply + Rpc/ Minimal JSON-RPC-ish request/response types + blocking socket client + Server.php TUI-process side: unix socket server, decodes requests, mutates AppState + Console/ + main.php DI container wiring (php-di) for the TUI process + Application.php Owns the ReactPHP event loop: render timer, resize timer, input handling + Component.php Base class for every UI component (see docs/console-framework.md) + EventBus.php Pub/sub used for key/mouse/custom events + CommandInvoker.php Thin wrapper around php-di's Invoker (see Commands note below) + MessageFormatter.php JSON syntax highlighting for log payloads + CommandAttributes/ #[KeyPressed] #[Mouse] #[OnEvent] #[Periodic] #[FirstRender] + Commands/Command.php Abstract marker class — currently has zero implementations (see known-issues.md) + Components/ Header, LogList, LogItem, Details, Navigation, Splash + Windows/ Main (root layout), Popup (stub, unused) + State/AppState.php Central observable state store (magic __get/__set) + State/LogItem.php Value object for a single log entry + Support/Scroll.php Cursor/offset scrolling math — the best-tested module in the repo +docs/ Deeper documentation — see docs/README.md +examples/BasicExample.php Runnable demo of the tp() API +tests/Unit/ScrollTest.php Only test suite that currently exists +``` + +## Running things + +```bash +composer install + +# Start the TUI (in one terminal) +php bin/tapper + +# In another terminal/process, run something that calls tp() +php examples/BasicExample.php +``` + +There is no `composer.json` `bin` alias beyond `bin/tapper`; it is run directly with `php`. + +### Tests & lint + +```bash +composer test:unit # pest --compact +composer test:lint # pint --test (check only, does not auto-fix) +vendor/bin/pint # auto-fix style +``` + +CI (`.github/workflows/tests.yml`, `static.yml`) runs `test:unit` on PHP 8.2/8.3/8.4 × ubuntu/macos, and `test:lint` on PHP 8.3. There is no static analysis tool (phpstan/psalm) configured despite the workflow being named "Static Analysis" — it currently only runs Pint (code style), not type checking. + +## Key architectural facts an agent should know before editing + +- **Two processes, two concurrency models.** The debuggee (anything calling `tp()`) runs normal synchronous PHP and blocks on a socket round-trip in `Runtime/Tapper.php::send()`. The TUI process (`bin/tapper`) is fully async on a ReactPHP event loop. Never assume `tp()` callers can be non-blocking, and never introduce blocking I/O inside `Console/*` — it runs on the loop. +- **State flows one way into the TUI:** `Server.php` decodes a socket message → builds a `State\LogItem` → calls `AppState::appendLog()` → `AppState` notifies its `onChange` callback → `Application` sets `shouldDraw = true` → next 1/60s tick redraws. Components read `AppState` directly in `view()`; they do not receive props from parents in a React sense — see `docs/console-framework.md`. +- **`AppState` uses magic `__get`/`__set`.** Every field must exist as a constructor-promoted property *and* be documented in the `@property` docblock at the top of the class, or IDE/static tooling won't see it. `observe($name, $cb)` validates `$name` against `get_class_vars()` at runtime, not compile time. +- **There is a known, unresolved bug in `AppState` batching** (`deffer()`/`commit()`) — see the `@TODO` comment in `src/Console/State/AppState.php`. Don't build new features on top of `deffer()`/`commit()` without checking whether it still needs fixing. +- **Components declare behavior via PHP 8 attributes**, wired up by reflection in `Component::__construct` (`#[KeyPressed]`, `#[Mouse]`, `#[OnEvent]`, `#[Periodic]`, `#[FirstRender]`). See `docs/console-framework.md` for exact semantics (especially `global: true`, which is easy to get wrong). +- **Rendering is not backend-agnostic.** `Component::view()` returns a concrete `PhpTui\Tui\Widget\Widget`, and every component uses `php-tui` layout primitives (`Area`, `Constraint`, `Direction`, `Layout`) directly. If you are working toward the GLFW goal, do not scatter more `php-tui`-specific calls into component logic without first reading `docs/known-issues.md#framework-extraction-readiness` — the abstraction boundary that would make backends swappable does not exist yet, and it's the single biggest gap. +- **The RPC/transport layer is otherwise unfinished** (see below), though the socket-path bug is fixed: both `Server.php` and `Rpc/JsonRpcClient.php` now resolve the unix-socket path through the shared `Tapper\SocketPath::resolve()` (`src/SocketPath.php`) instead of computing it independently. See `docs/known-issues.md` before touching `Rpc/*`. +- **`docs/openrpc.json` is stale.** It documents an `appendLog` method with a nested `details` param; the actual server (`Server.php`) implements `log` and `wait` methods with flat params. Don't treat `openrpc.json` as ground truth for the wire format — see `docs/rpc-protocol.md` for what's actually implemented. +- **`Commands/Command.php` and `CommandInvoker` have no concrete usages anywhere in the codebase.** Treat this as unfinished scaffolding, not an established pattern to extend — confirm with the project owner before building on it. + +## Conventions to follow when editing + +- PSR-4 autoload root: `Tapper\` → `src/`. Namespaces mirror directory structure exactly. +- `declare(strict_types=1);` at the top of every PHP file — keep this on new files. +- Style is enforced by Laravel Pint (`vendor/bin/pint`) — run it before finishing a change; don't hand-format. +- No PHPDoc/comment blocks explaining *what* code does — only ones explaining non-obvious *why* (this mirrors the project owner's general preference, not something specific to this file). +- Prefer extending the existing attribute/`EventBus` pattern over inventing a new event mechanism for TUI interactions. +- New `AppState` fields: add to the constructor-promoted property list **and** the `@property` docblock in the same change — they're both authoritative and must stay in sync. + +## Where to look next + +- [`docs/README.md`](docs/README.md) — documentation index. +- [`docs/architecture.md`](docs/architecture.md) — full data-flow walkthrough, process boundaries. +- [`docs/console-framework.md`](docs/console-framework.md) — Component/EventBus/AppState mechanics in detail. +- [`docs/rpc-protocol.md`](docs/rpc-protocol.md) — the actual wire protocol between debuggee and TUI. +- [`docs/known-issues.md`](docs/known-issues.md) — bugs, gaps, and the framework-extraction roadmap. \ No newline at end of file diff --git a/docs/README.md b/docs/README.md new file mode 100644 index 0000000..cc750bf --- /dev/null +++ b/docs/README.md @@ -0,0 +1,21 @@ +# Tapper documentation index + +This is the documentation set for the Tapper codebase, written for both humans and AI agents working in this repo. Start with [`../AGENTS.md`](../AGENTS.md) for the condensed orientation (commands, layout, must-know gotchas); come here for depth on a specific subsystem. + +| Doc | Covers | +|---|---| +| [`architecture.md`](architecture.md) | The two-process design, full request/render data flow, event loop structure | +| [`console-framework.md`](console-framework.md) | The TUI's component model: `Component`, attribute-based event binding, `EventBus`, `AppState` reactivity | +| [`rpc-protocol.md`](rpc-protocol.md) | The actual wire protocol between a debuggee process and the TUI process (and where `openrpc.json` disagrees with it) | +| [`known-issues.md`](known-issues.md) | Known bugs, incomplete abstractions, and what "ready to extract a framework" would require | + +## Reading order + +- **New to the codebase, want the big picture:** `architecture.md`, then `console-framework.md`. +- **About to touch anything under `Console/`:** `console-framework.md` first — the attribute/event/state wiring has non-obvious rules. +- **About to touch `Rpc/`, `Server.php`, or `Runtime/Tapper.php`:** `rpc-protocol.md` first — there's a live bug in the client's socket path. +- **Planning ahead toward the GLFW/desktop-framework goal:** `known-issues.md#framework-extraction-readiness`. + +## Source of truth + +Code is the source of truth; these docs describe the state of the repo as of 2026-08-15 and should be updated in the same change that changes the behavior they describe. If a doc and the code disagree, trust the code and fix the doc. \ No newline at end of file diff --git a/docs/architecture.md b/docs/architecture.md new file mode 100644 index 0000000..babf453 --- /dev/null +++ b/docs/architecture.md @@ -0,0 +1,93 @@ +# Architecture + +## Two processes + +Tapper is always at least two OS processes talking over a local unix socket: + +1. **The debuggee** — any ordinary PHP script/app that calls the global `tp()` helper (`src/helpers.php`). Fully synchronous. One `tp()` call = one blocking socket round-trip. +2. **The TUI** — `bin/tapper`, a long-running process built on the ReactPHP event loop, rendering with `php-tui`. Fully async; nothing in this process should ever block. + +There can be many debuggee processes talking to one running TUI. The TUI process owns the socket file and is the server; every debuggee is a client, despite "server" (`src/Server.php`) living in the same package as the "client" (`src/Rpc/JsonRpcClient.php`) — don't let the class names imply which process they run in without checking. + +```mermaid +sequenceDiagram + participant App as Debuggee (sync PHP) + participant Tapper as Runtime\Tapper + participant Sock as unix socket (tapper.sock) + participant Server as Server.php (TUI process) + participant State as AppState + participant Loop as ReactPHP loop + participant TUI as php-tui Display + + App->>Tapper: tp($value) + Note over Tapper: collects caller, backtrace,
code excerpt, microtime + App->>Tapper: (end of statement -> __destruct) + Tapper->>Sock: write JSON-RPC-ish request, blocks on fgets() + Sock->>Server: decoded NDJSON message + Server->>State: appendLog(new LogItem(...)) + State-->>Loop: onChange callback fires + Server-->>Sock: write {"result":"ok"} + Sock-->>Tapper: response + Tapper-->>App: unblocks, statement completes + Loop->>Loop: next 1/60s render tick sees shouldDraw=true + Loop->>TUI: Application::draw() -> Display::draw(widgets) +``` + +## Inside the TUI process + +### Boot (`bin/tapper` → `src/Console/main.php` → `Application::run()`) + +`main.php` builds a `php-di` `ContainerBuilder` with autowiring and attribute support enabled, registers a handful of framework objects (`Terminal`, `PhpTermBackend`, `Display`, ReactPHP `LoopInterface`) as factories, and resolves `Application` out of the container. Every other class in `Console/*` is constructor-injected — there is no manual wiring beyond `main.php`. + +`Application::run()`: +1. Puts the terminal into raw/alternate-screen/mouse-capture mode. +2. Builds the component tree (`Main`, `Popup`) via the container. +3. Registers two `ReactPHP` periodic timers: a resize poll (`RESIZE_RATE = 1/4`s) and a render tick (`RENDER_RATE = 1/60`s, gated by a `shouldDraw` dirty flag). +4. Wires stdin into the event parser and dispatches parsed key/mouse events onto `EventBus`. +5. Starts `Server::run()` (opens the unix socket listener) and then calls `$this->loop->run()`, which blocks until the loop is stopped. + +**Known wart:** `Application::run()` registers `SIGINT`/`SIGTERM` handlers *after* `$this->loop->run()` — since `loop->run()` blocks, those two `addSignal` calls never execute until the loop has already stopped, making them dead code. See `known-issues.md`. + +### Render loop + +Rendering is pull-based and coarse-grained: nothing re-renders incrementally per component. Every render tick (when dirty), `Application::draw()` calls `$this->window->render($area)` on the root `Main` component, which recursively calls `render()`/`view()` on its children and returns one big composed `Widget` tree, handed to `php-tui`'s `Display::draw()`. There is no diffing — `php-tui` and the terminal backend handle only drawing the frame, not minimizing writes at the Tapper level. + +The dirty flag (`shouldDraw`) is set from exactly two places: `AppState::setOnChange()` (any state mutation) and the resize-poll timer noticing the terminal size changed. + +### Input handling + +Raw stdin bytes are parsed by `php-tui/term`'s `EventParser` into typed events (`CharKeyEvent`, `CodedKeyEvent`, `MouseEvent`, ...). `Application::handleEvent()` forwards the supported event types onto `EventBus::emit()`. A second path, `handleEventInTypingMode()`, exists for a not-yet-fully-used "typing mode" on `AppState` that redirects character input differently (used for the pause/`wait()` continue flow scaffolding — not deeply exercised yet). + +See [`console-framework.md`](console-framework.md) for how components subscribe to these events. + +## Outside the TUI process + +### `tp()` client path + +`src/helpers.php` exposes the global `tp($value)`, which constructs `Runtime\Tapper` and calls `->tap($value)`. `Runtime\Tapper::__construct()` immediately walks `debug_backtrace()` to capture caller file/line and a small source-code excerpt around that line (`getCodeExcerpt()`), and resolves the project root via the Composer `ClassLoader`'s own file location (`findProjectRoot()` — brittle if Composer's install layout changes, but currently the only signal available). + +The actual network send happens in `__destruct()`, not in `tap()` — this means a `tp($value)` call sends *after* the full statement completes (e.g. `tp('x')->wait()` sends once, when the temporary `Tapper` object is destroyed at the end of the statement). `wait()` just changes `$this->type` from `'log'` to `'wait'` before send. + +### Transport (`Rpc/*`) + +`Runtime\Tapper::send()` builds a `Rpc\JsonRpcRequest` (method + params) and calls `JsonRpcClient::call()`, which opens a **blocking** `stream_socket_client()` connection, writes one line of JSON, and blocks on `fgets()` for the reply (up to `pauseTimeout`, 3600s by default — long enough to support `wait()` pausing indefinitely for a user to press Enter in the TUI). See [`rpc-protocol.md`](rpc-protocol.md) for the wire format and a live bug in this path. + +### Server (`Server.php`, runs inside the TUI process) + +Listens on a unix socket (`SocketServer('unix://' . $socketPath)`), decodes newline-delimited JSON (`clue/ndjson-react`) per connection, and switches on `method`: +- `log` → builds a `State\LogItem` and calls `AppState::appendLog()`, replies `{"result":"ok"}`. +- `wait` → builds a `State\LogItem` with a "paused" message, then instead of replying immediately, registers a **one-shot-ish** `EventBus::listen(KeyCode::Enter, ...)` closure that writes the reply whenever Enter is next pressed anywhere in the TUI. (Note: this listener is never removed — see `known-issues.md` for the implication if multiple `wait()` calls are in flight.) +- anything else → JSON-RPC style `-32601` "method not found" error. + +## Directory-to-responsibility map + +| Path | Responsibility | +|---|---| +| `src/helpers.php`, `src/Runtime/` | Public API surface used by debuggee apps (`tp()`) | +| `src/Rpc/` | Wire types + blocking client used by `Runtime\Tapper` | +| `src/Server.php` | TUI-side socket listener, the only writer to `AppState` from outside `Console/` | +| `src/Console/State/` | The single source of UI truth (`AppState`) and its value objects | +| `src/Console/*Attributes/` | Declarative event-binding attributes, consumed by `Component` | +| `src/Console/Component.php`, `Windows/`, `Components/` | The component tree and its rendering | +| `src/Console/EventBus.php` | Cross-cutting pub/sub for key/mouse/custom events | +| `src/Console/Support/` | Pure logic extracted out of components (currently just `Scroll`) | \ No newline at end of file diff --git a/docs/console-framework.md b/docs/console-framework.md new file mode 100644 index 0000000..3aa47f6 --- /dev/null +++ b/docs/console-framework.md @@ -0,0 +1,77 @@ +# The console component framework + +Everything under `src/Console/` (minus `State/`, `EventBus.php`, `main.php`, `Application.php`) is built around one base class, `Component`, plus five attributes and one pub/sub bus. This doc is the detailed reference; `AGENTS.md` has the condensed must-know version. + +This layer is **not backend-agnostic today** — `Component::view()` returns a concrete `php-tui` `Widget`, and layout is done with `php-tui`'s `Area`/`Constraint`/`Direction`/`Layout`. Treat everything below as "how the TUI is built," not yet as "the framework" — see [`known-issues.md`](known-issues.md#framework-extraction-readiness). + +## `Component` (`src/Console/Component.php`) + +Abstract base class. Every concrete component (`Header`, `LogList`, `LogItem`, `Details`, `Navigation`, `Splash`, and the two `Windows/*`) extends it and implements one method: + +```php +abstract protected function view(Area $area): Widget; +``` + +### Construction-time wiring + +`Component::__construct()` does five things, in order, using PHP reflection over the concrete class's methods: + +1. Calls a `beforeInit()` method if one exists (via the DI container, so it's autowired). +2. Scans methods for `#[KeyPressed]`, `#[Mouse]`, `#[OnEvent]` attributes and registers a closure with `EventBus::listen()` for each. The closure captures the attribute instance and does the matching described below before calling the method. +3. Scans methods for `#[Periodic]` and registers a ReactPHP periodic timer calling the method directly. +4. Scans methods for `#[FirstRender]` and stashes them to be called once, the first time `render()` is invoked (not from the constructor — `$area` isn't known yet at construction time). +5. Instantiates every class listed in `protected array $components = [...]` via the container (`registerComponents()`), then calls `afterInit()` if it exists. + +Because this all happens in the base constructor via reflection, **every component pays this cost on every instantiation** — including the `LogItem` instances created per visible row in `LogList::ensureVisible()`. This is fine at current scale (a few dozen components) but is worth knowing if you're ever tempted to construct components in a hot loop. + +### Event matching rules + +These are the actual runtime rules baked into the closure registered in step 2 above — read this before adding a new `#[KeyPressed]`/`#[Mouse]` handler, the logic is not obvious from the attribute alone: + +- **`#[KeyPressed($key, $keyModifiers = null, $global = false)]`** — if `$keyModifiers` is set, the event's modifiers must match *exactly* (not "at least contain"). If the component `isActive() === false` and `$global` is `false`, the handler is skipped entirely. +- **`#[Mouse($key, $button = MouseButton::Left, $global = false)]`** — for scroll events (`ScrollUp/Down/Left/Right`) the button is ignored. For click events, the event's button must match `$button` exactly. Same `global`/`isActive()` gating as above. +- **`#[OnEvent($key, $global = false)]`** — matches on `EventBus::emit()`'s custom string keys (e.g. `'resize'`, `'input'`). Same `global`/`isActive()` gating. +- **`#[Periodic($interval, $global = false)]`** — note the `$global` parameter is declared but **not read anywhere** in `Component::__construct()`; periodic timers always fire regardless of `isActive()`. Don't rely on `global: false` doing anything for `#[Periodic]`. +- **`#[FirstRender]`** — fires once, on the first `render()` call, regardless of `isActive()`. + +`isActive()`/`activate()`/`deactivate()` is a per-component boolean the component tree uses to route input only to the "focused" component — see `Windows/Main.php::afterInit()` for the pattern: it observes `AppState`'s `previewLog` and toggles `Details` vs `LogList` active state via `Loop::futureTick()` (deferred by one tick so the activation change doesn't happen mid-event-dispatch). + +### Rendering + +`render(Area $area)` is called top-down starting from `Application::draw()` on the root `Main` component. First call captures `$area` and fires any `#[FirstRender]` methods; every call (first or not) then delegates to the abstract `view($area)`, which returns the actual `Widget` tree for that subtree. Parent components call `renderComponent($childClass, $area)` / `getComponent($childClass)->render($area)` on their registered children — there is no automatic recursive render; each parent's `view()` explicitly renders the children it wants, in the layout it wants (see `Windows/Main.php::view()` for the canonical 3-row `GridWidget` split: header / body / nav). + +Components do **not** receive props from parents. A component that needs external data (e.g. `LogItem` needs to know which log entry to draw) exposes an explicit setter (`LogItem::setData()`) that the parent (`LogList::fill()`) calls before rendering. There's no generalized prop-passing mechanism — each case is bespoke. + +### Commands (currently unused) + +`protected function execute(Command $command): mixed` delegates to `CommandInvoker::invoke()`, which just calls `Invoker\InvokerInterface::call($command)` (php-di's auto-wiring invoker). `Commands/Command.php` is an empty abstract class. **No component anywhere calls `execute()` and no concrete `Command` subclass exists in the codebase.** This looks like the start of a command-object pattern (useful for e.g. undo/redo or macro recording) that was never followed through. Don't assume it's an established convention to build on without checking with the project owner first. + +## `EventBus` (`src/Console/EventBus.php`) + +A flat `string|int => callable[]` registry. `listen($event, $func)` normalizes `KeyCode`/`MouseEventKind` enum keys to strings (`$event->name`, or `"Mouse{$event->name}"` for mouse kinds) before storing. `emit($event, $data = [])` does the same normalization on the way out, so a `KeyPressed(KeyCode::Enter)` attribute and a raw `EventBus::listen(KeyCode::Enter, ...)` call in `Server.php` end up on the exact same registry key (`Server.php` relies on this — see `architecture.md`'s note on `wait`). + +There is no unsubscribe. Every `listen()` call is permanent for the lifetime of the process. Combined with `Server.php`'s `wait` handler registering a fresh `KeyCode::Enter` listener per `wait()` call (see `known-issues.md`), this bus only ever grows. + +`emit()` silently no-ops if nothing is registered for a key — there's no way to detect "nobody handled this event." + +## `AppState` (`src/Console/State/AppState.php`) + +A single, app-wide, mutable object. It is the *only* channel by which the socket-facing `Server.php` communicates with the rendering tree — there is no separate "props" or "context" system. + +- **Magic properties**: `__get`/`__set` proxy to real private/constructor-promoted properties. The `@property` PHPDoc block at the top of the class is what makes these visible to IDEs/static analysis at all — it is not generated, it must be hand-kept in sync with the constructor's promoted properties. If you add a field to one and forget the other, you get either a silent dynamic property (constructor not updated) or an IDE that can't see a real property (docblock not updated). +- **Change notification**: every `__set` (outside a batch) calls `notifyChange()` (the single `$change` callback registered once, by `Application::startRendering()`, to flip `shouldDraw = true`) and `callObservers($name)` (per-field subscribers registered via `observe($name, $callable)`). +- **Field-specific observers**: `observe(string $name, callable $callable)` validates `$name` exists via `get_class_vars($this::class)` at call time (a `RuntimeException` if you typo a field name — this is your only safety net, and it's runtime-only). Multiple components observe overlapping fields today, e.g. `LogList::beforeInit()` observes `logs`, `cursor`, and `live` to keep unread counts and live-follow behavior in sync — read that method as the canonical example of cross-field reactive logic. +- **Batching**: `deffer()` (sic — note the actual method name has this typo, not "defer") sets a `batching` flag; subsequent `__set` calls accumulate field names into `$changed` instead of notifying immediately. `commit()` flips batching off, replays observers for every accumulated field, then calls `notifyChange()` once. **There is a documented, unresolved bug here** — see the `@TODO` comment directly in `AppState.php`: repeatedly setting the same field while batched (the example given is pressing Enter repeatedly while `tp()->wait()` is paused) causes `$changed` to "overflow" in some way that isn't fully diagnosed. Don't build new batched-write logic on top of `deffer()`/`commit()` until this is understood; prefer direct `__set` (unbatched) if you're unsure. + +## `Support/Scroll.php` + +The one piece of business logic in `Console/` that's pure enough to unit test in isolation, and the only class in the repo with a real test suite (`tests/Unit/ScrollTest.php`). It operates purely on `(cursor, offset)` pairs read/written through `AppState`, given `count`/`visible` as parameters — no widget, no rendering, no reflection. `LogList` and `Details` both have their own scroll-adjacent logic; `LogList` delegates to a `Scroll` instance, `Details` reimplements similar (but not identical, and untested) up/down/page logic inline (`Components/Details.php::up()/down()/pageUp()/pageDown()`) instead of reusing `Scroll`. If you're touching scrolling behavior in `Details`, consider whether it should be unified with `Support/Scroll` rather than diverging further. + +## Practical checklist for adding a new component + +1. Extend `Component`, implement `view(Area $area): Widget`. +2. If it needs to be constructed by a parent, add its class name to the parent's `protected array $components = [...]` and register it in the container graph implicitly via autowiring (no explicit binding needed for a plain class). +3. If it needs external data from its parent, add an explicit setter — don't reach for a generic prop-passing mechanism, there isn't one. +4. If it needs input, use `#[KeyPressed]`/`#[Mouse]`/`#[OnEvent]` — remember `global: true` if it should react while not the "active" component (e.g. a `q`-to-quit handler). +5. If it needs one-time setup once its `Area` is known, use `#[FirstRender]`, not the constructor. +6. If it reads `AppState`, read directly (`$this->appState->foo`) inside `view()` or an event handler — don't invent a new state channel. \ No newline at end of file diff --git a/docs/known-issues.md b/docs/known-issues.md new file mode 100644 index 0000000..93b329f --- /dev/null +++ b/docs/known-issues.md @@ -0,0 +1,86 @@ +# Known issues, gaps, and the path toward a framework + +This doc consolidates everything found during a full-codebase review (2026-08-15) that isn't just "how it works" (that's `architecture.md`/`console-framework.md`/`rpc-protocol.md`) but "this is broken, incomplete, or a decision point." Re-read the relevant section before touching an area listed here. + +## Bugs (fix these regardless of the framework question) + +### ~~Blocking: hardcoded socket path breaks the package for everyone but one machine~~ — fixed 2026-08-15 + +`src/Rpc/JsonRpcClient.php` used to connect to a literal absolute path (`unix:///Users/mateuszcholewka/Projects/tapperphp/tapper.sock`) instead of computing it. Fixed by extracting the shared computation into `Tapper\SocketPath::resolve()` (`src/SocketPath.php`), used by both `Server.php` and `Rpc/JsonRpcClient.php`, so the two sides can no longer diverge. `tapper.sock` is now also in `.gitignore` (it's a runtime artifact, not something to commit). + +Verified with an isolated smoke test (no terminal required): booted `Server` directly against a real `AppState`/`EventBus`, confirmed the socket file appears at `SocketPath::resolve()`, and confirmed `tp()` from a separate process round-trips successfully (`result: ok`) instead of throwing `[Tapper] server not responding.`. + +Note: `SocketPath::resolve()` still resolves to "the installed package's own directory" (`realpath(__DIR__.'/..')`), same semantics as the original `Server.php` code — when Tapper is installed as a dependency, that's `vendor/tapperphp/tapper/tapper.sock`, not the consuming project's root. That's fine functionally (both processes resolve the same install, so they agree), but if this ever needs to be more conventional (e.g. `sys_get_temp_dir()`-based, to avoid touching `vendor/`), that's a deliberate follow-up, not part of this fix. + +### `wait()` listeners on `EventBus` are never cleaned up + +See `rpc-protocol.md`'s `wait` section — every `tp(...)->wait()` call adds a permanent `KeyCode::Enter` listener to `EventBus` that's never removed, even after it fires. Two overlapping `wait()` calls will both resolve on the same keypress instead of one at a time. `EventBus` has no unsubscribe mechanism at all today (see below). + +### `Application::run()` registers signal handlers after the blocking call that would need them + +```php +$this->loop->run(); // blocks until loop stops + +$this->loop->addSignal(SIGINT, function () { echo 'kill'; }); +$this->loop->addSignal(SIGTERM, function () { echo 'kill'; }); +``` + +Both `addSignal` calls are unreachable until `loop->run()` returns, at which point registering them is moot. Move them before `$this->loop->run()`. Also worth deciding what SIGINT/SIGTERM *should* do — right now the handler bodies just `echo 'kill'` rather than calling `$this->close()` to restore the terminal, so a Ctrl+C could leave the terminal in raw/alternate-screen mode. + +### `AppState` batching bug (documented in-code, unresolved) + +`src/Console/State/AppState.php`, directly above `__set`: + +```php +/* + * @TODO investigate why `changed` overflows + * when setting something multiple times, + * like pressing enter many times + * when waiting is set on tp + */ +``` + +Don't build new logic on `deffer()`/`commit()` batching until this is diagnosed — prefer unbatched (direct) `__set` calls if a new feature doesn't clearly need batching. + +### `JsonRpcRequest::payload()` references an undefined `$id` + +```php +'id' => $id ?? uniqid('rpc_', true), +``` + +`$id` is never assigned in scope, so this always evaluates to `uniqid('rpc_', true)` — it works, but only because of PHP's `??` suppressing the undefined-variable notice, not because there's an actual optional-id feature. Either add a real `?string $id = null` constructor parameter to `JsonRpcRequest` if per-request ids are wanted, or simplify to a direct `uniqid('rpc_', true)` call and drop the `??`. + +### `Server::$id` is `static` inside a container-managed singleton + +`Server` is constructor-injected with `AppState`/`EventBus` (i.e., it's a normal DI-managed instance), but its log-id counter is `private static $id = 0`. Harmless today (one `Server` instance ever exists), but inconsistent with the rest of the class's design and would break if `Server` were ever instantiated more than once (e.g. in a test). Make it an instance property. + +## Incomplete abstractions (finish or remove, don't extend as-is) + +- **`Rpc\JsonRpc` interface / `JsonRpcResult` / `JsonRpcError`** — `JsonRpcResult.php` and `JsonRpcError.php` are empty files; `JsonRpc`'s encode/parse methods exist only as commented-out code; `Server.php` builds raw arrays instead of using these types. See `rpc-protocol.md`. +- **`Commands/Command.php` + `CommandInvoker`** — an empty abstract class and an invoker with zero concrete `Command` subclasses anywhere in the codebase. Either commit to modeling user actions as `Command` objects (useful for undo/redo, macro recording, remapping keys later) or remove the layer until there's a real consumer. +- **`docs/openrpc.json`** — describes a method (`appendLog`, nested `details` param) that doesn't match the implemented protocol (`log`/`wait`, flat params). Regenerate from `Server.php` or delete. +- **`Windows/Popup.php`** — exists, is instantiated and checked (`Application::draw()` renders it when `isActive()`), but its `view()` just returns an empty `BlockWidget::default()` and nothing in the codebase ever calls `activate()` on it (confirmed by grep — zero hits). Dead scaffolding for a not-yet-built feature (likely a modal/dialog system) — fine to leave, but don't assume it's a working popup system if you go looking for one. +- **`AppState::typingMode` is only ever set to `false`**, never `true`, anywhere in the codebase (confirmed by grep). `Application::handleEventInTypingMode()` therefore always returns early — its entire body (redirecting character input, exiting on Esc) is currently unreachable dead code, presumably scaffolding for a not-yet-built text-input feature. +- **`EventBus` has no unsubscribe** — every `listen()` is permanent. This is fine for the fixed component tree Tapper has today (components are never torn down mid-run), but is a real gap the moment anything needs dynamic component lifecycles (the `wait()` bug above is a direct symptom of this gap). + +## Framework-extraction readiness + +Context: the long-term goal is a general PHP desktop-app framework with a swappable rendering backend (this TUI, later a GLFW-based GUI). Decision as of 2026-08-15: **keep building Tapper as an application; do not extract a framework package yet.** The reasoning, and what would change this: + +**Why not yet:** the framework/backend boundary doesn't exist as a real interface today — it only exists as intent. Concretely: + +1. **No renderer abstraction.** `Component::view()` returns a concrete `PhpTui\Tui\Widget\Widget` directly (see `console-framework.md`). Every component builds `php-tui` widgets (`BlockWidget`, `GridWidget`, `ParagraphWidget`, ...) inline. A GLFW backend can't consume this — there is no intermediate, backend-neutral description of "what to draw" that a `TuiRenderer` and a future `GlfwRenderer` could both adapt. +2. **No layout abstraction.** Components use `php-tui`'s `Area`/`Constraint`/`Direction`/`Layout` directly, which model discrete terminal cells (rows/columns), not the continuous pixel/DPI space a GUI layout needs. This isn't a find-and-replace; it's a real design task (something flex-like that both a cell-grid and a pixel-canvas renderer can implement). +3. **No input abstraction.** `EventBus`/`#[KeyPressed]`/`#[Mouse]` are typed against `php-tui/term`'s `KeyCode`/`MouseEventKind`. A GLFW backend has entirely different key/mouse event types that would need to be normalized to something backend-neutral before components could react to them uniformly. +4. **Only one real example so far.** Extracting general abstractions from a single application (Tapper) means guessing at what's actually shared vs. TUI-specific — the layout/rendering boundary in particular is easy to get wrong without a second concrete case pushing on it. + +**What would make extraction well-timed** (revisit this list before starting a framework package): + +- Tapper has grown enough real UI surface (a second distinct view/screen, a working `Popup`, resizable/responsive layout, something beyond lists+details) that the *TUI-specific* parts of `Component`/`AppState`/`EventBus` have had to flex more than once — giving more confidence about what's actually backend-agnostic. +- Either a second real consumer of the component model exists (even a small non-TUI proof of concept), or there's a concrete, non-hypothetical GLFW spike that stress-tests the rendering/layout/input boundary directly, rather than reasoning about it in the abstract. +- The bugs in this document that touch the framework-adjacent layers (`AppState` batching, `EventBus` unsubscribe) are resolved — extracting a framework on top of an unresolved reactivity bug just relocates the bug into the framework's foundation. + +**If/when extraction starts**, the natural package boundary based on the current code is: +- `core` (backend-agnostic): `Component` lifecycle/attribute-binding mechanics (minus the `php-tui`-typed bits), `EventBus` (generalized past `KeyCode`/`MouseEventKind`), `AppState`-style reactive state container, `CommandInvoker`. +- `adapter-tui`: everything that's `php-tui`-specific today — the widget-building code inside every `view()`, `Application`'s render/resize loop, the `php-tui/term` event types. +- `tapper` (the app): `Server.php`, `Runtime/`, `Rpc/`, and the concrete components (`Header`, `LogList`, `Details`, ...), as a consumer of `core` + `adapter-tui`. \ No newline at end of file diff --git a/docs/rpc-protocol.md b/docs/rpc-protocol.md new file mode 100644 index 0000000..f740be2 --- /dev/null +++ b/docs/rpc-protocol.md @@ -0,0 +1,87 @@ +# Wire protocol + +This documents what is **actually implemented** between a debuggee process (`Runtime\Tapper` / `Rpc\JsonRpcClient`) and the TUI process (`Server.php`). `docs/openrpc.json` exists in this repo but describes a different, unimplemented shape — see [Divergence from `openrpc.json`](#divergence-from-openrpcjson) below. Treat this document, not the OpenRPC file, as ground truth until someone reconciles them. + +## Transport + +- Unix domain socket, path `/tapper.sock` (computed by `Server.php` via `realpath(__DIR__.'/..')` from inside the installed package). +- One line of JSON per message, `\n`-terminated, decoded with `clue/ndjson-react` on the server side. +- The client (`Rpc/JsonRpcClient.php`) is a **blocking** `stream_socket_client()` — connect timeout 0.5s by default, then `stream_set_timeout()` + blocking `fgets()` for the reply, using a separate `pauseTimeout` (default 3600s) so a `wait()` call can block for up to an hour waiting for the user to press Enter in the TUI. +- TCP is not currently wired up despite `JsonRpcClient` having `$host`/`$port` constructor properties — those fields are dead; the actual connection target is a hardcoded string, see the bug below. + +## Socket path resolution + +Both sides resolve the socket path through one shared helper, `Tapper\SocketPath::resolve()` (`src/SocketPath.php`): + +```php +$projectPath = realpath(__DIR__.'/..'); // src/ -> package root +return $projectPath.'/tapper.sock'; +``` + +`Server.php` calls it to bind the listener; `Rpc/JsonRpcClient.php` calls it to connect. Because both sides call the same function instead of computing the path independently, they can't diverge. (This previously *did* diverge — `JsonRpcClient` had a hardcoded absolute path to one developer's machine, fixed 2026-08-15; see `known-issues.md` for the history if you're wondering why this indirection exists instead of a literal path.) + +## Request shape (client → server) + +Built by `Rpc\JsonRpcRequest::payload()`: + +```json +{ + "jsonrpc": "2.0", + "method": "log", + "params": { "...": "..." }, + "id": "rpc_..." +} +``` + +- `id` is generated by `uniqid('rpc_', true)` on every call — `JsonRpcRequest` has no way to accept an explicit id (the constructor doesn't take one; `payload()` references an undefined `$id` variable that null-coalesces to always calling `uniqid()` — works, but is dead/misleading code, not an intentional "reuse an id" feature). +- Two methods are implemented server-side: `log` and `wait`. + +### `log` params (sent by every plain `tp($value)`) + +| field | type | notes | +|---|---|---| +| `message` | any (JSON-encodable) | the raw value passed to `tp()` | +| `microtime` | float | `microtime(true)` at call time | +| `caller` | string | `"basename(file):line"` of the immediate caller | +| `trace` | array of `{file, line}` | backtrace, `DEBUG_BACKTRACE_IGNORE_ARGS`, first 2 frames skipped | +| `rootDir` | string\|null | resolved via Composer's `ClassLoader` file location | +| `code` | array of `{number, line, active}` | ±3 lines of source around the caller line | + +Server response: `{"jsonrpc":"2.0","result":"ok","id":}`. + +### `wait` params + +Same shape as `log`. Server behavior differs: it still calls `AppState::appendLog()` immediately (so the "paused" message shows up right away), but **does not reply immediately**. Instead it registers a listener on `EventBus` for `KeyCode::Enter` that writes the deferred reply the next time Enter is pressed anywhere in the TUI: + +```json +{"jsonrpc":"2.0","result":"continue","id":} +``` + +⚠️ **This listener is never removed.** If two `tp(...)->wait()` calls are in flight concurrently (e.g. two debuggee processes, or a loop that somehow overlaps), *both* accumulated `KeyCode::Enter` listeners fire on the next Enter press, and each writes to its own socket — so both would resolve on the same keypress rather than one-at-a-time. `EventBus` has no unsubscribe mechanism (see `console-framework.md`), so fixing this requires either adding one or tracking wait-listeners explicitly in `Server.php`. + +### Any other method + +```json +{"jsonrpc":"2.0","error":{"code":-32601,"message":"Method 'xyz' not found"},"id":} +``` + +### Malformed request (missing/wrong `jsonrpc` field) + +```json +{"jsonrpc":"2.0","error":{"code":-32600,"message":"Invalid Request"},"id":} +``` + +## Response value objects exist but are unused + +`Rpc\JsonRpcResult` and `Rpc\JsonRpcError` are both **empty files** (no class body at all). `Server.php` builds response arrays by hand with raw `$encoder->write([...])` calls instead of using typed value objects. The `Rpc\JsonRpc` interface declares one real method (`payload(): array`) and has four more (`encodeRequest`/`encodeResult`/`encodeError`/`parse`) present only as commented-out code. This layer was clearly meant to be more structured than it currently is — if you're touching `Rpc/`, either finish it (give `JsonRpcResult`/`JsonRpcError` real bodies and have `Server.php` use them) or remove the dead files/commented code so the interface reflects what's actually implemented. + +## Divergence from `openrpc.json` + +`docs/openrpc.json` documents a method called `appendLog` with params `message`, `microtime`, and a nested `details: {trace, rootDir, code}` object. The actual implementation (`Server.php`) has: + +- Method name `log`, not `appendLog`. +- A second method, `wait`, not documented in the OpenRPC file at all. +- Flat params (`caller`, `trace`, `rootDir`, `code` all siblings), not a nested `details` object. +- `caller` is not present in the OpenRPC schema at all. + +This file appears to have been written as a target/spec before (or independent of) the current implementation, and hasn't been kept in sync. Don't generate client/server code from it as-is — either regenerate it from the real `Server.php` switch statement, or delete it if it's not being used to drive anything. \ No newline at end of file diff --git a/src/Rpc/JsonRpcClient.php b/src/Rpc/JsonRpcClient.php index 1554805..a949697 100644 --- a/src/Rpc/JsonRpcClient.php +++ b/src/Rpc/JsonRpcClient.php @@ -4,6 +4,8 @@ namespace Tapper\Rpc; +use Tapper\SocketPath; + class JsonRpcClient { public function __construct( @@ -17,7 +19,7 @@ public function call(JsonRpc $jsonRpc): ?array { $payload = $jsonRpc->payload(); - $socket = @stream_socket_client("tcp://{$this->host}:{$this->port}", $errno, $errstr, $this->timeout); + $socket = @stream_socket_client('unix://'.SocketPath::resolve(), $errno, $errstr, $this->timeout); if (! $socket) { return null; diff --git a/src/Server.php b/src/Server.php index e9f8b9d..be83918 100644 --- a/src/Server.php +++ b/src/Server.php @@ -23,7 +23,9 @@ public function __construct( public function run(): void { - $server = new SocketServer('127.0.0.1:2137'); + $socketPath = SocketPath::resolve(); + @unlink($socketPath); + $server = new SocketServer('unix://'.$socketPath); $server->on('connection', function (\React\Socket\ConnectionInterface $conn) { $decoder = new Decoder($conn, true); diff --git a/src/SocketPath.php b/src/SocketPath.php new file mode 100644 index 0000000..234a801 --- /dev/null +++ b/src/SocketPath.php @@ -0,0 +1,21 @@ +