wine-access/docs/superpowers/plans/2026-10-11-wine-flat-review.md

Wine Flat Review Implementation Plan

For agentic workers: REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox syntax for tracking. Native execution is recommended for these closely connected interfaces.

Goal: Make Cthulhu's existing flat-review keys read exposed visible Wine text without moving the application's caret, selection, or focus.

Architecture: wine-access supplies owned window snapshots through an optional D-Bus capture endpoint. A Go provider registry and capturer composition feed those snapshots to the existing review worker and flat cursor. Event-loop foreground ownership and provider lifetime checks reject stale results.

Tech Stack: C++20 Winelib, Wine Unicode/Win32/MSAA APIs, existing GLib/GIO and XCB; Go 1.24+, existing godbus and AT-SPI snapshot/review code; Python/PyGObject and XTest private-display fixtures. Delivery testing required an explicit libxcb link/development dependency for actual Wine-associated X11 geometry; XCB was already installed on the tested system.

Spec: Approved Wine flat-review design.

Global Constraints

• Preserve existing uncommitted 32-bit Edit/menu changes and cthulhu.log.
• Keep desktop/laptop review bindings, speech/spelling/phonetics, braille queues, and silent movement boundaries.
• Never move native focus/caret/selection, scroll, copy to the clipboard, or inject application navigation keys to implement review.
• Password text is excluded before copying, serialization, presentation, and debug logging.
• Capture deadline: 500 ms total; one active native capture and one pending capture; command FIFO remains 16.
• Limits: 4096 nodes, depth 64, 256 ranges per control, 4096 code points per visual line, 65536 captured code points per control, 1 MiB text per window, 2 MiB serialized snapshot, one million UTF-16 units of full Edit source reads per window.
• Captions: 4096 UTF-8 bytes; identities: 256 UTF-8 bytes. Reject duplicate IDs, cycles, malformed UTF-8, offsets, bounds, and versions.
• Return complete snapshots or an explicit non-ok status; at most one stale recapture under the original deadline.
• Preserve PresentAccessibleEvent, NVDA/Tolk, optional-helper behavior, Linux review, and XTerm/Fenrir handoff.
• Core input/event loops never perform native capture or synchronous D-Bus capture calls.
• No native pointers/COM objects cross the bus. Keep each native object's release on its COM STA.
• Xorg correctness is required; private displays run sequentially. Distinguish recorded output, installed-binary checks, and live speech/braille acceptance.
• Exposed standard Edit/native/MSAA text is supported in both architectures. Custom painted text, RichEdit/Scintilla text providers, semantic object navigation, Say All, and native braille routing are outside this change.
• Use tool approval before writing reader source or installed files outside writable roots. Read applicable AGENTS.md before edits. Leave submission and human acceptance to the contributor.

Review Focus

1. A blank command field must anchor review of other visible text; test in Tasks 1 and 6.
2. Same-length text replacements during capture must not produce mixed old/new text; test in Tasks 1 and 2.
3. Two applications with identical titles must not share review ownership; test in Tasks 4 and 6.
4. Negative monitor coordinates and Wine virtual-desktop child focus must match current foreground geometry; test in Tasks 1, 6, and 7.
5. A stalled MSAA call must leave menus, input, and shutdown responsive; test in Tasks 2 and 7.

File ownership and interface map

Paths prefixed reader/ mean /home/deedra/git/cthulhu-go; other paths belong to this standalone helper repository. These describe source responsibilities, not new packages for every file.

• Helper review_capture.h/.cpp: owned capture records, offsets, visibility, native lifetime identity, and window/control capture.
• Helper review_service.h/.cpp: D-Bus serialization, authorization, registration, bounded STA capture worker, and cancellation/lifetime handling.
• Helper main.cpp: start/stop service, post focus/lifecycle notices; retain existing event/menu policy.
• Helper tests/review_fixture.c, tests/review_integration.py, tests/review_capture_test.cpp: native behavior and transport tests.
• Reader internal/wine/review_protocol.go, review_snapshot.go: wire types, limits, and conversion to owned trees.
• Reader internal/wine/review_registry.go, review_client.go: connection ownership, registered helper state, focus mailbox, capture calls, and provider disappearance.
• Reader internal/remote/wine_review.go: optional authorized registration/focus methods and introspection.
• Reader internal/app/wine_review_capture.go, wine_review_focus.go: capturer composition and foreground/focus/result policy respectively.
• Reader internal/review/flat_worker.go: optional fresh-capture policy; preserve the existing flat navigation algorithm.
• Reader runtime/event files: dependency wiring and event ordering only.

