6 Commits
Author SHA1 Message Date
PierrunoYTandAmp bb3346a828 fix(execution): reserve process capacity before launching (#1092)
Reject starts when all slots are live or reserved, prune only completed history, and release reservations on launch failure. Keep live processes tracked even after failed termination. Add concurrent admission and real-process regressions for #1085.

Amp-Thread-ID: https://ampcode.com/threads/T-01a0dcd5-eb93-74c9-a1ac-5b140edaf973

Co-authored-by: Amp <[email protected]>
2026-10-12 15:03:04 +08:00
Anandan d73ec81e2d feat(tui): interactive terminal attach for tty exec sessions (#1055)
* feat(execution): add a stdin-only write for interactive processes

* feat(tui): attach overlay for interactive terminal sessions

* feat(tui): add /attach for interactive terminal sessions

* feat(tools): point the model at tty sessions for interactive prompts

* feat(tui): open the terminal automatically for tty exec sessions

* feat(tools): tell the model to keep polling while the user types

* feat(tui): give attached terminal sessions the full viewport

* feat(tools): route sudo through an escalated tty session

* fix(tui): route the auto-attach tick to its run

* fix(tui): defer auto-attach while another view owns the screen

* fix(tui): keep bracketed-paste framing for attached sessions

* fix(tools): let tty sessions run interactive commands

* fix(tui): capture the auto-attach baseline before the exec starts
2026-09-15 08:40:47 +08:00
PierrunoYTandClaude Opus 5 27b319ca88 test(execution): fail loudly when ps cannot answer instead of skipping (#961)
* test(execution): fail loudly when ps cannot answer instead of skipping

Every test in process_unix_test.go proves something about zombie detection, and
every one of those proofs is read out of ps. When ps could not answer, the
tests skipped — which is indistinguishable from success in CI output — and one
assertion passed vacuously for the same reason. A regression in ps parsing or
availability would have shown up as green.

requireProcessState probes BOTH ps pipelines against this test process, which
is by definition alive and not a zombie, so a working ps must report a live
state for it. When it cannot:

- Under CI (or ZERO_REQUIRE_PS=1) this is a hard failure. ps is present on every
  image used today, so its absence means the pipeline regressed and that has to
  surface immediately.
- Otherwise it stays a skip, so a contributor on an unsupported box is not
  blocked.

Both pipelines are probed because they are deliberately separate: the tests
assert what the production process-table parser concludes, so this file keeps
its own per-PID ps oracle rather than borrowing that parser to establish its own
preconditions. One regression must not be able to silence both sides at once.

Once the probe passes, failing to observe an EXPECTED state is a defect rather
than an environment problem, so the remaining escapes become failures:

- The zombie waits (awaitZombie) fail instead of skipping. The caller never
  waits on the child, so its zombie row persists until reaped — reaching the
  deadline means the child never exited or ps stopped answering, and the two
  are reported distinctly.
- A zombie group that is not signalable fails instead of skipping: a zombie
  still occupies the process table, so if kill(-pid, 0) fails the case under
  test is not present and the assertions after it would prove nothing.
- processSignalTarget returning anything but the individual PID fails instead
  of skipping: the child inherited this process's group, so its PGID is this
  process's PID and can never equal its own PID.

The vacuous assertion in TestTerminateProcessTreeStopsRunningGroup now requires
ok. That is sound rather than merely stricter: the test never waits on the
child, so the group's zombie row must still be present after termination, and
an unknown state means ps stopped answering rather than that the group is
clean.

Verified on Linux (WSL, go1.26.6) rather than by cross-compilation alone:

- all three tests pass unmodified
- with a stub ps that exits 1: CI unset skips with the reason named; CI=true
  fails, naming which pipeline could not answer
- the same broken-ps run against the ORIGINAL file produced two silent skips
  plus one unrelated failure after 12s, versus an immediate, explicit failure now

Windows is unaffected (the file is !windows); go vet passes for linux, darwin
and windows.

Fixes #863

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01U389qmQUhoZB3YtXkFmiTq

* test(execution): treat an unsignalable zombie group as a kernel difference

The macOS Smoke job failed:

    the zombie group 5931 is not signalable (operation not permitted)

That was my error. I converted this skip along with the ps-availability ones,
but it never had anything to do with ps: whether kill(2) may signal a group
whose only member is a zombie is a KERNEL difference. Linux permits it, so
signalTargetRunning goes on to consult ps — the branch this test was written
for. Darwin returns EPERM, so signalTargetRunning short-circuits on the kill and
never reaches ps.

Rather than restore the blanket skip, the test now records which branch answers:

- Signal permitted: assert processGroupHasRunningMember directly, so a ps-side
  regression cannot hide behind the kill result. This is new coverage; the old
  test only checked signalTargetRunning, which conflates the two paths.
- Signal refused: log it and continue.

The outcome assertion — a zombie-only group must not count as running — now runs
on EVERY platform. Previously Darwin skipped before reaching it, so this is
strictly more coverage there, not less.

Any errno other than EPERM still fails: it means the group is not in the state
the test needs, and nothing after it would be meaningful.

TerminateProcessTree is asserted only where the signal is permitted, because
terminateTarget's own kill(2) would hit the same EPERM and return it as a
cleanup error. Worth a maintainer's eye: that may be a real gap in
TerminateProcessTree on such a platform — a zombie-only tree is already exited
and arguably should terminate cleanly rather than report failure, which is the
same spurious-cleanup-failure class #774 set out to remove. Confirming that
needs a Darwin host and is a production change, so it is flagged rather than
guessed at here.

Verified on Linux (WSL, go1.26.6): all three tests pass, including the new
direct ps assertion. The Darwin branch was exercised by forcing the EPERM path
on Linux — it logs both explanations and passes with the outcome assertion
still enforced. go vet passes for linux, darwin and windows.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01U389qmQUhoZB3YtXkFmiTq

---------

Co-authored-by: Claude Opus 5 (1M context) <[email protected]>
2026-08-26 13:22:43 +08:00
beardthelion 6edf9a8b78 fix(sandbox): normalize launcher names before the command-prefix denylist (#934)
* fix(sandbox): retire recorded approvals through a policy bump

Ahead of tightening which command prefixes may be approved, make the
upgrade path safe. A grant already on disk that the narrower rules
reject fails validation on read, and that path rejects the entire grants
file rather than the one entry. Every caller treats a read error as "no
grant" (engine.go LookupCommandPrefix, ApprovedCommandPrefixes, and the
Lookup in Evaluate), so one stale prefix would silently disable the
user's persisted deny grants too, with the error text never reaching
them.

Bump the approval policy version instead and let the store's existing
migration handle it: it backs the file up, keeps deny grants,
invalidates prior approvals, and reports the counts once through the
startup notice that both frontends already print. That is what the
mechanism was built for.

The v1-migration test pinned the policy version as a literal, so it now
derives the expected value from the constant and will not need editing
on the next bump.

* fix(sandbox): normalize launcher names before the command-prefix denylist

unsafeCommandPrefixLauncher matched the program name as written, so
python3.11, python2.7, python.exe, node.exe and git.exe all validated as
ordinary commands and could be persisted as command-prefix grants.
Lookup is a string-prefix match, so a single approval then auto-allowed
every later invocation: granting [python3.11 -c] covers
`python3.11 -c "import os; os.system(...)"` with no further prompt.

internal/agent's commandName already normalizes case and Windows
executable extensions for the allow side of the same decision; the deny
side did not, and the two disagreeing is the bug. Reduce the program to
the launcher it actually runs and match on that: executable extension,
Windows' trailing dots and spaces (python. starts python.exe), CPython
ABI flags, then a trailing version, testing the digit-stripped form as
well so python3.11 and python3 both arrive as python. A name is only
ever narrowed toward an existing entry, so the failure direction is an
extra permission prompt, never a silent grant. Digits without a
separator stay part of the name, which keeps base64, 7z and sha256sum
grantable.

Two Windows spellings cannot be resolved by name at all, so the shape is
refused alongside a path: 8.3 short names truncate the stem, which makes
POWERS~1.EXE unrecognizable, and python.exe::$DATA reaches the same
executable through its default stream. Normalizing the first token
before the banned-suggestion comparison keeps git.exe at parity with
git.

The list also gains launchers of the same class it already covers and
that normalization cannot reach: cmd, wsl, the remaining POSIX shells,
busybox, uv and uvx. Entries normalization now covers (/bin/bash,
/bin/zsh, powershell.exe, python3, pypy3) are gone; the first two were
already unreachable behind the path check above them.

Covers the persisted store, the session-scoped grants the agent takes
during a turn, the legacy-schema migration that prunes such a grant
per entry, and the Windows spellings hermetically.

* fix(sandbox): treat free-threaded CPython as a launcher too

The ABI flags came from the pre-3.13 set, so "python3.13t" and
"python3.13t.exe" normalized to themselves and stayed grantable as
command prefixes while the GIL build "python3.13" was refused. A
free-threaded interpreter runs arbitrary code just the same.

Add "t" to the flags trimmed under the existing digit guard, which is
what keeps ordinary names intact: "zstd" and "cat" have no digit beneath
the trimmed letters, so they keep their own names and stay grantable.

Reported by CodeRabbit on #934.

* fix(sandbox): refuse the remaining spellings of listed launchers

Same defect as the free-threaded build, found by sweeping the families
already on the list for names it cannot reach:

- nodejs is node on Debian and Ubuntu, and was grantable.
- python3-dbg, python3.11-dbg and python3.13t-dbg are interpreters that
  run arbitrary code; so are bash-static, pwsh-preview and
  node-nightly. A closed set of build-channel suffixes now normalizes
  them. The set stays closed deliberately: a general "-word" strip would
  also swallow node-gyp, python3-config and ruby-lsp, which are ordinary
  tools a user should still be able to approve, and those are pinned as
  must-stay-grantable.
- pnpm, yarn and bunx run package scripts exactly as npm and npx do, and
  npm and npx are already refused.
- sudoedit escalates like sudo.

The launcher-wrapper family (winpty, chroot, unshare, nsenter, parallel,
script, unbuffer, taskset, flock) has the same inconsistency against the
env/nohup/timeout/setsid entries already on the list, and is left for a
separate decision rather than widened here.

* test(sandbox): cover the -debug and -beta launcher suffixes

Four of the six build-channel suffixes had regression cases; -debug and
-beta did not, so removing either from the set left every test green.
Both now fail RED when their entry is dropped.

Reported by CodeRabbit on #934.
2026-08-21 12:50:11 +08:00
9c292092a2 fix(daemon): clean up child after startup timeout (#774)
* fix(daemon): clean up child after startup timeout

* fix(daemon): terminate startup process tree

* test(background): accept reaped-later zombie children

* fix(background): retain configured process group target

* fix(background): address termination review feedback

* fix(daemon): use launch-time group identity, fix zombie/reap classification

Addresses jatmn's three #774 review findings:

- TerminateCommand rediscovered the process group via
  execution.TerminateProcessTree's Getpgid(pid) lookup. On Darwin,
  that lookup can return ESRCH once an unreaped group leader has
  already exited, even though live descendants remain in the group
  it configured at launch — silently falling back to signalling only
  the (already dead) leader PID and leaving descendants running (the
  required macOS smoke job's failure). Add
  execution.TerminateProcessGroup, which signals the negative PID
  directly using the launch-time Setpgid invariant instead of
  rediscovering it, and route TerminateCommand through it.

- signalTargetRunning's zombie check matched the individual-PID
  fallback target (a process that is not its own group leader)
  against the process table by PGID. That target's actual PGID
  differs from its own PID by definition, so no row ever matched,
  unknown resulted every time, and a zombie individual-PID target
  was always conservatively reported as still running. Branch on the
  sign of the target: group members by PGID, individual PIDs by PID.

- TerminateCommand returned the termination error even when the
  subsequent reap succeeded — a successful cmd.Wait is authoritative
  proof the leader is gone and collected, regardless of how the kill
  attempt itself went (e.g. taskkill /T racing an already-exited PID
  on Windows). Classify a successful reap as success.

All three regression tests are verified to fail against the pre-fix
code (confirmed by temporarily reverting each fix and rerunning).

Co-Authored-By: Claude Sonnet 5 <[email protected]>

* fix(daemon): harden command termination inputs

Amp-Thread-ID: https://ampcode.com/threads/T-019fa018-abb8-73de-9489-51e9e6514c6b

Co-authored-by: Pierre Bruno <[email protected]>

* fix(background): preserve process tree failures

Amp-Thread-ID: https://ampcode.com/threads/T-019fa7a0-1223-701d-9529-48ba5d7cf8c8

Co-authored-by: Pierre Bruno <[email protected]>

* docs(daemon): clarify boundary-probe and Setsid-guard comments per PR #774 review

Co-authored-by: Cursor <[email protected]>

---------

Co-authored-by: Claude Sonnet 5 <[email protected]>
Co-authored-by: Amp <[email protected]>
Co-authored-by: Cursor <[email protected]>
2026-08-15 20:31:10 +08:00
Anandan 96859c9bd1 feat(sandbox): unify command execution and enforcement (#781)
* feat(sandbox): unify command execution and enforcement

* test(sandbox): make platform contracts portable

* fix(sandbox): close execution lifecycle races

Bound retained interactive output, preserve interruption state, and start workspace observation before process launch.

Protect active runtime roots with cross-platform leases, capture transient protected-path creation events, and restore process attributes after PTY fallback.

* fix(sandbox): restore strict macOS isolation

Remove Seatbelt host-local network allowances after native validation showed localhost filters can reach host interfaces.

Track process reap completion so retained sessions cannot signal a stale numeric PID.
2026-07-21 18:55:13 +05:30