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.
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: — fixed 2026-08-16: SocketPath::resolve() still resolves to "the installed package's own directory"SocketPath::resolve() and LogPath::resolve() now resolve to sys_get_temp_dir().'/tapper.sock' / '/tapper.log' instead of realpath(__DIR__.'/..'). This was flagged as a deliberate follow-up above, and became a real requirement once native packaging entered the picture — a static-php-cli/phpmicro build (docs/architecture.md roadmap) embeds src/ inside a read-only Phar, so a path computed from __DIR__ would point at a virtual filesystem that can't hold a socket file or an appended-to log file. sys_get_temp_dir() is always a real, writable directory regardless of how the app is packaged. Trade-off: this drops the previous per-install isolation (two different projects each requiring tapperphp/tapper from their own vendor/ used to get distinct socket paths for free, since __DIR__ differed per install) — now every local Tapper session shares <tmp>/tapper.sock, so only one debuggee/TUI pair should run at a time per machine. Revisit if that turns out to matter in practice (e.g. namespace the filename by project directory hash).
Previously every tp(...)->wait() call added a permanent KeyCode::Enter listener to EventBus, never removed, so overlapping wait() calls all resolved on the same keypress instead of one at a time. Fixed in Server.php: wait resolvers are now pushed onto a FIFO queue ($waitResolvers), and a single KeyCode::Enter listener is registered once (registerWaitListener(), guarded by $waitListenerRegistered) that pops and resolves the oldest pending wait per keypress. No per-call listener is added anymore, so there's nothing to leak, and N overlapping waits now require N separate Enter presses, in order. EventBus itself still has no general unsubscribe mechanism — see below, this remains a real gap for any future feature with dynamic listener lifecycles.
Application::run() registers signal handlers after the blocking call that would need them — fixed 2026-08-15
Application::run() registers signal handlers after the blocking call that would need themaddSignal(SIGINT, ...)/addSignal(SIGTERM, ...) are now registered before $this->loop->run(), so they're actually reachable, and their bodies call $this->close() (restores raw mode, mouse capture, cursor, alternate screen) instead of echo 'kill'. This alone does not make Ctrl+C work, though — see below.
Root cause: Terminal::enableRawMode() shells out to stty raw (vendor/php-tui/term/src/RawMode/SttyRawMode.php), which disables ISIG at the tty driver level. With ISIG off, Ctrl+C is never translated into a SIGINT signal in the first place — it arrives as a normal byte, \x03 (ETX), on stdin, same as any other keypress. So $this->loop->addSignal(SIGINT, ...) was structurally unreachable for the Ctrl+C case specifically, regardless of registration order (it still matters for external kill -INT/SIGTERM, which aren't tty-mediated). Fixed in Application::startInputHandling() by treating "\x03" the same as the existing 'q' raw-byte check — both now call $this->close().
$changed was a list, so __set-ing the same field repeatedly while batched (e.g. pressing Enter many times while tp()->wait() is paused) appended a duplicate entry per write, growing unbounded and replaying that field's observers once per duplicate on commit(). Fixed by keying $changed on field name ($this->changed[$name] = true) so it behaves as a set; commit() iterates array_keys($this->changed). Same fix applied to appendLog()'s batched branch ('logs' key).
Added a real private ?string $id = null constructor parameter; payload() now reads $this->id ?? uniqid('rpc_', true) instead of an undefined $id that only worked by accident via ??'s notice suppression.
Changed to private int $id = 0 (instance property), consistent with the rest of the class's DI-managed design.
Reported case: pressing Space in LogList with zero log entries hit LogList::select() reading $this->appState->logs()[$this->appState->cursor] with an empty logs array — an undefined-array-key warning, repeated once per render tick, each one printed raw over the alternate-screen buffer. Two fixes:
- Root cause:
LogList::select()now reads via?? nulland no-ops when there's no log at the cursor, instead of indexing blind. - General case:
Console\ErrorHandler::install()(called first thing inApplication::run(), before raw mode/alt-screen are enabled) installs aset_error_handlerthat logs any warning/notice/deprecation totapper.log(LogPath::resolve(), same directory astapper.sock) and sets a 5-secondAppState::errorNoticebanner (rendered byHeader) instead of letting PHP print to stdout. UncaughtThrowables still propagate normally —bin/tapper'scatch (\Throwable $e)(widened from\Exception) callsApplication::close()to restore the terminal, logs viaErrorHandler::logThrowable(), then rethrows so the trace prints on a normal, restored terminal rather than corrupting the TUI.
Note: this intentionally does not route the "check the logs" notice through Windows/Popup.php (a full-screen modal isn't the right shape for a transient 5-second notice); the banner lives in Header via two new AppState fields (errorNotice, errorNoticeExpiresAt) instead.
Blocking: typed class constants (PHP 8.3+ syntax) broke every PHP 8.2 install despite README/CI claiming 8.2 support — fixed 2026-08-16
private const string FOO = '...';-style typed class constants are a PHP 8.3 feature — on PHP 8.2 the parser throws ParseError: syntax error, unexpected identifier "FOO", expecting "=" on the file, at include time, regardless of whether the constant is ever read. This pattern was scattered across most of Console/* (Application, Component, ErrorHandler, Palette, MessageFormatter, PhpHighlighter, Components/{Splash,Details,LogItem}, Windows/Main) — i.e. it would have broken bin/tapper for any PHP 8.2 user the moment a log rendered (MessageFormatter/Palette are on the hot render path for every list row).
It went undetected by CI (which does run PHP 8.2 in the matrix) purely because the old, minimal test suite (Scroll, ScrollbarRenderer, SpanTruncator) never autoloaded any of the affected classes — PHP never parsed the offending files under 8.2, so there was nothing to fail on. It surfaced the moment new tests started exercising MessageFormatter/PhpHighlighter directly. Fixed by dropping the type annotation from every class constant repo-wide (behavior is identical, const FOO = '...' works on 8.2–8.4); composer.json also had no "php" constraint at all, so composer required installs wouldn't have been warned either — added "php": "^8.2" to require to match what the README already promises.
This is a strong argument for the still-open Console/Components/Server/Application test-coverage gap noted elsewhere in this doc: coverage that's too thin doesn't just miss logic bugs, it can hide the codebase not even parsing on a supported PHP version.
Rpc\JsonRpcinterface /JsonRpcResult/JsonRpcError—JsonRpcResult.phpandJsonRpcError.phpare empty files;JsonRpc's encode/parse methods exist only as commented-out code;Server.phpbuilds raw arrays instead of using these types. Seerpc-protocol.md.Commands/Command.php+CommandInvoker— an empty abstract class and an invoker with zero concreteCommandsubclasses anywhere in the codebase. Either commit to modeling user actions asCommandobjects (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, nesteddetailsparam) that doesn't match the implemented protocol (log/wait, flat params). Regenerate fromServer.phpor delete.— corrected 2026-08-16: this entry was stale.Windows/Popup.phpPopupis a working shortcuts-help modal (toggled with?, closed withEsc, realview()content) — not dead scaffolding.Application::draw()renders it based onAppState::popupOpen, notisActive();Popup's own bindings areglobal: trueso itsisActiveis never touched at all, by design.— resolved 2026-08-16: it's now the basis for log filtering.AppState::typingModeis only ever set tofalseLogList::startFilter()(bound to/) sets ittrue;Console\Components\Filter(a standalone overlay owned byApplication, mirroring howPopupis owned rather than being aMainchild) usesApplication::handleEventInTypingMode()'s'input'event to buildAppState::filter, live-filtered viaAppState::filteredLogs()(case-insensitive substring match onLogItem::$message). Enter confirms and exits typing mode keeping the filter; Esc cancels and clears it. SeeFilter.phpfor why its key bindings are deliberately non-global. One related fix required to make this safe:handleEventInTypingMode()used to pass the raw stdin chunk as the'input'payload for every drained event — since one stdin read can drain several events (fast typing/paste), they'd all have shared and duplicated the same bytes. It now emits the event's ownCharKeyEvent::$charinstead. Also:Application::startInputHandling()'s'q'-quits-the-app check now only fires outside typing mode, sinceqis a normal character you'd type into the filter box.EventBushas no unsubscribe — everylisten()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. (Thewait()leak that used to be a direct symptom of this gap is fixed above by havingServerregister one permanent listener instead of one per call — that sidesteps the gap for this one case, it doesn't close it.)
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:
- No renderer abstraction.
Component::view()returns a concretePhpTui\Tui\Widget\Widgetdirectly (seeconsole-framework.md). Every component buildsphp-tuiwidgets (BlockWidget,GridWidget,ParagraphWidget, ...) inline. A GLFW backend can't consume this — there is no intermediate, backend-neutral description of "what to draw" that aTuiRendererand a futureGlfwRenderercould both adapt. - No layout abstraction. Components use
php-tui'sArea/Constraint/Direction/Layoutdirectly, 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). - No input abstraction.
EventBus/#[KeyPressed]/#[Mouse]are typed againstphp-tui/term'sKeyCode/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. - 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 ofComponent/AppState/EventBushave 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 (
AppStatebatching,EventBusunsubscribe) 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):Componentlifecycle/attribute-binding mechanics (minus thephp-tui-typed bits),EventBus(generalized pastKeyCode/MouseEventKind),AppState-style reactive state container,CommandInvoker.adapter-tui: everything that'sphp-tui-specific today — the widget-building code inside everyview(),Application's render/resize loop, thephp-tui/termevent types.tapper(the app):Server.php,Runtime/,Rpc/, and the concrete components (Header,LogList,Details, ...), as a consumer ofcore+adapter-tui.