Task 1: Read-only native visible-window capture

Files: Create review_capture.h, review_capture.cpp, tests/review_fixture.c, tests/review_capture_test.cpp; modify CMakeLists.txt.

Interfaces produced:

• ReviewFocus { std::string instance, window, focus, title, role; uint64_t revision; bool protected_text; RECT bounds; }.
• ReviewRange { uint64_t start, end, utf16_start, utf16_end; std::string text; RECT bounds; }.
• ReviewNode { std::string id, parent, role, name; bool protected_text, visible, text_known, caret_known; uint64_t caret, caret_utf16; RECT bounds; std::vector<ReviewRange> ranges; }.
• ReviewCapture { ReviewFocus source; std::string status; std::vector<ReviewNode> nodes; }.
• class ReviewIdentityStore: ReviewFocus observe_foreground(), void destroyed(HWND, LONG object, LONG child), void reordered(HWND, LONG object), void retire(); synchronize lifetime registry access between native event and capture threads. Revisions change on focus/lifetime changes, not text changes.
• ReviewCapture capture_review_window(ReviewIdentityStore&, const ReviewFocus&, uint64_t deadline_tick); deadline uses GetTickCount64() and all source/size limits above.

• Write tests/review_capture_test.cpp and fixture cases named visible_unicode, wrapped_and_blank, empty_anchor, password, clipped_hidden, same_length_update, native_controls, source_limit, and negative_coordinates. Assert exact exposed text/rune offsets, unchanged focus/selection, and limited with zero nodes at limits. Include a surrogate before the first visible range so absolute rune offsets cannot be computed from visible text alone.
• Add CTest entries review-capture64 and review-capture32. Compile fixture binaries build/review-fixture64.exe and build/review-fixture32.exe with the existing MinGW compilers; review-capture-test.exe --fixture <path> launches and checks each fixture under its isolated prefix. Run ctest --test-dir build -R review-capture --output-on-failure on a private display before production capture exists; confirm the failure names the missing capability rather than fixture/setup failure.
• Implement identities and capture. Query native Edit text/positions, visible line/wrap ranges and caret without edits; traverse bounded MSAA/native controls, omit protected contents, strip caption mnemonics only, and preserve empty focus as a node. Map each absolute UTF-16 position through the bounded source read. Verify source identity and reread collected ranges before returning, including same-length updates.
• Build with cmake --build build --parallel; run the focused capture test on a private display with both fixture architectures. PASS means expected text and positions, no password token anywhere, and unchanged native state. Run git diff --check and review this task's diff.

Exact offset assertion for an offscreen prefix 😀alpha\n followed by visible beta:

assert(range.start == 7 && range.end == 11);
assert(range.utf16_start == 8 && range.utf16_end == 12);
assert(range.text == "beta");

Task 2: Optional bounded helper capture service

Files: Create review_service.h, review_service.cpp, tests/review_integration.py; modify main.cpp, CMakeLists.txt, tests/review_fixture.c.

Consumes: Task 1 records, identity store, and capture function. Produces: class ReviewService with bool start(GDBusConnection*, ReviewIdentityStore&), void focus_changed(const ReviewFocus&), and void stop(); exact version-1 methods/dictionary in the spec. No extra installed artifact.

• Write private-session tests test_capture_wire_contract, test_wrong_caller, test_old_reader, test_busy_capture, test_capture_deadline, test_same_length_capture_race, and test_stop_during_capture. Assert an unavailable optional reader does not stop ordinary helper speech; stale/busy/limited replies have zero nodes; a blocked capture does not delay existing menu/focus events; shutdown remains bounded.
• Run python3 tests/review_integration.py --build-dir build --32-bit on a private display and confirm missing endpoint/registration is the red failure.
• Implement native records-to-a{sv} conversion and exact dictionary field types. Register /org/stormux/Cthulhu1/WineReview on the existing unique connection; authorize the current same-user Cthulhu service owner. Own a dedicated GLib main context/dispatcher thread because the current helper's Win32 message loop does not dispatch a GLib main context. Dispatch native work to a separate COM STA; the dispatcher must reject excess work even when capture blocks. Reply once per invocation, expire pending work within the original 500 ms, and release native references on their STA. No forced thread termination.
• Wire start/stop and focus/destruction/reorder notices from the existing event loop. Poll recovers missed focus events without registering a new provider per notice. Retry registration when a compatible reader owner appears. Continue existing speech if methods are unavailable.
• Run both python3 tests/review_integration.py --build-dir build and its --32-bit variant sequentially; PASS verifies typed schema, authorization, native state, bounded busy behavior, and older-reader compatibility. Re-run existing menu/editor fixtures because startup and shutdown changed. Review the diff.

