fix!: ui_set_text replaces by default; append becomes opt-in - #9
Open
timrichardson wants to merge 4 commits into
Open
timrichardson wants to merge 4 commits into
timrichardson wants to merge 4 commits into
Conversation
… had nothing to talk to A private dbus-daemon cannot activate at-spi2-registryd: the a11y broker routes the start through org.freedesktop.systemd1, and on a bus with no systemd that name is the stub that answers /bin/false. So the headless a11y bus existed but no registry ever owned org.a11y.atspi.Registry, and ui_tree/ui_find failed with 'no application named gnome-shell' while the user's real session, whose bus has real systemd behind it, worked. headless.py now starts registryd itself, BEFORE the shell: order matters, because the shell's atk-bridge stops retrying registration when no registry answers (measured: registry-after-shell = 16/18 self-test, registry-before-shell = 18/18). Unit tests for the spawn/liveness/stop paths run anywhere; the live proof is tests/test_headless_atspi.py, which fails 2/3 checks on main and passes 3/3 here, and a cold headless self-test at 18/18.
…ix for every finding setup --check proves the machine before anything exists; the doctor proves the RUNNING stack after install and registration, and every finding prints its action. Born from tristanmuzzu#3, where every static check was green and even the self-test said 16/18 while the a11y registry quietly did not exist: a doctor has to ask the live system (does anything own org.a11y.atspi.Registry on this session's a11y bus? does the registered command answer MCP on stdio?), not the package manager. Checks: session, deps (setup's probes, same install lines), the a11y flag, the registry, the extension (files/enabled/live), the server command (version + provenance + a real initialize/tools/list handshake), and each running headless session's registry. --self-test adds the headless self-test (private virtual monitor, safe unattended). Never changes anything, never sudos; exit 0 healthy / 1 action needed / 2 not a target. 11 in-process tests (seams only, subprocess booby-trapped); verified live on Ubuntu 26.04: correctly demands the one logout this machine still owes, spots a pre-fix headless session and prints the restart command.
…nment MCP hosts that spawn stdio servers with a sanitized environment (Hermes passes only PATH/HOME/USER/LANG/XDG_*; others do the same to avoid leaking credentials) strip DBUS_SESSION_BUS_ADDRESS, so the guard in shell._gdbus refused every extension and window call with extension_unavailable -- on the user's own, perfectly working desktop. The env var is an address hint, not the mechanism: since the systemd user bus, GLib, gdbus and AT-SPI all connect to $XDG_RUNTIME_DIR/bus when it is absent, and XDG_RUNTIME_DIR survives every sanitized baseline. Resolve the reachable bus once (explicit address, else the runtime-dir socket), export it so subprocesses agree, and refuse only when neither exists -- the bare ssh login / system service case the guard exists for. Also normalize execution.session_key() through the resolver: hashing the raw env var handed two servers on ONE desktop different lease keys (fallback-connected vs explicit-connected). desktop_health now reports the resolved address instead of crying MISSING while everything works.
Issue tristanmuzzu#2 chose option 1 ('set means set'), but what landed was option 2's machinery with option 2's default: replace=False, so the tool still appends unless a caller knows to pass replace=True. The name, the README ('Preferred text entry') and the tool description all say set; a caller writing a field twice, or retrying after a failure, silently doubles the value. - Default flips to replace=True. replace=False keeps append for the demo/ progressive-writing case the issue was found building. - The append path's verify tightens from 'text in after' (a substring check that passes on a doubled value) to equality with before+text, symmetric with the replace path's existing check. A retried append that corrupts the widget now FAILS instead of reporting verified:True. - Schema default and description updated to say which behaviour is which. Breaking only for callers relying on the accidental append default.
This was referenced Sep 15, 2026
timrichardson
marked this pull request as ready for review
September 15, 2026 10:39
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #2
The gap
#2 picked option 1 ("set means set"), but what landed was option 2's
replaceargument with option 2's default —False. The original repro still reproduces onmain:What this changes
replacedefaults totrue— the tool now does what its name, the README ("Preferred text entry"), and its description say. The issue's exact repro now yields"TWO".replace: falsekeeps append for progressive writing (the demo that found the bug).text in after— a substring check that passes on a doubled value — to equality withbefore + text, symmetric with the replace path's check. The silent-corruption case from the issue (retry doubles the value, read-back "passes") now raisesatspi_write_failedinstead of returningverified: true.defaultand description updated to state which behaviour is which.Verification
tests/test_document_regressions.py: the issue's repro (fails onmain), opt-in append (passes both ways by design), and the doubled-append verify failure (fails onmain).deskwright/while keeping the tests → 2/3 new tests fail.ruff check .clean.Stacking
Stacked on #8 (
fix/session-bus-fallback); merge order #4 → #5 → #8 → this. Base staysmainper repo convention; until the lower PRs merge the diff includes their commits.Compatibility
Breaking only for callers relying on the accidental append default; those pass
replace: falseand get the old behaviour verbatim.