Fifth review found that a cancel landing while a stderr consumer was
still delivering the script's last lines - after "step 1" but before the
error - made the reason "step 1: seeding": 76b619e23 let a cancel end the
post-exit wait at once, and ede786396 then took the line as it stood.
4edac0170 gave the error, because its 500 ms settle ran whatever the
caller did. A cancel now ends the wait only once that first settle has
passed, so the consumer gets as long as it always had and a cancel waits
no longer than it did before the wait could stretch. The abort leaves
the race once it has fired, so the loop does not spin while it waits.
Fourth review found that a cancel landing within 500 ms of a stderr
consumer's late line dropped that line from the reason: the settle ended
on the abort before stderr had been quiet for 500 ms, and the reason fell
back to the line as bash exited - empty, the consumer not having written
yet. A cancel says nothing about whether a job is still writing, so a
settle it ended now takes the stderr line as it stands.
Fourth review found that a block of the tool server's event loop across
the 500 ms mark - a synchronous call in an I/O callback - made the reason
freeze on the line before the script's error. After such a block the
due timer runs before the poll that reads what arrived meanwhile, so both
the quiet watch and the settle judged stderr quiet on stale data: with a
stderr consumer and a surviving job, the reason ended with "step 1:
seeding" instead of "FATAL: the real error". Each now lets one turn go by
after a timer, so pending pipe data is read before the judgement, and the
watch stops once the settle ends so a late check cannot take a line the
stop provoked.
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 fd41a28b5 let a job's stray stderr line become a
bash step's reason: a mock server that logs requests on stdout keeps the
settle open, and its one stderr line a second after bash exited
("listening on :8080") was taken over the script's error, which
4edac0170 reported. After bash exits, stderr carries the script's own
lines late - a consumer working through its backlog - and a job's lines;
the first come as one run from the exit, the second whenever the job
writes. The reason now takes the line stderr stood on when it first went
quiet for 500 ms after the exit. Streams that close while stderr is
still running on count in full; stderr that never goes quiet is a job
still writing, and then the line is the one bash exited on. The docs say
the same.
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.
Second review found that removeTree (851daeec2) opened the directory it
was handed with opendir, which follows a symbolic link. The stale sweep
hands it every entry of $TMPDIR whose name carries the executor's prefix
and a past stamp, so a link planted under such a name had the directory
it pointed to emptied - where the recursive rm it replaced removed the
link alone. A script that replaced its own exchange directory with a
link got the same. removeTree now lstats the top and removes anything
that is not a directory as itself; below the top a directory entry
already says whether it is a link.
Second review found that 76b619e23 ended the post-exit wait at once for
every cancelled run, including one cancelled before the script answered.
That run was already stopped, and a job in a group of its own outlives
the stop: before, the wait still saw it writing and marked the log cut;
after, the log stopped short with no flag and no note. The settle now
takes the abort only when it had not happened by the script's exit, so
only a cancel during the wait itself ends it. The cancel case also pins
that a job still writing then marks the log cut.
Second review found that a1913b052 read the settle's "cut" as a sign that
stderr was still being written, when a job chattering on stdout alone
also ran the settle to its limit. With a stderr consumer in front of the
script's error and such a job, the reason took the line as bash exited -
empty, the consumer not having written yet - and dropped the error that
4edac0170 reported. The line as bash exited is now used only when stderr
itself was still being written as the settle ended.
Second review found that 4a5f9b5bd, by answering the probe once the
candidate had exited and its stdout had ended, cut off the stderr line a
refusal quotes when that line arrives after the candidate is gone: a
wrapper that sends stderr through `tee`, or hands it to a job. The early
answer is only needed for a candidate that answered with a version, so a
refused one waits for `close` or the settle again, as it did before.
The probe-timing case's grandchild now lives 1 s instead of 10.
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.
Review found the case that pins the log-cut note tight under load: its
job wrote its first line 200 ms after bash exited, and a stall of that
size between the runner's answer and the read of stderr would put the
job's line in the reason. The job now waits 300 ms, still inside the
500 ms quiet window the case needs. The case's name and comment no
longer say Argent stops the job, which it cannot do everywhere.
Review found that 979c9c473, by piping a candidate's stderr to quote it,
made a candidate that leaves a job holding stderr cost the probe's 250 ms
settle on every .sh step: `close` now waited for stderr as well. The
answer is on stdout, so the probe now answers once the candidate has
exited and its stdout has ended, as it did before stderr was piped.
Review found that 851daeec2 weakened "has finished its sweep by the time
the step resolves": its spy delayed every fs.promises.rm call, and since
the step now removes its own exchange directory through fs.promises.rm
too, that removal outlasted the sweep and the case passed with the
executor's `await pendingSweep` deleted. The spy now delays only the
abandoned directory's removal, so the case fails again without the wait.
Review found that 06b7891da's longer settle ignored the request's abort:
the timer and the abort listener are removed when the script's process
exits, so a run cancelled while a job the script left running was still
writing waited up to 3 s more (before: at most 0.5 s). The settle now
takes the request's signal and ends at once; if output was still
arriving, the log is marked cut as it is at the limit.
Review found that 979c9c473 quoted the last non-blank line a refused bash
candidate wrote to stderr. asdf, the version manager the fix was for,
writes its reason first ("No version is set for command bash") and then
lists the versions it has, one per line, so the refusal quoted
"bash 5.2.37" - a line that reads as if the candidate were a bash.
The refusal now quotes the first non-blank line. The docs say which line
is quoted, and that only a candidate that exits without a version gets
one.
Review found a regression in edbdf4dfb: when stderr went through a consumer
(`exec 2> >(…)`) and a quiet background job held the streams open, the
reason took the stderr line as it stood when bash exited - often nothing,
since the consumer had not written yet - and dropped the script's error
that 4edac0170 reported.
The line now follows how the settle ended: the last line overall when the
streams closed on their own, the line stderr ended on before Argent's stop
when what held them went quiet, and the line as bash exited only when a
process was still writing at the settle's limit. A job's answer to the
stop still never becomes the reason.
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.
After the script's process exited, the executor waited at most 500 ms for
the output pipes and then stopped the process group. A stderr consumer
still working through its backlog - `exec 2> >(…)`, the plain-bash
timestamp idiom - was killed mid-drain: the log lost its tail with
scriptLogTruncated still false, and a failing step's reason ended with a
progress line instead of the script's error.
The settle now stretches while the tree keeps writing: it ends when the
streams close, after 500 ms of quiet, or 3 s after the exit. A tree still
writing at that limit is stopped, the log is marked cut, and the step
carries a note saying so.
The bash version probe ran in the tool server's own working directory and
dropped the candidate's stderr. A version-manager shim picks its bash from
the directory it starts in (asdf reads .tool-versions), so a shim on PATH
was refused and the step fell through to /bin/bash - Apple's 3.2 on a Mac -
with nothing said, and a shim pinned in scripts.bash errored with "is not a
bash" or passed depending on where the tool server was started.
The probe now runs in the step's working directory, the refusal quotes the
last line the candidate wrote to stderr, and a step that runs under a later
candidate after a refused PATH bash carries a note naming the refusal.
A failing script that left a background job running took its reason from
whatever that job wrote to stderr after bash exited, often the line that
Argent's own SIGTERM provoked ("mock-server: SIGTERM received, closing").
The reason now takes the stderr line as it stood when the runner reported
bash's exit whenever the stop had to end a process still holding stderr.
A step whose streams close on their own keeps the last line overall.
The parent classifies a runner that ended with no verdict by how it ended,
and reads the clock rather than its own timer for one case: a SIGKILL past
the step's limit is the deadline watchdog's stop, not an unexplained kill,
because a stall in the tool server's event loop can hold the timer behind
the exit it is racing.
Windows has no signal for that rule to see. The watchdog's `taskkill` ends
the runner with exit code 1, so a stalled tool server's deadline stop there
read as "The script stopped its own process with exit code 1" - the case
the Windows runner kept reporting as `exit` after the runner itself stopped
sending bash's forced exit. Past the limit, a runner that ended with no
verdict on Windows is now that same stop. POSIX is unchanged: the rule
still needs a signal there.
The deadline case asserts the whole failure now, so a wrong verdict says
which side produced it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X2EFhLrnjAkfgPkN1fva1k
Two more cases read their descendant's pid from `sleep & echo $!`: the time
limit and the cancellation, both of which then ask `process.kill(pid, 0)`
whether it is gone. Under Git Bash `$!` is an MSYS number, not a Windows pid,
and a small one can belong to an unrelated Windows process - the time-limit
case passed on one Windows run and failed on the next at "no process left
behind".
One helper now starts the descendant as a Node process that writes its own
`process.pid`, as the lifeline case already did, and the deadline case uses
it too. The POSIX-only cases keep `$!`, which is a real pid there.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X2EFhLrnjAkfgPkN1fva1k
With the IPC fix in, ten bash cases still failed on the Windows runner.
None is a product failure; each asserts something Windows does not do, or
measures it in a way that cannot work there.
- A named pipe as the document: Git Bash's `mkfifo` makes a Cygwin FIFO
that Node sees as no file at all. The directory row still runs there.
- Three CRLF cases: Git for Windows' bash drops a carriage return from the
line it reads, so a CRLF script runs there as its LF twin - all three
ran clean on the runner. POSIX only.
- The stray-sibling case wrote a literal carriage return inside `$'...'`,
which that same rule turned into an empty word, so the redirection
emptied the real document. It now writes U+F00D on Windows, the name
msys2 gives such a file, as the case beside it already does.
- Three signal-verdict cases: the step dies by a signal, and Windows has
none to send. POSIX only, as the file already says of signal verdicts.
- The deadline-watchdog case found its descendant through `sleep & echo
$!`, and under Git Bash that pid is an MSYS number `process.kill` cannot
ask Windows about, so it read as dead from the start. A Node descendant
writes its own pid now, as the lifeline case does - which makes this the
case that runs the watchdog's `taskkill` arm on Windows.
- The sweep's 12 ms stall bound sits under Windows' ~15.6 ms timer tick,
so a 5 ms heartbeat cannot resolve it there. Skipped on Windows.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X2EFhLrnjAkfgPkN1fva1k
Both were written against macOS and had never run on Linux: until this
branch targeted `main`, only the description check ran on its pull request.
The mode case read a file's mode with `stat -f '%Lp' "$1" || stat -c '%a'
"$1"`. GNU `stat -f` is `--file-system`, so on Linux it treats `%Lp` as a
file it cannot find and still prints the file system `$1` sits on to
stdout before the fallback runs - and that report became the document the
script wrote. GNU's `-c` comes first now; BSD `stat` refuses it with
nothing on stdout.
The body `kill 0` case pinned macOS's answer, where the signal reaches bash
alone and arrives as a plain signal. On Linux it reaches the whole process
group, the runner that leads it included, which is the case the runner
already reports as a group signal. Both reports name `kill 0` and the
remedy, so the case now accepts either.
Reproduced in a `node:20-bookworm` container (GNU bash 5.2.15) before the
change - `expected 'exit' to be 'signal'`, and a document of `undefined`
- and green there after it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X2EFhLrnjAkfgPkN1fva1k
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
With #970 no longer under this branch, `main`'s `ScriptLogCapture` is back,
but the split this branch made - `prepare` handing a `ChildRun` to
`runChild` - still carried #970's `StderrSignalWatch` in the run. The
capture was built in one function and read in the other, so the file did
not compile. The run now carries the capture itself.
The three step-run helpers #970 added and this branch's bash tests call
(`markPath`, `markingScript`, `readMark`) come back with it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X2EFhLrnjAkfgPkN1fva1k
The abort added after `resolveBashInterpreter` was found to hold a step
un-cancellable for about six seconds per candidate was pinned by one case, and
that case calls `pinGlobalConfig` first - so it covers the configured branch and
none of the search. Three deletions left all 517 green:
- the check inside the candidate loop, which is the PATH search every ordinary
install takes. Without it the cancellation is swallowed and the resolver
reports success, or `spawn`/"install bash" on a host where nothing answers.
- the entry check, which makes an already-raised signal cost nothing. Without
it an aborted call replayed is a full probe round per replay, plus the
detached probe children each round leaves.
- `request.signal` in the executor's own call, so nothing asserted the executor
wires it through at all and the `"cancelled" in found` arm was reachable
through no test.
One case each. All three fail on their deletion - 310 ms against 6008 ms and
6016 ms - and the executor one aborts DURING the lookup, because the check at
the top of `runOne` answers a signal already raised and would pass either way.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Z6tHq3XazgZbrS4YT73xs
The case written for 103a25f94 - the whole-message clamp landing inside the
reason's own marker - could not fail on it, for two reasons.
`pinBash` wrote `<ws>/.argent/config.json`, and `scripts.bash` takes the global
scope alone, so `readScopeValue` returned before that file was read. The 934-
character symlink never reached a message, and the case ran under the host's
ordinary short bash path. It now pins the value in a home of its own, and
asserts the pinned path is IN the message so the setup cannot go inert again.
The arithmetic then pointed the wrong way: `- 50` put the message 50 characters
SHORT of the ceiling, where the premise of the fix is an exit line LONGER than
the room `MAX_REASON_CHARS` reserves for it. `+ 20` passes the ceiling and keeps
the path inside PATH_MAX.
With both, reverting the fix in the runner - the budget back to a fixed
`MAX_REASON_CHARS` and the `budget <= 0` guard dropped - fails the case:
"expected 'The script exited with code 5 (bash: …' to match
/this report keeps the first \d+ chara…/".
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Z6tHq3XazgZbrS4YT73xs
Neither test written for `scripts.bash` taking the global scope alone could
fail on it. Both wrote a valid GLOBAL value alongside the committed project one,
and `merge` is `prioritize-global` - `global ?? local` - so the assertion holds
through the merge whatever `scopes` says, with the project scope fully readable.
`flow-script-interpreter.test.ts` now sets no global value, so the committed
Windows path is the only one there is: read, it would BE the answer, the key
would count as set and there would be no PATH fallback. The search running at
all is the assertion.
`config-access.test.ts` loses "keeps the developer's global pin in front of a
committed project value": with the project scope unread its two lines writing
the file are inert, and what it asserts is what the case two above it already
asserts by writing the project file alone.
All three fail with `scopes: ["global", "project"]` put back.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Z6tHq3XazgZbrS4YT73xs
`SCOOP`, `SCOOP_GLOBAL` and `%ProgramData%\scoop` - the override, the
administrator tree and the global Scoop layout - were built by nothing: the one
win32 `fixedLocations` case that could reach them deletes all three before it
asserts, and the Scoop case beside it covers `%USERPROFILE%\scoop` alone. The
`it.each` table now carries a row per rung, on a platform this repo cannot run
unit tests on by default.
The case-insensitive de-duplication had the matching gap. Its only case fed
`ProgramFiles` and `where git`'s answer as byte-identical strings, so a plain
`Set` passed it; the divergent-casing case the `toLowerCase()` is written for -
and which costs the duplicate five-second probe - was pinned by nothing. The
rung is now spelled `c:\program files` against `where git`'s
`C:\Program Files\...`.
All four fail on mutation: dropping the three rungs, and keying the `Set` on the
candidate as written.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Z6tHq3XazgZbrS4YT73xs
`readConfigObject` answers an empty document for a file that is absent, one a
`chmod` made unreadable and one an interrupted `updateConfig` left half-written,
so `scripts.bash` read as unset in all four cases. The same file content then
gave opposite outcomes:
readable, bash=/nonexistent/bash -> ok=false "The configured bash … does not exist."
chmod 000, same content -> ok=true notes=[] bash 5.2.37 from the PATH
truncated JSON, same content -> ok=true notes=[] bash 5.2.37 from the PATH
The last two are exactly "a wrong path papered over by a fallback that happens
to exist on this machine" - the outcome the resolver's first rule says it
prevents, and `configuration.mdx` stated to users as a rule.
`configDocumentProblem` in configuration-core says why a document could not be
read; it is additive and changes what no key resolves to. The resolver asks it,
and a step that fell through to the PATH now carries a note naming the file and
the reason. Refusing instead would break every `.sh` step on a host whose
corrupt file never held the key.
The docstring and `configuration.mdx` now separate the two cases: a value that
is READ and unusable errors the step; a document that could not be read at all
is a note.
(The JSON `null` half of this thread was already resolved in 1971840c7 -
`asPresentText` maps `null` to absent, as every other parser in the file does,
so `{"scripts":{"bash":null}}` means unset by decision.)
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Z6tHq3XazgZbrS4YT73xs
`startedAt` is taken before `resolveBashInterpreter` and the step's own timer is
armed after it, inside `runChild`, so the lookup is inside `durationMs` and
outside `timeoutMs`. It is not the queue either, so `queuedMs` is 0. A step
declared `timeoutMs: 500` measured:
candidate slow only for the version probe durationMs 3398 notes []
candidate that traps SIGTERM and never answers durationMs 6054 notes []
.mjs control durationMs 41 notes []
A `.mjs` step has no such gap - everything between those two points there is a
`statSync` and a `JSON.stringify` - so nothing anywhere said why a `.sh` step
ran seven or twelve times its declared limit.
The step now says so, when the lookup is worth a sentence beside that limit, and
names the remedy the search already has. `flow-yaml.mdx` said the queue was the
one source of an over-run and that the step reports it; it now names both.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Z6tHq3XazgZbrS4YT73xs
The window on the candidate's standard output is a window the answer falls out
of, whichever end it is on, because a wrapper prints on both sides of the bash
it runs. The head window was moved to the tail for a wrapper that greets before
`exec`ing bash; a wrapper that RUNS bash and then prints - to clean up, or to
exit with bash's own status - lost the marker off the other end, at exactly
`BASH_PROBE_MAX_CHARS` minus the marker line:
trailing banner 4058 -> pass
trailing banner 4059 -> "is not a bash: running it printed no $BASH_VERSION"
On the search path the same cut is silent rather than an error: the step ran
under the NEXT candidate instead, which on a Mac is Apple's bash 3.2 - the case
`scripts.bash` exists to let an author avoid.
The probe prints the marker alone on a line, so the lines are now read as they
arrive and only the unfinished last line is held, capped at its head. Nothing is
windowed, and no amount of output on either side reaches the answer: measured
PASS at 0, 4000, 4058, 4059, 5000 and 100000 characters, before and after, both
configured and on PATH. The existing wrapper case became two, one per shape.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Z6tHq3XazgZbrS4YT73xs
`removeExchange`'s `fs.rmSync(dir, { recursive: true, force: true })` and the
sweep's `fs.promises.rm(..., { recursive: true, force: true })` differ only in
being sync and async, so a cause that refused the removal refuses the sweep too.
A script that runs `chmod 500` on its own exchange directory leaves the note
"a later bash step sweeps it" and an `output.json` that is still in $TMPDIR
after the next step's sweep. The note now separates the two causes: one that
clears on its own is swept, and a mode the script put on the directory needs it
removed by hand.
Two lifecycle guarantees stated near here were also asserted by nothing -
deleting the throttle or the `await pendingSweep` left the whole suite green.
Both now have a case: the root is read at most once across four steps inside the
interval, and an abandoned directory is gone the moment `execute` resolves, with
the sweep's own `rm` slowed so that a step which did not wait for it could not
pass.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Z6tHq3XazgZbrS4YT73xs
`STRICT_UTF8`'s docstring names `toString("utf8")` as the thing it exists to
avoid - "silent, unequal to what the script wrote" - and `readReasonFile` did
exactly that, so the two exchange files had opposite policies for the same
bytes. `printf 'caf\351 unreachable' > "$ARGENT_REASON"; exit 3` reached the
report as `caf<U+FFFD> unreachable`, while the same two bytes in $ARGENT_OUTPUT
were refused. The reason now gets the parallel sentence.
`markReason` had the mirror-image problem. The BYTE read lands on a UTF-8
boundary through `utf8SafeCut`, but the CHARACTER cut after it counts UTF-16
units, so a cut between the halves of an astral character left a lone surrogate
at the end of the message: a reason of 9000 U+1F600 came back with
`isWellFormed()` false, carried through `JSON.stringify` as `\ud83d` and turned
into U+FFFD by any UTF-8 write of the report.
Both pinned in `flow-script-bash.test.ts`, and both cases fail against a runner
copy with the two changes reverted.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Z6tHq3XazgZbrS4YT73xs
`commitOutput` is the whole of a `.sh` document's validation - the document is
JSON text, so it never meets the runner's `walk` - and two values `walk` refuses
reached flow state unchecked.
A number JSON can spell and JavaScript cannot hold. `printf '{"n":1e999}'`
parses to `Infinity`, and the step passed carrying a value every later encode
turns into `null`, while the same `.mjs` document was refused with "output
numbers must be finite". Both interpreters now answer with that sentence.
Depth. The 1 MiB cap does not bound it - nested arrays cost two bytes a level -
and `renderOutput` in `flow-add-script.ts` is a bare `JSON.stringify` reached
AFTER the step has been appended to the flow file, so an over-deep document made
the recorder write the step and then die with an uncaught `RangeError`.
`MAX_OUTPUT_DEPTH` is 4096, measured to sit in the gap between two stack-derived
ceilings on Node 20, 22, 24 and 26: the runner's recursive `walk` gives out at
~3450-3925, so no `.mjs` document that reaches this file is newly refused, and
V8's `JSON.stringify` gives out at ~6100 up to Node 24. A number rather than a
try/catch around a trial encode, because Node 26 encodes any depth a 1 MiB
document can reach, and a verdict that follows the host's Node is one a flow
file cannot be written against.
`findOwnProtoKey` becomes `documentProblem`, one iterative pass for all three
rules. The deep-document case in `flow-script-protocol.test.ts` now runs at 4000
- still past the recursive-walk ceiling it was written for - and a second case
pins that a 20 000-deep document from a mismatched runner is a verdict, not a
throw.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Z6tHq3XazgZbrS4YT73xs
Both hard-coded the bash 5.x verdict, so on a stock Mac - Apple's `/bin/bash`
3.2.57, the interpreter the POSIX fallback list exists for - the file was 65/67
with `expected undefined to be 'exit'`, which reads as a source regression. No
job sees it: unit tests run on ubuntu, the macOS jobs run no unit tests, and the
Windows job runs Git bash 5.x.
They now assert the way their SIGQUIT sibling already does. The single case
takes the document when the step passes and the `kill 0` guidance when it fails;
the repeated case asserts ONE verdict over 120 runs and never `signal`, which is
the race it was written for rather than the name of the winner.
Verified under both: `PATH=/usr/bin:/bin` (3.2.57) 67/67, ordinary PATH
(5.2.37) 67/67. Both still fail on mutation - dropping the held-signal branch
gives `kind: signal`, and reporting half of them as `signal` gives
`kinds: ["exit","signal"]`.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Z6tHq3XazgZbrS4YT73xs
`scopes: ["global"]` makes `readScopeValue` return before a project
`.argent/config.json` is read, so the mixed-OS-team case four comments give as
their reason cannot happen through that file any more. The reason that survives
is a global file that travels: a home directory restored onto a machine of
another family carries a spelling that host refuses.
- `flow-script-interpreter.ts` said the filesystem checks are read-side because
a project file is shared by hosts that do not all have it.
- `config-schema.ts` said `scripts.bash` "takes both scopes", 45 lines above
`scopes: ["global"]`.
- `config-access.test.ts` said "the key lands in a project file a mixed-OS team
shares".
- `flow-script-interpreter.test.ts` said "The resolver reads `scripts.bash` from
BOTH scopes", where the same file says the global scope alone at :67 and in
its describe title; it also carried the last "this PR" in the diff.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Z6tHq3XazgZbrS4YT73xs
`scripts.bash` was the one key in `CONFIG_SCHEMA` taking project scope while
being host-specific. Its `expected` requires the value be "spelled the way the
host running the tool server spells one", and `asAbsolutePath` enforces that
against `process.platform` - so a committed `.argent/config.json` could not
satisfy both for a team that is not all on one OS family.
On macOS, with a project config a Windows teammate committed
(`{"scripts":{"bash":"C:\\Program Files\\Git\\bin\\bash.exe"}}`), through
`argent flow run`:
✗ 1 script prove.sh — The configured bash (scripts.bash = C:\Program
Files\Git\bin\bash.exe, from …/.argent/config.json) is not an absolute
path. Point scripts.bash at a bash executable, or unset it to use the one
on this host's PATH.
Every `.sh` step errored, and there was no PATH fallback because the value was
set. `merge: "prioritize-local"` made it worse: the committed value shadowed
the developer's own valid global pin, and the message then named the working
bash it was declining to use. Editing the shared file was the only local fix,
and it un-fixed the teammate who committed it - `config set scripts.bash
'C:\...' --scope project` is refused on macOS, and the POSIX spelling is
refused the same way on Windows, so neither developer could write the other's
value into the file they share.
The key now takes the global scope alone, like `scripts.maxTimeoutMs` and
`scripts.heapLimitMb` above it. `readScopeValue` gates READS on `scopes` as
well as writes, so a committed value is not merely unwritable - it is unread,
and the resolver falls through to its PATH search rather than refusing the
step. Each developer pins their own bash.
That leaves one scope for the resolver to read, so the machinery that existed
to serve two goes with it: `resolveBashInterpreter` no longer takes an anchor
it cannot use, `configuredSource` and `unsetAdvice` are gone, and the message
names the global file and offers PATH - advice that is now always true.
Four behaviours confirmed through `argent flow run`, on an isolated
tool-server and home so the shared ones were untouched:
committed project value PASS (ignored, falls through to PATH)
wrong global pin errors, names the global file, no PATH fallback
good global pin honoured
set --scope project "cannot be set at project scope (allowed: global)"
Full tool-server suite 6277 passed, matching the pre-change baseline;
configuration-core 152; argent-cli 615; lint, knip and the docs build clean.
`commitOutput` reported a parse failure as `The script's output did not parse:
${errorMessage(err)}`, and V8's `SyntaxError` embeds about ten characters of
the offending document verbatim, mid-sentence:
The script's output did not parse: Unexpected token 's',
"{"auth":s3cr3t-tok"... is not valid JSON
On the `.mjs` path that branch is not reachable by ordinary script behaviour -
the runner's `encodeOutput` always emits valid JSON. On the `.sh` path any
script that writes non-JSON to `$ARGENT_OUTPUT` lands there, and what it wrote
is script-controlled text, so this branch adds the reachability.
The excerpt also defeats the redaction that runs afterwards.
`scrubSecretValues` matches whole values, so the prefix of a secret the
excerpt cut in half matches nothing, and `redactTruncated` repairs a cut
marked at the END of the message - by the runner - not one V8 made in the
middle of it. The same secret on the `.mjs` path comes back
`{"auth":"{{secret:API_KEY}}"}`.
Every double-quoted run is dropped from the message rather than the exact
sentence matched: the wording is V8's and is not a stability contract, but the
quoting is where a document's own bytes are, and the rest of that family
(`… at position 6`, `Unexpected end of JSON input`) quotes only with
apostrophes and survives unchanged. The author has the file; a ten-character
excerpt of it was worth nothing to them.
Reproduced through `argent flow run`: before, the message carried
`s3cr3t-tok`; after, `Unexpected token 's' is not valid JSON`.
`resolveBashInterpreter` was awaited before `deadlineAt` and the timeout were
armed, and with no signal plumbed into it. Each candidate costs up to
`BASH_PROBE_TIMEOUT_MS` (5 s) plus `BASH_PROBE_FORCE_GRACE_MS` (1 s), and the
resolver is deliberately not memoized - so the step's declared time limit
bounded nothing until after the probe, and an abort was not observed for about
six seconds per step. A flow of N bash steps was un-cancellable for roughly 6N
seconds, which matters against a 30 s MCP client budget.
The request's signal now reaches the probe: an abort stops the candidate at
once and the resolver answers `cancelled`, which the executor reports as the
cancellation it is rather than as a bash that would not answer.
`askForBashVersion` also stops the candidate's whole group on POSIX rather
than the candidate alone. A shim that backgrounded a job left it re-parented
to pid 1 and running after `execute()` returned - the module docstring says
this lookup "costs a few milliseconds against a step that already starts a
process", which is exactly the case it did not cover.
Measured with `#!/bin/sh` + `trap '' TERM` + `sleep 297 &` + `sleep 300` as
the configured bash, aborting at 300 ms: before, `execute()` returned after
6004 ms and the `sleep 297` was still running; after, 302 ms with the
cancelled verdict and nothing left behind. The probe-timeout path leaves
nothing behind either.
`sweepStaleExchanges` ran a `readdirSync` on the tool server's main thread -
it is called synchronously from `createExchange`, which `runOne` calls on the
bash path - and in production its root is `os.tmpdir()`, a directory shared
with every other process on the host and bounded by nothing here.
The 60 s throttle bounds how often the read happens, not what it costs. On
this machine a root holding 60 000 entries cost 33 ms of blocked loop per
sweep: no MCP request, device socket or timer ran during it, and the step's
own wall time went from 53 ms to 89 ms. The stall grows over a machine's life,
since the directory it reads is one the tool server never prunes.
Awaiting a `readdir` is not the fix - the read is scheduled off-thread either
way, and it is building the array of every name that blocks. The sweep now
walks a directory handle in batches of 64, which hands back a little work per
turn, and it runs beside the step rather than in front of it; `runOne` waits
for it at the end, so a step never outlives its own sweep.
Measured with the same rig, one bash step against a 60 000-entry root:
before step 89 ms, worst blocked loop 33 ms
after step 71 ms, worst blocked loop 0 ms
(an empty root is 53 ms / 1 ms either way)
Pinned by a case that plants 40 000 entries and watches a 5 ms heartbeat:
21-22 ms of block before, 0 after.
Removing the `Math.ceil` left all 2165 tests in `packages/tool-server/test/
flows/` green, so a future simplification there was silent in CI.
The fractional value really reaches that line: `flow-script-step-parse.test.ts`
pins `timeout: 1500.5` as a legal step, and `clampTimeout` returns a wanted
value at or under `maxTimeoutMs` un-rounded. What the guard prevents is what
the comment above it already describes - the directory is named
`argent-flow-script-<n>.5-XXXXXX`, `exchangeOwnedUntil`'s `/^(\d+)-/` cannot
match it, the sweep passes over it forever, and an exchange whose owner could
not remove it keeps an `output.json` that may hold values derived from a
secret under the shared `os.tmpdir()`.
The new case runs a step with `timeoutMs: 1500.5` and has the script report
its own exchange directory - otherwise removed before anything outside the
executor could look at it - then reads the stamp back the way the sweep does.
Removing the `Math.ceil` now fails it.
The fallback the five-line comment above it exists for was pinned by nothing:
replacing the whole expression with `scriptInterpreter(canonical)` left the
entire flow-script suite green, and no test in it used a symlinked script path
in either direction.
The case is reachable. With `scripts/seed.sh` a symlink to an extensionless
`tools/seed`, `resolveFlowRelativeFile` realpaths it, `hasScriptExtension` is
false, and only the step's own spelling can answer - so the regression the
fallback guards against is a passing bash step turning into a `load` failure
with a stack pointing into a shell file.
Both directions are pinned now: a `.sh` link to an extensionless file runs
under bash, and a `.sh` link to a `.mjs` runs under node. The mutant fails the
first.
`GROUP_SIGNAL_SETTLE_MS` is 1000 ms and was weighed against exactly one other
number, the parent's 1500 ms stop grace. It was never weighed against the
step's own time limit, whose legal floor is `MIN_SCRIPT_TIMEOUT_MS` = 100 and
whose entire range up to about 1050 ms sat below that settle.
So when bash died from SIGTERM, SIGINT or SIGHUP without the same signal
reaching the runner, `whenGroupSignalHeld` parked for the full second; the
executor's timer fired first, sealed the interruption and discarded the
terminal message that was already correct, and `classifyOutcome` fell through
to `timedOut`. The step was reported as a time limit that was never exceeded,
about the one fact that explains the failure - and the `.mjs` twin does not
behave this way, because this wait is on the bash path alone. No step-level
`timeout` was needed to reach it: `scripts.maxTimeoutMs` alone did.
The request now carries the parent's own limit, and the wait takes what is
left of it minus the room the verdict needs to travel back. A limit too short
to spare any of it reports at once, which is what the signal alone would have
said.
Measured through the executor with `kill -s TERM $$; sleep 30`:
before {100,timeout,177ms} {500,timeout,562} {900,timeout,965} {1200,signal} {3000,signal}
after {100,signal,64ms} {500,signal,292} {900,signal,695} {1200,signal} {3000,signal}
The `kill 0` group case still classifies as `exit` at the default limit, where
the settle is what it was.
The docstring claimed "a mixed-ending script is the only kind that ever gets
here: a fully CRLF one dies at `set -euo pipefail\r` with exit 2". Neither
half holds.
A fully CRLF script dies there only if it HAS that line. Without one it runs
to completion and leaves exactly the stray sibling this function looks for:
$ printf 'echo hi > "$OUT"\r\n' > noexit.sh
$ OUT=out2 /opt/homebrew/bin/bash noexit.sh ; echo "exit=$?"
exit=0
$ ls -b .
out2\r
The exit code is the bash's own too: the same `set -euo pipefail\r` script
exits 2 under GNU bash 5.3 (`set: pipefail: invalid option name`) and 1 under
the Apple bash 3.2 the docs point macOS users at.
The comment now says both, and a wholly CRLF script that runs to the end is
pinned beside the mixed-ending one.
Hubert Gancarczyk