Task 3: Validate snapshots and build Go review trees

Files: Create reader/internal/wine/review_protocol.go, review_snapshot.go, review_snapshot_test.go.

Interfaces produced: ReviewFocusNotice mirrors the spec focus fields and includes actual sender/path copied from registration; ReviewSnapshot/ReviewNode/ReviewRange mirror wire records with uint64 offsets and atspi.Rectangle bounds. DecodeReviewSnapshot(fields map[string]dbus.Variant) (ReviewSnapshot, error) validates types/limits; ApplyReviewSnapshot(tree *atspi.AccessibleTree, snapshot ReviewSnapshot, sender string) (*atspi.AccessibleTree, atspi.ObjectID, error) creates or replaces a stable owned tree. IsReviewID(atspi.ObjectID) bool checks the exact reserved namespace. Define sentinel errors ErrReviewUnavailable, ErrReviewStale, ErrReviewBusy, ErrReviewLimited, and ErrReviewInvalid for status and validation handling.

• Write TestReviewSnapshotUnicodeAndStableTree, TestReviewSnapshotEmptyAnchor, TestReviewSnapshotProtectedContent, and table-driven TestReviewSnapshotRejectsInvalidPayload. Assert code-point counts, valid /wine/ identities, unchanged tree pointer/live generations after text updates, new generations after removal/reuse, and rejection of every spec bound plus cycles, duplicates, malformed nested types, offset overflow, and clipped-range violations.
• Run GOCACHE=/tmp/cthulhu-go-cache go test ./internal/wine -run ReviewSnapshot -count=1 from the reader checkout; confirm tests fail for the missing protocol/conversion.
• Implement exact dictionary decoding and conversion. Opaque IDs map to safe object-path components; root has window/dialog role; protected controls become password text with no ranges/value. Use existing tree validation/Replace rather than a parallel tree model. Never convert uint64 to int before range checking.
• Re-run the focused tests and go test -race ./internal/wine -count=1; PASS includes the existing manager tests. Review the diff.

The invalid-payload table must assert rejection rather than acceptance of partial data:

snapshot, err := DecodeReviewSnapshot(fields)
if err == nil || len(snapshot.Nodes) != 0 {
    t.Fatalf("invalid payload accepted: nodes=%d err=%v", len(snapshot.Nodes), err)
}

Task 4: Provider registration, lifetime, and bounded Go capture

Files: Create reader/internal/wine/review_registry.go, review_client.go, review_registry_test.go, review_client_test.go, reader/internal/remote/wine_review.go, wine_review_test.go; modify reader/internal/remote/service.go, introspection.go.

Consumes: Task 3 wire records, errors, and tree conversion. Produces: NewReviewRegistry(logger *slog.Logger) *ReviewRegistry; registry methods Start(context.Context) error, Stop(context.Context) error, RegisterProvider(sender string, path dbus.ObjectPath, version uint32) bool, PresentFocus(sender string, notice ReviewFocusNotice) bool, Changes() <-chan struct{}, Focuses() []ReviewFocusNotice, Capture(context.Context, atspi.ObjectID) (*atspi.AccessibleTree, error), and Current(atspi.ObjectID) bool. Changes is a capacity-one notification; registry retains latest state per sender, not one lossy notice for all prefixes. Remote optional WineReviewHandler has the RegisterProvider/PresentFocus signatures; expose spec method names through authorized wrappers using the actual dbus.Sender.

• Write TestReviewRegistryTwoProviders, TestReviewRegistrySenderAssociation, TestReviewRegistryProviderDisappears, TestReviewCaptureDeadlineAndStaleReply, and real private-bus TestNativeSessionBusWineReview. Assert unique connections/instances do not collide even for matching HWND/title; owner disappearance removes focus/tree; re-registration retires the old instance; focus notices from an unregistered or unauthorized sender are rejected; capture returns by 500 ms and Stop cancels work.
• Run go test ./internal/wine ./internal/remote -run 'ReviewRegistry|ReviewCapture|WineReview' -count=1; confirm the red result is the missing registry/endpoint behavior.
• Implement same-user service wrappers and introspection without changing existing Handler implementations. Registry owns a separate session-bus connection for captures/disconnection monitoring, unregisters its match rules on Stop, and targets only registered unique senders. Apply one original deadline, one stale retry, and exact expected identity/revision checks to capture; cache stable trees per provider/window lifetime, never cached text as a substitute for a successful current capture.
• Run unit/race checks, then dbus-run-session -- env CTHULHU_NATIVE_DBUS_TEST=1 GOCACHE=/tmp/cthulhu-go-cache go test -race ./internal/wine ./internal/remote -run NativeSessionBusWineReview -count=1. PASS proves actual wire registration, capture, and disappearance. Review the diff.

Task 5: Feed fresh Wine snapshots to the existing review worker

Files: Create reader/internal/app/wine_review_capture.go, wine_review_capture_test.go; modify reader/internal/review/worker_contract.go, flat_worker.go, flat_worker_test.go, and reader/internal/app/runtime_review.go.

Consumes: ReviewRegistry.Capture and IsReviewID. Produces: type review.FreshReviewWindowCapturer interface { FreshReviewWindow(atspi.ObjectID) bool }. App reviewCaptureRouter composes the existing review.TreeCapturer and registry, implements its four capture methods plus FreshReviewWindow. Wine review uses registry capture; document capture rejects Wine IDs; native IDs delegate exactly as before.

• Write TestFlatWorkerFreshProviderPreservesCursor, TestFlatWorkerFreshProviderFailureDoesNotSpeakCachedText, TestReviewCaptureRouterNativeDelegation, and TestReviewCaptureRouterWineIDsNeverReachATSPINativeCalls. Read alpha beta, move independently, replace the same live control's text, then assert the next command sees new text at the preserved offset. A native-provider spy must see no Wine object paths.
• Run go test ./internal/review ./internal/app -run 'FreshProvider|ReviewCaptureRouter' -count=1; confirm the missing fresh-provider policy is the failure.
• In readFlat, detect the optional policy and capture once per Wine command before cursor refresh. Keep capture on the review worker, apply the 500 ms Wine timeout, preserve the same tree through Replace, and prohibit repeated capture retries beyond the one allowed under the original deadline. Leave the existing AT-SPI stale-capture path and navigation algorithm intact.
• Map wine.ErrReviewLimited to review.ErrLimitedReview in the router so the established limit message is used. Inject the composed capturer through runtime review construction. Run focused tests and existing flat-worker/cursor tests with race checks; PASS includes all review units and queued spelling styles. Review the diff.

Task 6: Own Wine review focus and output in the event loop

Files: Create reader/internal/app/wine_review_focus.go, wine_review_focus_test.go; modify app.go, runtime_remote.go, live_settings.go, event_loop.go, event_review_results.go and the focused lifecycle/focus tests only as needed.

Consumes: registry focus snapshots/change notifications, Current, router from Task 5, existing foreground/title source and review worker. Produces: wineReviewFocusController with changed(), foregroundChanged(), retire(), owns(atspi.ObjectID) bool, and resultCurrent(review.Result) bool. Event-loop-owned active state stores provider, window/focus lifetime, native source revision, foreground epoch, and root bounds. Registry connection/capture state stays outside the event loop.

