Skip to content

ci: require live GTK host smoke checks - #128

Merged
bvolpato merged 8 commits into
am-will:mainfrom
bvolpato:bvolpato/ci-gtk-host-smoke
Aug 1, 2026
Merged

ci: require live GTK host smoke checks#128
bvolpato merged 8 commits into
am-will:mainfrom
bvolpato:bvolpato/ci-gtk-host-smoke

Conversation

@bvolpato

@bvolpato bvolpato commented Aug 1, 2026

Copy link
Copy Markdown
Collaborator

Summary

Make real GTK host behavior a blocking pull-request check instead of relying only on unit tests and socket startup.

Current Xvfb harness can open control socket while Ghostty surface creation has already failed. It also contains stale assumptions that agent-team creates one workspace per peer, so several later checks cannot exercise intended paths.

This PR moves live smoke to headless Weston and requires terminal surfaces to become realized with nonzero grid dimensions before continuing.

Changes

  • add required GTK host smoke CI job on Ubuntu 24.04
  • run real host under headless Weston with Mesa software rendering
  • verify terminal health, command submission, read-screen, exact-surface routing, pane creation, environment propagation, hook translation, session persistence, restart, and process cleanup
  • parse CLI JSON structurally with jq instead of sed or text greps
  • keep CLI help as raw multiline text so command additions produce focused diffs
  • isolate test from inherited LIMUX_* pane and workspace IDs
  • force GLAD archive symbols into host binaries and test executables
  • submit new-pane --command through text plus Enter instead of pasted newline
  • translate injected shifted keys through active GDK keymap and verify A! live

This branch carries two reviewed commits from #114 because blocking smoke reproduces that baseline defect: send-key Enter returns success on current main but leaves command at prompt. Authorship is preserved. Once this PR lands, #114 is superseded by same fix plus live regression coverage.

Repro

Before fixes:

cd ghostty
zig build -Dapp-runtime=none -Doptimize=ReleaseFast -Dcpu=baseline
cd ..
./scripts/check.sh

Host test link fails locally with unresolved gladLoaderLoadGLContext and gladLoaderUnloadGLContext.

Old Xvfb smoke reports socket ready while surface-health shows:

{"healthy":false,"realized":false,"columns":0,"rows":0,"width_px":0,"height_px":0}

After switching to Weston, new blocking assertions exposed and fixed dropped Enter events, nonexecuting pane commands, inherited environment contamination, and stale peer-workspace assumptions.

Testing

./scripts/check.sh
LIMUX_SMOKE_PROFILE=debug ./scripts/xvfb-smoke-test.sh
shellcheck scripts/xvfb-smoke-test.sh
bash -n scripts/xvfb-smoke-test.sh
git diff --check upstream/main...HEAD

Results:

  • 295 Rust tests passed
  • release-version checks passed
  • 14 package SVG loader checks passed
  • live GTK/Ghostty smoke passed repeatedly, including restart
  • no ignored or filtered tests

Did this cause any problems?

Revert this PR. Existing unit-test workflow remains independent from GTK smoke job.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8b21fd46fa

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread .github/workflows/rust-quality.yml
tairea and others added 7 commits August 1, 2026 11:27
Two defects made limux undrivable from the control socket. Both are fixed
here, and together they close the four-verb loop (send / send-key /
read-screen / identify) that agent orchestration depends on.

1. send-key never reached the PTY.

   Terminal::send_key synthesized a ghostty key event with `keycode: 0`.
   Ghostty resolves the *physical* key from the hardware keycode and does
   not fall back to the keyval, so every injected key arrived as an
   unidentified key and was dropped. The caller still got `OK`, so the
   failure was silent: `limux send "echo hi"` followed by
   `limux send-key enter` left the text sitting at the prompt forever.
   No command sent over the socket could ever execute.

   Map the keyval back to a real keycode through the display's keymap
   (the inverse of the existing keyval_unicode_unshifted) so socket keys
   are encoded exactly like keys typed by a human.

2. Panes in a background workspace never realized.

   A pane only realizes its ghostty surface once its workspace is
   displayed. Splits made in a workspace that isn't selected stay
   unrealized: surface-health reports healthy=false and send_key returns
   false, which the bridge reports as the misleading "unsupported key".
   The host has always implemented `workspace.select` — it simply was not
   reachable from the CLI, so a CLI-driven fleet could not bring its own
   workspace forward and could never start.

   Expose it as `limux select-workspace --workspace <id|ref>`.

Verified end to end on Wayland: create a workspace, select it, split a
2x2, launch an interactive REPL in one pane, prompt it over the socket and
read the answer back off the screen.
Addresses @bvolpato's review. He is right, and the fix was incomplete.

A socket-injected key needs BOTH halves of the ghostty key event, and supplying
only one drops it silently:

  keycode only  ->  `limux send-key a` returns OK and writes nothing
  text only     ->  Enter, arrows, F-keys and ctrl-chords cannot be encoded

The original commit fixed the first half (keycode 0 meant ghostty saw an
unidentified physical key and dropped the event), which made Enter and arrows
work and closed the four-verb loop. But `translate_key_event` still left `text`
null, so ordinary printable input had nothing to write and vanished exactly as
before. It went unnoticed because orchestration uses `send` for text and
`send-key` only for Enter.

The codebase already says so, 550 lines above the bug, on the GTK key controller:

    // Send key events with the text field populated. Ghostty uses the
    // text field for actual character input and the keycode for bindings.

So: mirror the GTK path. Populate `press.text` from the existing `key_event_text`
helper, keeping the CString alive across `ghostty_surface_key`; leave the release
event textless, as the GTK controller does. The GTK path sources this text from
the input method -- a socket-injected key has no IM behind it, so derive it the
same way GTK's own fallback does.

`key_event_text` returns None for control characters, and that is load-bearing
rather than incidental: Enter and ctrl-chords must be *encoded* by ghostty from
key + mods, not written as literal bytes. Writing "\r" as text instead of letting
ghostty encode Return is how you break the kitty keyboard protocol.

This makes am-will#112 and this PR complementary rather than alternatives -- am-will#112 supplies
the text half, this supplies the physical-key half, and both are required.

Verified live on Wayland, all four classes at once, because fixing one by breaking
another is the obvious failure mode here:

  printable   `send-key e,c,h,o,space,h,i`  -> `echo hi` appears at the prompt
  Enter       `send-key enter`              -> it executes (once)
  arrows      `send-key up` then enter      -> history recalled; it executes twice
  ctrl-chord  `send-key '<ctrl>c'`          -> ^C interrupts a running `sleep 60`

Regression test pins the contract for all three key classes.
@bvolpato
bvolpato force-pushed the bvolpato/ci-gtk-host-smoke branch from bfdf5d7 to 3cbf24e Compare August 1, 2026 15:30
@bvolpato
bvolpato merged commit 38f70ff into am-will:main Aug 1, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants