Skip to content

land: kill-survivors - #848

Open
aft-alfonso[bot] wants to merge 12 commits into
mainfrom
land/kill-survivors
Open

aft-alfonso[bot] wants to merge 12 commits into
mainfrom
land/kill-survivors

Conversation

@aft-alfonso

@aft-alfonso aft-alfonso Bot commented Oct 24, 2026 •

Copy link
Copy Markdown

Accepted agent delivery; branch-keyed search descriptor included.


Summary by cubic

Reports processes that survive a bash kill. After a timeout, bash_kill, or cancelled call, AFT now re-checks the task's process group and names any member still running in live_descendants, the task text, and the completion notice — a process AFT cannot signal, such as one started with sudo and owned by another user, used to be reported as a clean kill.

  • Surviving members are listed with pid, owner, and executable name only, since arguments can hold credentials and are never kept in the record, status, or notice.
  • A check that cannot read every process reports listing_incomplete and keeps the members it did find; an unreadable /proc reports listing_failed, and a null list always carries the reason. A bash_kill on a finished task re-signals a group whose last check was incomplete or missing, but never one last seen empty.
  • macOS no longer drops another user's process from the listing, PTY tasks are checked after a kill, and a terminal state published mid-kill no longer skips the check.
  • The group listing is bounded: it describes at most 16 members cheaply and counts the rest without reading their command lines.
  • The bridge exports LiveDescendant and the unavailable reasons; docs now recommend bounding sudo commands yourself with timeout or a trap, since AFT cannot kill them.

Written for commit 477f627. Summary will update on new commits.

View guided diff Turn on auto-fix

RetriggerView in GreptileConfidence Score: 5/5

The changes since the previous review appear safe to merge.

Summary

AFT reports processes left running after a kill, without saving their arguments.

  • AFT reports processes that remain after a bash task is killed.
  • A later kill reaches a finished task’s group only when its identity still matches.
  • Plugins can read typed survivor details and reasons for missing lists.

No new actionable issues were found in the changes since the previous review.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Check task process group] --> B[Display up to 16 members]
  A --> C[Save up to 1,024 process identities]
  B --> D[Status and completion notice]
  C --> E[Later kill of finished task]
  E --> F{Saved process still in group?}
  F -->|Yes| G[Signal group and check survivors]
  F -->|No| H[Leave group alone]
Loading

Reviews (5) · Last reviewed commit: "mason: recognise a finished task's group..." · Reviewed by Greptile

ualtinok and others added 4 commits October 24, 2026 10:08
After a timeout, bash_kill or cancelled call, AFT lists the task's process
group again and names any member still running (pid, owner, command) in
live_descendants, the task text and the completion notice, with one line
that says the process survived the kill. A null list now always carries
live_descendants_unavailable with the reason the group could not be checked.

- macOS listing no longer drops another user's process: proc_name fails
  with EPERM for it, so owner and name now come from kern.proc.pid.
- Linux reports listing_failed instead of an empty list when /proc cannot
  be read, and records each member's owner and command line.
- PTY tasks are checked after AFT kills their group.
- A terminal state published while a kill was in flight no longer skips
  the check.

Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.com>
…istry

Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.com>
A process-group check used to read the command line and owner of every
member and only then keep the first 16. It now lists members by pid and
group (cheap), sorts them, describes at most LIVE_DESCENDANT_CAP and counts
the rest without reading them. bounded_members_stops_describing_at_the_cap
pins the bound.

Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.com>
Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.com>
Comment thread crates/aft/src/bash_background/process.rs Outdated
Comment thread crates/aft/src/bash_background/process.rs
Comment thread crates/aft/src/protocol.rs Outdated
ualtinok and others added 5 commits October 24, 2026 10:37
A survivor's command was its full argument list, so a lingering
`client --token "$DEPLOY_TOKEN"` saved the expanded token in
live_descendants, the survivor line, the task record and the completion
notice. The completion masker cannot be relied on here: its patterns do not
recognise argv-shaped secrets (--token X, --password=X, KEY=value), and the
exact-value list only knows secret-named environment variables, not a value
read from a file.

The shown command is now the executable name plus a fixed '…' when the
process had (or may have had) arguments; argv0 keeps only its first word, and
both pass through aft_redactor. Tests that pinned 'sleep 30' now expect
'sleep …' because the arguments are no longer kept, which is the point of
this change; the approved survivor line template is unchanged.

Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.com>
With /proc mounted hidepid, or any process whose stat cannot be read, the
group probe said the group still existed while the listing found nobody, and
the task reported live_descendants: [] - a clean kill.

A listing now counts each /proc entry as a member, a zombie, elsewhere, or
unreadable. If any entry is unreadable, or the group is alive but the listing
found no member or zombie, the group is probed once more: if it has exited in
the meantime the result is a clean [], otherwise the new
live_descendants_unavailable reason listing_incomplete. macOS applies the same
rule to an empty proc_listpgrppids answer. Test hooks hide or make unreadable
a group's members to reproduce both mount modes.

Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.com>
BashCompletedFrame, BgCompletion and the plugin types that mirror them
described a live descendant as {pid, comm, argv0} and had no
live_descendants_unavailable. The bridge now exports a LiveDescendant type
with the optional user and command fields, and the unavailable reasons as
LIVE_DESCENDANTS_UNAVAILABLE_REASONS with a type derived from it. A unit test
reads the engine's LiveDescendantsUnavailable enum and fails if the two lists
drift.

Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.com>
…830)

macOS /bin/sh is bash, so the survivor's kernel name is bash while its first
argument is sh. The test now finds the survivor by argv0 and expects its
shown command to be its own name plus the omitted-arguments marker.

Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.com>
Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.com>
Comment thread crates/aft/src/bash_background/process.rs Outdated
)

With /proc mounted hidepid=1, any unreadable process, even one outside the
task's group, made the group check return unavailable and discard the
survivors it had found, so live_descendants was null. A later bash_kill on the
finished task then counted zero survivors and sent no signal, leaving the
group running.

The listing now returns the members it identified with an incomplete flag.
The record keeps those members next to live_descendants_unavailable =
listing_incomplete; only an incomplete listing that found nobody stores null,
so [] still always means a clean check. The task text and completion notice
add one sentence (INCOMPLETE_LISTING_NOTE) whenever the listing was
incomplete. bash_kill on a finished task now signals its group when members
were recorded, or when the last check was incomplete or missing and the group
still answers signal 0. A group last seen empty is left alone, since an empty
group never regains members and a group answering under that id later is a
different one.

Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.com>
Comment thread crates/aft/src/bash_background/registry.rs Outdated
…its own (#830)

A later bash_kill on a finished task signalled whatever group answered
under its saved number whenever its last check was incomplete or missing
(a PTY task, for example). Once the original group has exited, the system
can give that number to an unrelated group, which would then be signalled.

Each check now records the identity (pid and start time) of the members it
found. A finished task's group is signalled only if its leader (the saved
number, with the start time recorded at launch) or one of those members is
still running in that group with the same start time. Without that evidence
the group is left alone, and the record keeps its survivor and unavailable
fields as they were.

The previous test that signalled a fully hidden group whose leader had
exited now asserts the opposite: nothing proves that group is still the
task's, which is the case this change exists to refuse. The partial case,
where an incomplete listing found the survivor, is still signalled.

Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.com>
Comment thread crates/aft/src/bash_background/registry.rs Outdated
…nly those shown (#830)

group_member_identities held only the 16 members a task shows. When a
finished task left more than 16 processes and the shown ones exited while a
member that was only counted (+N more) kept running, bash_kill found no
matching identity and sent nothing.

Each check now records the identity (pid and start time) of every member it
finds, separately from the 16 shown, up to GROUP_IDENTITY_CAP (1,024), and
sets group_member_identities_truncated when it found more. A match on any
saved identity is enough evidence to signal the group. A task record holding
the most a check can store (1,024 identities at their largest values plus 16
shown members with names at their cap) measures 63,199 bytes; a test bounds
it below 96 KiB. The identity cap is registered as a list surface.

Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.com>
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.

1 participant