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 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 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.
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.
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
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
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
`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.
`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.
The probe's `spawn` passed no `env`, so the candidate inherited the tool
server's whole environment. The bash that actually runs the step does not:
`buildChildEnv` copies only `ALLOWED_ENV_NAMES`. The divergence ran both ways.
`BASH_ENV` is the one variable that changes what a non-interactive `bash -c`
does, and it is deliberately outside that allowlist. So a host that exported
it had every `.sh` step refused before the fork, reporting that no bash exists
- on a host with three - and the remedy the refusal names goes through the
same probe, which answered "is not a bash" about a bash 5.3 that answers
correctly when asked without that variable.
In the other direction the candidate is an arbitrary executable named `bash`,
and inheriting handed it `ARGENT_AUTH_TOKEN`, `ARGENT_PORT` and every
`ARGENT_SECRET_*` value - the three things the comment above
`ALLOWED_ENV_NAMES` says the allowlist exists to keep out of a script's reach.
The resolver now takes the environment to probe in, and the executor passes
the step's own. The test helper that gates the bash suites passes the same one
through `buildChildEnv`, which is now exported for it.
Reproduced through `argent flow run` with a shim first on the resolver's path
that records what it was given. Before: BASH_ENV set errored every step with
"No bash was found on this host", and the probe run saw 3 argent
token/port/secret names against the step run's 0. After: the step passes with
BASH_ENV set, and both runs see 0.
"or unset it to use the one on this host's PATH" was true only where exactly
one scope held a value. The key takes both with `merge: "prioritize-local"`,
so unsetting the file the message had just named falls through to the OTHER
scope's value, not to PATH.
With a project `.argent/config.json` of `/bin/ls` over a global one of
`/bin/bash`, following the printed advice silently swapped the interpreter to
the global value on a host whose PATH bash is a different one - and a step
that then fails does not mention `scripts.bash` at all, so nothing points back
at the value now in force, which is the situation this key exists to make
visible.
The refusal now reads "unset it there to fall back to /bin/bash, which
<global file> sets - unset it in both files to use the one on this host's
PATH" when a value is behind it, and is unchanged when none is. Reproduced
through `argent flow run` with exactly those two files.
The refusal explained a leading backslash, and the values that reach it most
often begin with a forward slash: `path.win32.isAbsolute("/usr/bin/bash")` is
true, so a POSIX path passes the check above and lands here. The comment on
`WINDOWS_ROOTED_PATH_RE` names exactly that case as the reason the write gate
exists - `argent config set` stored a POSIX path that every `.sh` step then
refused - so it is the expected input to this branch, not an edge case.
The sentence now says "begins with a slash or a backslash", which is the same
mechanism for both: on Windows either spelling is rooted on whatever drive the
process is on. Pinned by both spellings.
Both calls in the search loop computed a sentence and both were used as
predicates and `continue`d past. When every candidate was rejected the caller
got `notFoundMessage()`, which is written for the case where nothing exists:
it lists where the executor looked and says "Install bash". So on a host that
HAS a bash which fails the probe, the step's reason was the opposite of what
happened - `which bash` answers on that host, and the reason that would name
the real problem existed inside the resolver and was dropped.
The reasons are kept now, and the message names them: "No bash this host
offers could run the script: the executor looked at …, and refused what it
found - /usr/local/bin/bash is not a bash: running it printed no
$BASH_VERSION". A candidate a host simply lacks is left out, and so is the
WSL launcher, which the message already names - only a file that is really
there earns a line.
Pinned by a case that puts a stub on PATH and takes the fixed locations away:
the message names the stub and no longer says "Install bash". The "nothing
anywhere" message is unchanged and still pinned by the two cases beside it.
This one is not reachable through `argent flow run` on a host that has a real
bash at a fixed location, which is why the pin is the resolver itself.
The signal branch of `notBashProblem` fired on ANY signal death of the probed
candidate but offered one explanation - "(the check waits 5 seconds, then
stops the candidate)". That is false whenever the candidate is not the one
Argent stopped: a wrapper that segfaults, one the kernel kills for its memory,
one that kills itself, all answer in milliseconds and got the same sentence.
`askForBashVersion` already recorded which signal it sent, so it now reports
whether the death was its own doing and the message follows: a candidate the
check stopped reads "did not answer … within 5 seconds, and was stopped with
SIGKILL"; one that died on its own reads "died from SIGSEGV", with no claim
about a wait.
Reproduced through `argent flow run` with `scripts.bash` at `#!/bin/sh` +
`kill -SEGV $$`: before, "was stopped by SIGSEGV (the check waits 5 seconds…)"
at 240 ms; after, "died from SIGSEGV". A candidate that really does hang still
reports the wait, verified against `trap "" TERM` + `sleep 300` at 6.0 s.
Dropping the `\S` from `BASH_PROBE_MARKER` left the whole
`test/flows/script/` suite green, and the version after the marker is the
whole of what separates bash from the shells that would run the file with
different word-splitting and array semantics - `notBashProblem`'s own
docstring says so, naming zsh, ksh and dash as answering with an empty
version.
What the guard prevents is not a loud failure: a non-bash accepted there runs
the file, writes nothing to `$ARGENT_OUTPUT`, and exits 0, so the parent
reads back the document it seeded and reports the step green.
The new case configures a shell that answers the probe the way those three do
- the marker and nothing after it - and expects the step to be refused.
Dropping the `\S` now fails it.
The probe's output cap was applied per chunk and was all-or-nothing, but the
string it protects sits at the END of the output: `BASH_PROBE_COMMAND` prints
the version line last, after whatever the candidate greeted with. Once one
chunk pushed the buffer past 4 KiB every later chunk was dropped whole -
including the one carrying `argent-bash-version:` - and the marker was then
tested against text it had been cut out of.
So a wrapper of the shape the leading newline in the probe command exists for
(`#!/bin/sh`, a banner, `exec /bin/bash "$@"`) was refused with a statement
that is false about it: it printed its version. The cut was exactly at 4096
characters of greeting, and the same wrapper one character shorter ran the
step. On the search path the same candidate was skipped silently instead.
The window is now on the tail, where the answer is, and is bounded the same
way. Reproduced through `argent flow run` with `scripts.bash` at such a
wrapper: before, `is not a bash: running it printed no $BASH_VERSION`; after,
the step passes. Pinned by a case whose wrapper greets with 64 KiB.
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