The tool backgrounded EVERY long action and exposed an inverted `foreground`
flag. dsh-tool-bash does neither: its parameter is `run_in_background`, it
defaults to foreground, and the model opts in. That difference is what made a
finished job go uncollected.
The evidence, from this session's own log: the only slopo job that ever ran was
slopo-1, and the agent KILLED it — `kill()` sets `reported = true`, which
suppresses the completion notice by design, because a caller who asked to stop
something does not want to be told it stopped. The agent then worked around the
tool entirely and re-ran slopo through bash, which is where the six notices in
that session came from. Nothing was broken; the tool was asking the caller to
depend on a best-effort signal.
So the query actions return their report directly. Only `prepare` — index plus
embed, the genuinely long and watchable step, and the one the user asked to be
able to watch — backgrounds by default. Any action can still be backgrounded
with `run_in_background: true`.
The render now names the collect call explicitly (`job_output` for that id, with
`wait: true` when blocked on it) instead of the vaguer "read the job output when
it settles", so a caller that is not woken still has an unambiguous next step.
I checked the waiter path while I was in there: a timed-out `wait` decrements
`waiters` in a `finally`, so it does not leak and does not suppress the notice.
That was my first hypothesis and it is wrong — worth recording, since it was the
plausible one.
A review on a real repository indexed 113,768 code units and began an embed
that would have run for about eight hours. Two independent causes, both mine:
1. Nothing excluded anything. Slopo does NOT read .gitignore, so a repo with
node_modules in its ignore file still had all 21,104 files under it parsed -
13,091 of them source. The generated config carried no source_dir_exclude at
all, so the tool reviewed the dependencies instead of the code.
2. Nothing bounded the work. Slopo's own >1000 guard is useless here: it
measures the PRE-EXISTING backlog before indexing, and a fresh database has
no units at all. The guard that exists to protect an agent's latency is
silent in the one case that most needs it.
Measured on that repository:
no excludes 113,768 units ~7.9 h
default excludes 8,607 units ~36 min
default + a server-only scope 1,519 units ~6 min
So: default excludes for vendored and generated directories, ADDED to the
user's own list rather than chosen between (a settings patch replaces a whole
value, so a single sourceDirExclude pattern would otherwise re-enable the 13x
blow-up). And every action now indexes first, checks the backlog against
maxIndexUnits (default 5,000), and refuses with the count, the estimate and the
lever rather than starting the work.
The count comes from slopo's own database via node:sqlite - no new dependency,
and it degrades to the summary line on an older runtime or a locked file. That
matters because `Indexed N` reports only what was NEWLY indexed: on a warm
database it reads zero while the backlog is still full, which is exactly the
state a refusal leaves behind. Caught that hole by running the thing against the
real repository rather than trusting the first version.
Also fixes `status` reporting when the endpoint probe succeeded - the fix from
the previous commit is what made this run reachable at all.
The probe returned `error: undefined` on a dimension match. `undefined` is not a
JSON value -- JSON.stringify silently drops it from an object -- so the shape
looked harmless and was not: the harness enforces the declared output schema
against the canonical value before materialising it, and an explicit undefined
property fails that check. A perfectly healthy status call could not be returned
at all.
Fixed at the boundary rather than at each site. Every return now funnels through
`toJsonValue`, which drops undefined properties exactly as JSON.stringify would,
maps non-finite numbers to null (same reasoning: JSON.stringify(NaN) is "null"),
turns an undefined array slot into null rather than shifting indices, and refuses
a circular value instead of recursing forever. `update` was carrying
`errorCode: undefined` and `ensureSlopo` a `detail: undefined` on the same path,
so a per-site fix would have left two more of these waiting.
The probe also now omits `error` rather than assigning undefined, since it is a
plausible thing for another consumer to read.
Four regression tests, and they were checked against a negative control: with the
sanitiser reverted and the undefined restored, exactly the "probe SUCCEEDS" test
fails with "does not survive a JSON round trip" and the others still pass, which
is what makes it a test of this bug rather than of the JSON round trip in
general.
Slopo reports a bad embedding configuration as the two words "Embedding failed."
on stdout, with the real reason in its agent log. That is a slow way to learn
that a model string has the wrong provider prefix, or that the dimensions do not
match what the model actually returns — the second of which is expensive, because
slopo freezes the dimensions into the index at first use.
`status` now does three things instead of one: it reports the resolved settings,
it runs `slopo --version` through the shell seam, and it makes one tiny embedding
request against the configured endpoint and compares the returned width with the
configured one. Verified against a real Ollama: a correct config reports ok, a
dimension mismatch names both numbers, and a wrong model returns the endpoint's
own 404 rather than a silent failure minutes into an embed.
The probe is a direct fetch rather than a shell call, because it is one small
HTTP request and does not belong behind a boundary that exists to confine
filesystem writes.
Two smaller things this needed:
- the sandbox policy is now resolved once at the top of execute, so `status` can
use it without a second resolution
- provider-prefix stripping is an allow-list, not "drop the first segment".
`unclemusclez/jina-embeddings-v2-base-code:latest` is a real model name with no
prefix at all, and an unknown first segment is left alone so a wrong guess is
visible in the reported `sentModel` rather than silent
dsh-bash-sandbox is the composed ctx.shell, and its resolve() stamps
`request.sandboxPolicy ?? ctx.sandboxPolicy.resolve()`. Omitting the policy
therefore does NOT mean "unconfined" -- it silently substitutes the DEPLOYMENT
default (`DSH_PERMISSION_MODE ?? 'workspace-write'`) for the session's preset.
On a host with no sandbox runner that confines a danger-full-access session and
fails with SandboxUnavailableError before slopo is ever reached, which is
exactly what happened: the tool was unusable while bash worked.
Resolve the calling session's policy the way dsh-tool-bash does, and pass it on
every call. A confining executor with no ctx.sandboxPolicy is now refused
outright rather than guessed at.
That fix exposes a second problem worth naming. Slopo's state lives under
$DSH_HOME, which is outside the workspace root a workspace-write sandbox permits
writing to, so under that preset nothing would work. The tool now refuses before
running and names both ways out -- run under danger-full-access, or set stateDir
to a directory inside the workspace -- instead of letting a write fail halfway
through an embed.
Five tests pin this: the session's policy is resolved exactly once and travels
with the session, a missing policy service is refused, state outside the sandbox
is refused before any execution, state inside it proceeds, and status/configure
never touch the shell at all.
The three skills are ports of slopo's own agent configs, published
user-invocable and NOT model-invocable. Slopo makes that choice upstream and the
reason holds here: every one of these commands can spend minutes embedding, so a
model that decides to "check for duplication" mid-task can do it repeatedly.
settings goes through ctx.inject(['settings'], ...) rather than the top-level
inject, so a profile without a settings provider still gets the tool and the
skills and degrades to the composition config. A required dependency would have
suppressed apply entirely -- a silent whole-plugin failure for an optional
capability.
The tool resolves the CLI, materialises the config, and runs the long actions as
background jobs. Background is the right default because embedding takes minutes
and slopo writes progress to a log file rather than stdout, so there is nothing
to stream; the job handle carries the log path instead.
Nothing is defaulted. Guessing an embedding backend would mean guessing where
money is spent, so status says what is missing and configure sets it.