Third review found that removeTree (851daeec2) failed on an empty
directory without read permission - `mkdir -m 000`, or a umask of 0777 -
because opendir needs read permission where rmdir asks only the parent.
The step noted EACCES, and every later sweep failed the same way, where
the recursive rm it replaced removed the directory. An EACCES from
opendir, before anything was listed, now falls through to that final rm;
a directory with entries still fails into the note as before.
Third review found that the only case guarding 8e3d9b15b's threshold
runs on macOS alone, while CI runs unit tests on Linux, where no file
system takes a 765-byte name: putting REMOVE_TREE_HOIST_AT_BYTES back to
512 left every CI case green. The fs mock now records each rename, and
a case pins that a directory whose path has passed 257 bytes is moved
before it is walked - it fails at 512 on any host.
The cleanup helper moves below the imports and drops a chmod no case
needs, and the long-name case's comment no longer says one name alone
passes 1 024 bytes.
Second review found that removeTree's move-up (096afddd1) left an empty
read-only directory behind for good. Moving a directory to another
parent needs write permission on the directory itself, which a plain
rmdir never asks for, so the rename was refused and the whole removal
failed where the recursive rm it replaced succeeded. A refused move now
leaves the directory where it is and walks it there: its own path still
fits, and an empty directory needs nothing below it named.
Second review found that removeTree (096afddd1) moved a directory up only
once its path passed 512 bytes, on the premise that a name adds at most
255. APFS takes 255 characters, which in UTF-8 is up to 765 bytes, so a
file or directory with a long multibyte name under a 490-byte path got
ENAMETOOLONG and was left in $TMPDIR for good. The directory is now moved
once its path passes 257 bytes, so any name below it fits in the 1 023
bytes macOS takes.
The docstring and the deep-tree case no longer say the rmSync it replaced
walked such a tree: on Node 20, which CI runs, it fails the same way. The
cleanup cases now clear their roots with `rm -rf`, since a throw from
fs.rmSync in a finally would hide the failure that left the tree there.
Review found a regression in 851daeec2: removeTree built full paths, so a
tree the script left deeper than the system's longest path - 1 024 bytes
on macOS - failed with ENAMETOOLONG and stayed in $TMPDIR for good, since
every later sweep failed at the same place. The rmSync it replaced walked
such a tree by descriptor and removed it, after freezing the tool server.
removeTree now moves a directory whose path has grown past 512 bytes up
under the directory being removed before it walks it, which keeps every
path it hands the system short. The main thread still does not stall.
After every .sh step the executor removed the step's exchange directory with
a synchronous recursive rmSync on the tool server's main thread. A script
that left many files there - a fixture it unpacked, a clone - froze every
request, device socket and flow on the host: 100 000 files held the event
loop for about 3 s.
An awaited fs.promises.rm of the whole tree is no cure: it starts one
operation per entry at once and their completions come back in bursts the
loop runs in one turn, stalling it for up to 1.2 s on the same tree. The
directory, and each one the stale sweep collects, is now walked a batch of
64 names at a time with at most 64 removals in flight; the worst stall for
100 000 files is about 20 ms. Each entry still goes through fs.promises.rm
for its force and Windows read-only handling, and a removal that fails
still becomes the step's "could not be removed" note.
A `.sh` step now reports what it prints the way a `.mjs` step does on
`main`: bash inherits the runner's stdout and stderr, which are the pipes
`ScriptLogCapture` reads, so the log, its limits, the drain and the secret
scrub apply with no bash-specific code.
The failure reason comes from stderr rather than from a file. bash has no
`throw`; what it has is an exit code and stderr, and every command a
script runs already writes its error there. So a non-zero exit reports the
runner's exit line followed by the last non-blank line the script wrote to
stderr - under `set -euo pipefail`, the error of the command that stopped
it, with no Argent-specific code in the script.
The runner never reads stderr: bash writes straight into the parent's
pipe. The executor's capture keeps the line (`LastLineTracker`), from the
raw text ahead of the scrub and the limits, so a script that floods stderr
and then says why still says it. Only the head of a line is kept - 1,000
characters and the usual omission marker - so a cut always lands at the
end of the message, where `redactTruncated` repairs one, and the line
joins the failure text before it is clamped and redacted. A line of
nothing but whitespace is blank at any length. stdout is never the
reason, and only the `exit` kind gets the line.
`$ARGENT_REASON` goes, and with it the runner's bounded read of the
reason file, its kept-count marker and `REASON_KEPT_RE`, its UTF-8
refusal, the CRLF hint for a reason that strayed, and the second exchange
file. `ARGENT_REASON` is an ordinary environment name again; the CRLF hint
for `$ARGENT_OUTPUT` stays.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X2EFhLrnjAkfgPkN1fva1k
A `script` step's work is as often a shell one-liner as a Node program:
`curl` an internal API, `psql -f` a seed file, `adb shell` a settings
change. This adds `.sh` as a second language on the existing step - one
new extension, no new key, no new tool, no new step kind and no new
report field.
The contract is one line each: input is the environment, the output
document is the file named by `$ARGENT_OUTPUT`, the verdict is the exit
status, and a failure reason is the file named by `$ARGENT_REASON`.
Bash runs inside the runner rather than instead of it. The executor forks
the same `flow-script-runner.mjs`, as the entry module rather than as an
`--import` preload, and the runner spawns bash as its own child. That
keeps one process group per step, so the parent's group stop and both
watchdogs' group kills reach a bash descendant unchanged - which matters
because a bash polling loop is exactly the orphan the watchdogs exist for,
and a shell has no thread to watch a descriptor from.
Hubert Gancarczyk