• Write TestWineReviewEmptyFocusCanReachOutput, TestWineReviewIdenticalTitlesDoNotClaimForeground, TestWineReviewForegroundAndVirtualDesktop, TestWineReviewDropsOldResultsAndRestarts, and TestWineReviewSpeechBrailleAndSpelling. Assert matching geometry/epoch ownership, negative coordinates, virtual desktop child identity, duplicate notices preserving position, provider restart/lock invalidation, and restored Linux focus. Check exact existing line/word/character/spelling requests and password-free braille output.
• Run go test ./internal/app -run WineReview -count=1; confirm missing Wine review ownership is the red failure.
• Wire one registry instance to review construction and remote registration. Add a lifecycle entry whose shutdown cancels registry work before its connection closes; reuse the existing reader supervisor. Event-loop callbacks reconcile registry notices with current Xorg foreground geometry/title-child metadata and worker focus. Ambiguous/unknown ownership rejects Wine review; raw titles alone cannot establish it.
• Before presenting flat results, synchronize current foreground and require matching provider/focus lifetime/revision/epoch. On a revision transition, call ClearFocus to retire the old worker document epoch before restoring any new focus, even when the same control identity returns; duplicate notices without a transition preserve position. Retire queued work on focus loss, lock, destruction, or provider death. Prevent native events for an earlier Linux window from overwriting current Wine review focus. Keep policy in the controller and event ordering in the loop.
• Run app/review/remote/wine race checks and existing Linux flat-review, foreground, titles, password, spelling, and braille tests. PASS proves shared behavior and output queues; it is not a live speech claim. Review the diff.

Task 7: Prove physical key routing through both architectures

Files: Create reader/tests/wine_review_x11.py, reader/internal/app/wine_review_native_test.go; extend helper tests/review_fixture.c/review_integration.py with deterministic handshake and stalled-capture modes.

Consumes: installed-shaped helper build, real Go service/command router, registry and capture paths. Use the existing private X11 fixture's physical XTest keycode helper, not modifier-changing keysym synthesis.

• Write a private-session/display acceptance fixture that fails unless keypad line/word/character, spelling/phonetic presses, window start/end, line end and vertical-word review produce expected text. Assert fixture focus, caret, selection and document content remain unchanged; ordinary application arrow keys still work afterward.
• Add focus-switch, negative-coordinate, virtual-desktop, two-prefix, reader/helper restart, and streaming-output phases. Stall capture deliberately and prove menu/focus speech still arrives, key release remains balanced, Linux review responds, cancellation joins within the normal shutdown budget, and no late Wine text is spoken after switching away.
• Run the new fixture before final wiring is considered complete; investigate any failure at the helper/registry/focus/command/output boundary instead of weakening assertions.
• Run both architectures sequentially, then the existing helper MSAA/menu/edit/list/control fixtures and focused reader native Xorg/Fenrir handoff checks. Record command, exit status, capture/input timing and final output evidence; PASS requires real X11 command consumption and correct queued speech/braille. Physical listening remains a separate user check.

Task 8: Review, document, build, and deliver matching binaries

Files: Update helper README.md, tests/README.md; reader docs/testing/wine-accessibility.md, MIGRATION_PLAN.md only for scope/evidence/gaps. Source changes remain in their owning repositories.

• Review both complete diffs against the spec, including unrelated-file preservation and password exclusion. Obtain one independent whole-change code review for native execution, or the selected task reviews if subagent execution was chosen; resolve verified findings and rerun affected tests.
• Run git diff --check in both repositories, build the helper and reader, and run only outstanding relevant checks after the last edits. Do not broaden testing again without a new failure/change/concern.
• Update docs with exact protocol/support limits and actual command evidence. Clearly distinguish private fixtures, full-router/native checks, installed-binary verification and remaining MUSHclient listening/braille checks. Remove the now-resolved Wine flat-review gap while preserving excluded providers/routing gaps.
• Prepare matching binaries and backups, then use tool approval to install outside writable roots. Replace mapped helper artifacts atomically. Reload the helper/reader through verified lifecycle paths while preserving MUSHclient and the current game connection. Verify the new processes stay alive and registration/capture uses the new protocol.
• Provide concise live checks: current/previous/next line in output notepad, word/character/spelling, independent caret, new incoming output, plugin dialogs, and return to Linux/Fenrir. Keep changes reviewable and leave contribution submission to the user.

Execution prerequisites and handoff

The user approved the spec, plan and native inline execution on 2026-10-11. Tasks 1–8 are complete. Independent whole-change review and its verified fix pass are complete. Matching binaries are installed and the actual desktop check passed with the current MUSHclient process and game socket preserved. The isolated execution ledger records exact checks, decisions and live gates.

At execution start, apply the chosen execution skill and its isolation rules. Inspect both repositories' working state; preserve a scoped baseline of the existing helper changes, and obtain predictable tool escalation for repository metadata or reader-source writes outside writable roots. Do not discard or automatically submit the current work. Track task completion in this document.