Repository navigation
fix(shell): Restart on a stopped app's face responds on the button, not at its top left - #296
Merged
Merged
Conversation
The supervisor saw the kernel end (its stdout closing, or its exit status) and read the stderr tail at once, but stderr is copied into the tail by a task of its own. The line the kernel wrote just before it exited could still be in the pipe, so the reason lost it: CI's `a_kernel_that_dies_closes_its_connections_with_its_last_words` failed with "the kernel closed its output: fake kernel up", without the "asked to exit" the fake kernel wrote last. The tail is now read after the kernel is stopped and the stderr task has reached the end of the pipe (bounded at 2 s, for a process the kernel started that still holds the pipe). Failed reasons get the same complete tail. The test asks for the last words themselves, not just "exit". Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… probes The glance tile tests register probe system apps (os.glancetile, os.glanceinput, os.glancequiet, os.glancesweep) in App Hub's system-app registry. That registry is process-wide, has no removal, and is the one `system_card_apps` lists, so a catalog test running after a glance test in the same process saw the probes as launcher rows: `bundled_apps_open_without_catalog_files_or_child_processes` failed its catalog comparison 7 of 10 times beside `glance_card::` on main, and `the_system_apps_ship_as_card_apps` compares the same list. Tests now register their system apps through `apps::test_system_apps::register`, which notes the id before it registers it, and the tests of the build's own catalog leave noted ids out. Production code is unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
`approvals::tests::the_relay_gets_decisions_made_before_it_was_installed` began with `init_memory()`, which replaces the shell's process-wide approvals with empty ones. Every consent a test running beside it had given was gone. `app_chat::tests::the_panel_opens_a_sharing_context_and_sends_person_turns_there` allows its app, then reads `agents::line()`, which reports a peer slug only while the app is allowed. Beside `approvals::` it failed 50 of 50 runs with `peer_slug: null`. The same test also installed its recording relay into the shared approvals, where it would take other tests' decisions. The relay seam (`approval_requested`, `set_relay`, `bus_requested`) is now methods on `Approvals`. The free functions call them on the shell's approvals, as before. The test builds `Approvals::memory()` and never touches the shared one. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…the whole connect's `a_jail_only_app_reads_its_jail_and_is_refused_everything_else` failed on GitHub's macOS 14 runners with "the hub is reachable: " and nothing after it: the sandboxed `nc -z` exited 1 without a word, because nc prints why a connect failed only with `-v`. Its exit status says only that the connect did not complete. A sandbox refusal and a refusal past the sandbox (no listener on the port then, the network stack) look the same, though only the first is what this test is about. Closing the hub's listener before the probe reproduces the CI message exactly. The probe now runs `nc -v -n` and the test reads what nc says. The hub fails only on the sandbox's own refusal (EPERM from Seatbelt and seccomp, EACCES from Landlock), or when nc never reported a connect at all. A refusal past the sandbox is printed and passes. Any other port must now be refused by the sandbox itself, not just fail, which the old check could not tell apart. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
octos#2675 makes session/hydrate rows name their tool calls: a tool row carries tool_call_id and tool_name, an assistant row its tool_calls. The shell's system chat and "Ask <app>" panel already read tool_call_id and name/tool_name, and the broker's merged history passes rows through, so reloaded tool rows now show their tool's name instead of "tool". The two-lane scenario checks it against the real kernel: News's agent's news_share call in the panel's reloaded history, and the system chat's peer_send_input. The pin also brings #2261, #2664, #2665, #2669, #2671 and #2672. octos-core, octos-cli, octos-research and the kernel artifact move together; Cargo.lock changes only the 17 octos source lines. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…w behind it A module tile claimed presses but never a hover. A mouse move over a part of a card window that takes no hover (Mail's empty page) went on to the tile behind unclaimed, and the app there took it: a News button under Mail lit up and showed its hand cursor. A tile behind also handed its app every move, claimed or not, as it handed presses before #277. The live tile now treats a hover (a mouse move with no button held) the way #277 treats a press. A hover inside it that none of its app's widgets took is the tile's, and one a window in front already took is let by, so its app never sees it. The one exception is the move that leaves a widget of the app still holding the hover: it reaches the root, so that widget sees its hover end, and being claimed it lets no other hover start. The closed face of a stopped app claims a hover inside it too. Moves with a button held (drags) pass as before, so captures keep working. Tests, in module_input_tests.rs: a hover on the window in front never reaches the app behind; makepad's Button behind it never lights up or shows its hand; a hover on a stopped face never reaches the app behind it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ot at its top left The closed face draws its line and Restart button centred, but it kept the rect `walk_turtle` returned for the button as Restart's hit rect. The face's turtle centres its content when it ends, which moves the drawn button after it was walked, so the kept rect stayed at the face's top left. A press on the visible button only raised the window, and a press at the top left, where nothing is drawn, restarted the app. The face now hit-tests Restart by the button's drawn area, as any widget is hit-tested: clipped, at the time of the press. It keeps no rect of its own. Tests, in module_input_tests.rs: Restart responds where it is drawn (a click and a tap on the visible button restart the app, a press at the top left only raises the window), and #277's stopped-face test now presses the drawn Restart. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
ymote
force-pushed
the
fix/stopped-face-restart
branch
from
October 2, 2026 20:59
1d30ae2 to
713db98
Compare
This was referenced Oct 2, 2026
Merged
Contributor
Author
|
Merged in a merge train; local CI (Mac with rom toolchain + --linux-host) passed on the train head 72e5d55, which contains this PR.
The train was #291, #293, #297, #294, #298, #295, #296, #299, #300 and #292, on main f6678b5. After the last merge, main 301be0c has the tested head's tree. |
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.
Follow-up to #277. Its "Found on the way" noted that the closed face hit-tests Restart in the wrong place.
Cause
When an in-process app's module panics, its tile shows a closed face: a line and a Restart button, centred (
draw_stoppedincrates/shell/src/module_view.rs).cx.walk_turtle(Walk::fixed(120.0, 36.0))and kept the returned rect as Restart's hit rect (restart_rect).align: 0.5, 0.5) centres its content only when it ends. That moves the drawn button and its label after the walk, but not the kept rect. The kept rect stays where the button would sit without the centring: at the face's top left, under the line.restart_rectis at (100, 134.8), while the button is drawn at (240, 349.4).Fix
The face now hit-tests Restart by the button's drawn area (
on_restart). It usesdraw_button.area(), clipped, at the time of the press, as any makepad widget is hit-tested. It keeps no rect of its own:restart_rectis gone. The test accessorrestart_buttonreturns the drawn rect as well.Tests
Unit tests in
crates/shell/src/module_input_tests.rs, which need no kernel and no display:the_restart_on_a_stopped_face_responds_where_it_is_drawn. The drawn Restart is centred across the face, just under its middle. A click and a tap on it restart the app. A click and a tap at the face's top left only raise the window.a_stopped_tile_takes_only_a_press_nothing_in_front_tooknow presses the drawn Restart (restart_button(&cx)). Before, it pressed the kept rect, so it passed with or without the bug.Everything below ran on the Linux build host.
#277's head plus only the tests (no fix;
restart_buttonreturns the drawn button): both tests fail at the bug.[Raise(1)]where[Restart(1)]is expected.[Raise(1)]where[Restart(1)]is expected.With the fix, at 1d30ae2 on main cf526f4 (the merge of #277): the checks ran on 93bc4524, the same change on #277's rebased head d030129. The two commits have the same tree, as main and d030129 do.
phone/:cargo test --locked --features mobile-apps -p octosense-shell module_inputgives 5 passed, and themodule_view::tests 2 passed.phone/:cargo test --locked --features mobile-apps -p octosense-shellgives 794 passed, 0 failed on a rerun.apps::tests::bundled_apps_open_without_catalog_files_or_child_processes. That is a race already on main, not touched here: glance_card's tests register test system apps process-wide (register_test_app), and the catalog test then lists them.-- glance_card apps::tests::bundled_apps_open…).desktop/:cargo check --locked -p octosense, the same with--features mobile-apps, andbash ../tools/check-shell-graph.sh -p octosense.python3 tools/setup.py --check --cargo.module_inputandmodule_view::tests pass, 11 of them.Instrument, before and after
The product has no way to make a card app's module fail on purpose, so both binaries below carry the same scratch hook. The hook is not part of this PR.
/event?data=instrument-faultmakes the focused tile's root panic inside its contained event handling. That is the path a real panic in an app's event takes:contain, thenreport_fault, then the shell drains the fault and shows the closed face.The table is the run on main's tree (#277's rebased head d030129) and on 93bc4524. An earlier run on #277's original head 6524358 gave the same results.
Setup:
weston --backend=headless --renderer=pixman --idle-time=0with makepad's Wayland backend.--remote. Frames come fromMAKEPAD_WRITE_FRAMEBUFFER_PNG.wm: restarting news after its instance (client 1) failedNot run: a module panic that the product itself causes. The closed face was reached only through the scratch hook.
Found on the way, not changed here
ModuleHost::take_faultslabels a fault withinstance.module.label(), which is "Card app" for every script app.🤖 Generated with Claude Code