Repository navigation
Background tasks core: supervised PTY tasks, detection, preferences, bounded output (#382) - #383
Closed
raiseCatError wants to merge 5 commits into
Closed
raiseCatError wants to merge 5 commits into
raiseCatError wants to merge 5 commits into
Conversation
…preferences and bounded output #382, standalone modules; the running app is unchanged until integration. - supervisor.ts: each task is an NMSh-owned process in its own PTY, session and process group, started as `<shell> -c <command>` with the command as one argument (never re-quoted or parsed). Stop is SIGINT to the task's group (what Ctrl+C does), with SIGTERM and SIGKILL as explicit escalations; states are stopping until the exit is observed, then stopped/exited/failed. NMSh's shell, other tasks and detached helpers (an adb server) are never signalled. Group members left after the leader exits are reported and killed only on request. Remove is for finished tasks only. The environment is used for the spawn and never kept. - detector.ts: recognizes invocations whose words say they keep running (adb logcat but not adb logcat -d or adb devices, tail -f but not tail, npm run dev as "likely", npm test not), plain pipelines only; never substitutions, redirections, lists, sudo/ssh or full-screen programs. - preferences.ts: per-family Always/Never/Ask, no global always-background mode, retention and notification settings, stored privately without commands or output; decide() backgrounds only on an explicit Always. - output.ts: per-line sanitization with stripTerminalControls, progress lines keep their last state, PTY "\r\r\n" handled, bounded by lines and bytes with dropped and received counts. docs/design/background-tasks.md records the architecture choice, what a task inherits and does not, lifecycle under the session service, the protocol and frontend contract, a security review and physical QA.
Linux CI showed it: Ubuntu's /bin/sh (dash) does not exec the command, so SIGINT ended the wrapper shell while a program that ignores SIGINT kept running, and the task was reported stopped with processes left. After a stop request the task now stays `stopping`, and escalation still reaches its group, until the whole group is gone. The tests accept either exit form of the leader (exit 130 from the program itself, or signal 2 from a wrapper shell) and reproduce the dash case on macOS with a command the shell cannot exec.
…so the kill path is exercised where /bin/sh is dash On Ubuntu, SIGTERM ends the dash wrapper; the kernel then hangs up the task's group and the program, which did not ignore SIGHUP, ended there. That stop was real and reported truthfully; the test now keeps the program alive through the hangup so SIGKILL escalation is what ends it on every platform.
17 tasks
…gup, as a program detached from it would Linux CI: once the dash wrapper exits, the terminal is hung up and the fixture's next write failed with EIO, ending it through an unhandled stream error before SIGKILL was sent. The fixture now ignores write errors along with SIGHUP, and the assertion reports the task and its last lines if it ever differs again.
10 tasks
…til it is gone, never removed while it runs - A run of carriage returns made the trailing-\r regular expression quadratic: 128 Ki of them took 3.7 s and froze the process reading the task. They are trimmed by hand now. - Leftover group members were checked twice after the leader exited; one that lived longer kept "processes left" forever, and a later kill request could reach a reused group id. The group is probed until it is empty, then forgotten. - A finished task whose processes still run is not removed: nothing could stop them afterwards. - Stop requests record when they were made, so views can pace escalation.
raiseCatError
added a commit
that referenced
this pull request
Oct 11, 2026
…sk quality and security pass New heads for #375-#378, #383 and #384; a prioritized findings table with the fixes and their commits; the investigated locations that are not vulnerabilities (the demo renderer, configAssist.ts:164, the session socket, Git in untrusted repositories, dependencies); what cannot be validated before integration; and the contract changes Codex needs at integration, including one precedence for Up from an idle composer.
This was referenced Oct 12, 2026
raiseCatError
added a commit
that referenced
this pull request
Oct 14, 2026
Each session owns its background tasks beside its shell, in the session service or in the in-process client: SessionTasks wraps the #383 supervisor, tasks keep running while a window is detached, a reattached window receives the list and each task's recent output (never a restart), and ending the session ends its tasks. A shell switch keeps them. Protocol: the background-tasks feature on both sides; task (start, stop, remove) from the frontend; tasks and task-output from the owner, only to a frontend that listed the feature and only after created/attached and the replay. Every request, record and update is validated on arrival. Task output travels separately from the shell's stream and never reaches its PTY; an update carries at most 1 MiB, newest lines first. A start carries the live shell's PATH; the owner supplies the session shell and its launch environment, which is used for the spawn only. TaskOutput gains receive() so a frontend mirror keeps the owner's line numbers under the same limits.
This was referenced Oct 14, 2026
Owner
Author
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.
First slice of #382. Standalone modules in
src/backgroundTasks/; no shared files changed (TerminalApp, session service and protocol are untouched — the integration contract is indocs/design/background-tasks.md). Based onrelease/v0.18.0.Architecture. The real shell runs one foreground job; a background task is an NMSh-owned process in its own PTY and process group, run as
<shell> -c <command>with the command as one argv element (exact quoting preserved, tested). It inherits the session's launch environment, the live PATH and cwd — not live aliases, functions, unexported variables or options (stated in the doc and, in the UI PR, at the point of choice). A running foreground command is never moved, killed or re-run.supervisor.ts: stop = SIGINT to the task's group (Ctrl+C semantics, not a 0x03 byte), explicit SIGTERM/SIGKILL escalation;stoppinguntil the exit is observed; NMSh's shell, other tasks and detached helpers (adb-server style) never signalled; leftover group members reported and killed only on request; remove only for finished tasks; environment never on records; running-task limit; coalesced notifications.detector.ts: conservative family recognition (adb logcat≠adb logcat -d≠adb devices,tail -f≠tail,npm run dev= "likely",npm testnot); plain pipelines only; never substitutions, redirections, lists, sudo/ssh or full-screen programs; nothing executed.preferences.ts: per-family Always/Never/Ask, no global always-background, private storage without commands or output;decide()backgrounds only on explicit Always; per-invocation foreground override.output.ts: sanitized, numbered, bounded lines; progress lines; PTY\r\r\n(a real bug the tests caught).Tests (real PTYs, controlled programs mimicking logcat/tail -f/scrcpy/adb server):
tests/backgroundSupervisor.test.ts,tests/backgroundDetector.test.ts.Next: stacked UI PR (task bar, keys, suggestion strip,
/backgroundmanager model, task output view); integration (service protocol + TerminalApp) after #359's checkpoint. Physical QA with real adb/scrcpy is listed and not performed.Update (fd49bfa), from Linux CI: Ubuntu's
/bin/sh(dash) doesn't exec the command, so SIGINT ended the wrapper shell while a program ignoring SIGINT kept running, and the task was reported stopped too early. After a stop request the task now staysstopping, with escalation still reaching its group, until the whole group is gone. A new test reproduces the dash case on macOS and fails against the old code.Update (4b9685d), quality and security pass: output cleaning trimmed trailing
\rwith/\r+$/, quadratic on a run of carriage returns (128 Ki took 3.7 s and froze the reader); trimmed by hand now. Leftover group members were checked only twice after the leader exited, so a longer-lived member kept "processes left" forever and a later kill could reach a reused group id; the group is now probed every 250 ms until empty, then forgotten (the doc's earlier "signals only reach unreaped leaders" claim was inaccurate and is corrected). A finished task whose processes still run is no longer removable. Stop requests recordstopRequestedAtso views can pace escalation. Each fix has a regression that fails without it. CI green on4b9685d(macOS and Ubuntu shards, Node 22, Quality, Verify).