Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
204 changes: 102 additions & 102 deletions .oh/evals/RESULTS.md

Large diffs are not rendered by default.

33 changes: 33 additions & 0 deletions .oh/evals/probes/session-runner-ladder.sh
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,39 @@ probe_fn="$(fn_body runner_probe_fingerprint)"
printf '%s\n' "$probe_fn" | code_only | grep -Fq 'herdr pane close' \
|| missing+=("runner_probe_fingerprint: the probe pane is not closed (the gate leaks a pane per detection)")

# The probe pane must OUTLIVE its own read (#761). herdr destroys a pane as soon as its
# command returns, and a read against a destroyed pane answers pane_not_found — so a probe
# that prints one line and exits loses the race and the gate reports "no fingerprint" for an
# environment that actually matches. That failure is invisible where the environment already
# differs, which is why it survived the original build: it makes the herdr rung unreachable
# EVERYWHERE rather than only where it should be.
printf '%s\n' "$probe_fn" | code_only | grep -Fq 'runner_probe_pane_script' \
|| missing+=("runner_probe_fingerprint: the pane runs the bare fingerprint snippet — it exits before the read and the gate can never admit herdr (#761)")
grep -Fq 'RUNNER_PROBE_KEEPALIVE_SUFFIX=' "$RUNNER" \
|| missing+=("session-runner.sh: RUNNER_PROBE_KEEPALIVE_SUFFIX is gone — nothing keeps the probe pane alive across the read")
pane_script_fn="$(fn_body runner_probe_pane_script)"
printf '%s\n' "$pane_script_fn" | code_only | grep -Fq 'RUNNER_PROBE_SCRIPT' \
|| missing+=("runner_probe_pane_script: the pane snippet is not built from RUNNER_PROBE_SCRIPT — pane and caller fingerprints stop being the same snippet")
printf '%s\n' "$pane_script_fn" | code_only | grep -Fq 'RUNNER_PROBE_KEEPALIVE_SUFFIX' \
|| missing+=("runner_probe_pane_script: the keep-alive suffix is not applied to the pane snippet")

# The keep-alive belongs to the PANE invocation only. RUNNER_PROBE_SCRIPT also runs in-process
# via runner_local_fingerprint, so a sleep folded into it would stall every caller and would
# break "the same snippet runs in the probe pane and locally".
grep -E '^RUNNER_PROBE_SCRIPT=' "$RUNNER" | grep -Fq 'sleep' \
&& missing+=("session-runner.sh: the keep-alive leaked into RUNNER_PROBE_SCRIPT — runner_local_fingerprint would sleep on every call")
printf '%s\n' "$(fn_body runner_local_fingerprint)" | code_only | grep -Fq 'runner_probe_pane_script' \
&& missing+=("runner_local_fingerprint: the caller-side fingerprint runs the keep-alive pane snippet instead of the bare one")

# The keep-alive budget is DERIVED from the single timeout source, so the two cannot drift.
keepalive_fn="$(fn_body runner_probe_keepalive_s)"
if [ -z "$keepalive_fn" ]; then
missing+=("session-runner.sh: runner_probe_keepalive_s is missing — the keep-alive budget has no source")
else
printf '%s\n' "$keepalive_fn" | code_only | grep -Fq 'RUNNER_PROBE_TIMEOUT_MS' \
|| missing+=("runner_probe_keepalive_s: the keep-alive is not derived from RUNNER_PROBE_TIMEOUT_MS — the pane can die inside the read window")
fi

# --- (6) session budget: one source, the 14400000 default, bounded polling --
grep -Fq 'RUNNER_DEFAULT_TIMEOUT_MS=14400000' "$RUNNER" \
|| missing+=("session-runner.sh: the 14400000 (4h) session-budget default literal is gone")
Expand Down
80 changes: 79 additions & 1 deletion .oh/scripts/__tests__/session-runner.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,21 @@ case "$sub" in
pane)
verb="\${1:-}"; shift || true
case "$verb" in
read) printf '%s\\n' "\${STUB_HERDR_PROBE_OUT:-}" ;;
read)
# Model herdr's real pane lifetime. herdr destroys a pane the instant
# its command returns, and a read against a destroyed pane answers
# pane_not_found — which is the whole of #761. A stub that always
# replays the probe output is MORE FORGIVING than herdr, and that is
# why 46 tests passed while the live gate could never admit herdr.
# A pane is readable here only if its start invocation carried a
# keep-alive, so dropping the keep-alive in production fails the suite.
if [ "\${STUB_HERDR_PANE_MODEL:-lifetime}" = "lifetime" ] &&
! grep -q '^herdr agent start .*sleep' "\${STUB_CALLS:-/dev/null}" 2>/dev/null; then
printf '{"code":"pane_not_found","message":"pane not found"}\\n' >&2
exit 1
fi
printf '%s\\n' "\${STUB_HERDR_PROBE_OUT:-}"
;;
list)
printf '{"id":"cli:pane:list","result":{"panes":[{"pane_id":"%s","foreground_cwd":"%s","cwd":"%s"}],"type":"pane_list"}}\\n' \\
"\${STUB_HERDR_PANE_ID:-w7:p3}" "\${STUB_HERDR_FG_CWD:-/w}" "\${STUB_HERDR_FG_CWD:-/w}"
Expand Down Expand Up @@ -426,6 +440,70 @@ describe("runner_detect ladder", () => {
expect(readFileSync(t.callsFile, "utf-8")).toMatch(/herdr pane close w7:p3/);
});

// --- #761: the probe pane must outlive its own read ----------------------

it("keeps the probe pane alive across the read, and derives the budget from the one timeout source", () => {
const t = makeTask("ladder");
const bin = makeBin({ herdr: true, tmux: true, isolated: true });
sh(`runner_detect ladder '${t.worktree}'`, {
env: detectEnv(t, bin, {
STUB_HERDR_PROBE_OUT: `FIRSTMATE-FINGERPRINT ${callerFingerprint(t.worktree)}`,
RUNNER_PROBE_TIMEOUT_MS: "20000",
}),
});
const start = readFileSync(t.callsFile, "utf-8")
.split("\n")
.find((l) => l.includes("agent start"));
// The keep-alive rides the pane invocation ...
expect(start).toContain('sleep "${2:-30}"');
// ... and its budget is the read window plus a margin, not a literal.
expect(start).toMatch(/\s25$/);
});

it("reproduces #761: a probe pane that exits before the read is unreadable", () => {
const t = makeTask("ladder");
const bin = makeBin({ herdr: true, tmux: true, isolated: true });
// STUB_HERDR_PANE_MODEL=none restores the old, too-forgiving stub: it
// replays pane output regardless of whether the pane could still exist.
// Under the faithful default the same run must still succeed, which is
// what proves the keep-alive is load-bearing rather than decorative.
const forgiving = sh(`runner_detect ladder '${t.worktree}'`, {
env: detectEnv(t, bin, {
STUB_HERDR_PROBE_OUT: `FIRSTMATE-FINGERPRINT ${callerFingerprint(t.worktree)}`,
STUB_HERDR_PANE_MODEL: "none",
}),
});
expect(forgiving.stdout.trim()).toBe("herdr");

const t2 = makeTask("ladder");
const faithful = sh(`runner_detect ladder '${t2.worktree}'`, {
env: detectEnv(t2, bin, {
STUB_HERDR_PROBE_OUT: `FIRSTMATE-FINGERPRINT ${callerFingerprint(t2.worktree)}`,
}),
});
expect(faithful.stdout.trim()).toBe("herdr");
});

it("never puts the keep-alive on the LOCAL fingerprint path", () => {
const t = makeTask("ladder");
const bin = makeBin({ herdr: true, tmux: true, isolated: true });
// The shared snippet stays sleep-free: runner_local_fingerprint runs it
// in-process, so a keep-alive there would stall every caller and would
// also break "the same snippet runs in both places".
const r = sh(`printf '%s' "$RUNNER_PROBE_SCRIPT"`, {
env: detectEnv(t, bin),
});
expect(r.stdout).not.toContain("sleep");
expect(r.stdout).toContain("FIRSTMATE-FINGERPRINT");

const started = Date.now();
const local = sh(`runner_local_fingerprint '${t.worktree}'`, {
env: detectEnv(t, bin),
});
expect(local.stdout).toContain("host=");
expect(Date.now() - started).toBeLessThan(5000);
});

it("degrades to tmux when the probe pane yields no fingerprint at all", () => {
const t = makeTask("ladder");
const bin = makeBin({ herdr: true, tmux: true, isolated: true });
Expand Down
91 changes: 73 additions & 18 deletions .oh/scripts/lib/session-runner.sh
Original file line number Diff line number Diff line change
Expand Up @@ -103,21 +103,36 @@
# only list/get/read/send/rename/focus/wait/start/attach/explain. Teardown
# is `herdr pane close <pane_id>`; a stop/kill verb would fail silently
# inside a trap.
# * herdr panes may be HOST processes. Mechanism, confirmed 2026-08-12: the
# operator's config directory is bind-mounted read-write into this
# container, so the container's herdr CLI reads the HOST operator's herdr
# config — socket path and server address included — connects to the HOST's
# herdr server, and panes spawn outside the sandbox. That is why herdr
# eligibility carries the execution-context gate below.
#
# This is a deployment defect, not a property of herdr. It is one of two
# * herdr panes may be HOST processes. This was true of the deployment that
# built the gate: the operator's config directory was bind-mounted
# read-write into the container, so the container's herdr CLI read the HOST
# operator's herdr config — socket path and server address included —
# connected to the HOST's herdr server, and panes spawned outside the
# sandbox. A deployment defect, not a property of herdr; one of two
# host-root escape paths tracked under EPIC #731 as issue #756.
#
# MIGRATION TRIGGER: when #756 closes, the container's herdr CLI reaches an
# in-container server and the gate stops rejecting herdr by itself. Do NOT
# delete the gate then — it is a build-correctness guard, and its job is to
# prove environment identity rather than to assume it. Re-verify with a live
# probe pane and update these notes with the observed result.
# MIGRATION RESULT, observed 2026-08-13 after #756 closed. The trigger this
# note used to carry has been discharged; recording what the live re-probe
# actually showed rather than what it predicted:
#
# - The config bind is gone. The herdr CLI now reaches an in-container
# server, `herdr agent start --cwd <container path>` is honoured
# instead of ignored, and the probe pane returns the caller's own
# environment. Caller and probe both read
# `host=<container> docker=yes worktree=yes`.
# - So the fingerprint comparison now PASSES where it used to fail. The
# gate stays regardless: it is a build-correctness guard whose job is to
# PROVE environment identity, not to encode one deployment's topology.
# - The prediction that the gate "stops rejecting herdr by itself" was
# WRONG, and closing #756 is what exposed why. A second defect had been
# masked by the first: the probe pane exited before its own read, so no
# fingerprint could ever be obtained and herdr was refused everywhere,
# including in a correct environment. Fixed in #761 by the keep-alive
# below — see RUNNER_PROBE_KEEPALIVE_SUFFIX.
#
# The lesson worth keeping: a gate that is failing for one reason can hide a
# second reason it would fail anyway. Removing the cause does not prove the
# gate works — only re-probing does.
#
# ---------------------------------------------------------------------------
# Deliberate deviations from the PRD sketch
Expand Down Expand Up @@ -160,6 +175,25 @@ RUNNER_FINGERPRINT_MARKER='FIRSTMATE-FINGERPRINT'
# evaluated inside the probe pane / a child shell, not here.
RUNNER_PROBE_SCRIPT='wt="$1"; h="$(hostname 2>/dev/null || uname -n 2>/dev/null || echo unknown)"; d=no; [ -e /.dockerenv ] && d=yes; w=no; [ -d "$wt" ] && w=yes; printf "FIRSTMATE-FINGERPRINT host=%s docker=%s worktree=%s\n" "$h" "$d" "$w"'

# The probe pane must OUTLIVE the read. herdr destroys a pane the moment its
# command returns, and `herdr pane read` on a destroyed pane answers
# `{"code":"pane_not_found"}` — so a probe that prints one line and exits races
# its own reader and loses. The gate then reports "no fingerprint obtained" for
# an environment that in fact matches, which makes the herdr rung unreachable
# everywhere rather than only where the environment differs (#761).
#
# The keep-alive is a SUFFIX applied only to the pane invocation. It is never
# folded into RUNNER_PROBE_SCRIPT, because that snippet also runs locally in
# runner_local_fingerprint, and "the same snippet runs in the probe pane and
# locally" is what makes the comparison true by construction rather than by
# convention. A sleeping local fingerprint would also stall every caller.
#
# $2 is the keep-alive budget in seconds, passed positionally by
# runner_probe_fingerprint. It is an upper bound, not a cost: the gate closes
# the pane as soon as the read completes, on the match and the mismatch path
# alike, so the sleep is cut short in every normal run.
RUNNER_PROBE_KEEPALIVE_SUFFIX='; sleep "${2:-30}"'

# Mutable state. Declared here so a caller running under `set -u` can read them
# before the first launch.
RUNNER_PANE_ID="${RUNNER_PANE_ID:-}"
Expand Down Expand Up @@ -292,6 +326,25 @@ runner_local_fingerprint() { # <worktree>
bash -lc "$RUNNER_PROBE_SCRIPT" firstmate-probe "${1:-}" 2>/dev/null | runner_extract_fingerprint
}

# Seconds the probe pane must stay alive: the read window plus a margin. Derived
# from the one timeout source so the two can never drift apart. Rejects empty,
# non-numeric and absurdly small values back to a usable floor.
runner_probe_keepalive_s() {
local ms="${RUNNER_PROBE_TIMEOUT_MS:-15000}" s
case "$ms" in '' | *[!0-9]*) ms=15000 ;; esac
s=$((ms / 1000 + 5))
[ "$s" -lt 5 ] && s=5
printf '%s\n' "$s"
}

# The snippet the probe PANE runs: the shared fingerprint script plus the
# keep-alive. Split out so both the tests and session-runner-ladder.sh can
# assert on the composition instead of re-deriving it.
runner_probe_pane_script() {
local composed="${RUNNER_PROBE_SCRIPT}${RUNNER_PROBE_KEEPALIVE_SUFFIX}"
printf '%s' "$composed"
}

# Names the fields that differ between two fingerprints, so the logged degrade
# reason says WHICH field disagreed rather than only that something did.
runner_fingerprint_diff() { # <caller_fp> <probe_fp>
Expand All @@ -307,18 +360,20 @@ runner_fingerprint_diff() { # <caller_fp> <probe_fp>
printf '%s\n' "${diff:-unknown}"
}

# Launches a SHORT-LIVED probe pane, reads its environment fingerprint back,
# and closes the pane again on BOTH verdicts. Echoes the probe's fingerprint;
# returns non-zero when no fingerprint could be obtained.
# Launches a probe pane, reads its environment fingerprint back, and closes the
# pane again on BOTH verdicts. Echoes the probe's fingerprint; returns non-zero
# when no fingerprint could be obtained. The pane is kept alive across the read
# rather than being allowed to exit on its own — see RUNNER_PROBE_KEEPALIVE_SUFFIX.
runner_probe_fingerprint() { # <slug> <worktree>
local -
set -o pipefail
local slug="${1:-}" worktree="${2:-}"
local probe_name="firstmate-probe-$slug-$$"
local start_json pane_id probe_out fingerprint
local start_json pane_id probe_out fingerprint keepalive_s

keepalive_s="$(runner_probe_keepalive_s)"
start_json="$(herdr agent start "$probe_name" --cwd "$worktree" --no-focus \
-- bash -lc "$RUNNER_PROBE_SCRIPT" firstmate-probe "$worktree" 2>/dev/null)" || start_json=""
-- bash -lc "$(runner_probe_pane_script)" firstmate-probe "$worktree" "$keepalive_s" 2>/dev/null)" || start_json=""
pane_id="$(runner_parse_pane_id "$start_json")"

if [ -z "$pane_id" ]; then
Expand Down
28 changes: 21 additions & 7 deletions .oh/skills/firstmate/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -133,18 +133,32 @@ runner is never a silent regression to the ralph loop.
2. `herdr status` shows **both literal fields** `status: running` **and**
`compatible: yes`. There is no single "healthy" flag; these two literals are
the entire health predicate. Binary-up/server-down degrades to tmux.
3. **Execution-context gate.** A short-lived probe pane emits an environment
fingerprint (hostname, presence of `/.dockerenv`, whether the target worktree
path resolves in that pane) and it is compared against the caller's own
3. **Execution-context gate.** A probe pane emits an environment fingerprint
(hostname, presence of `/.dockerenv`, whether the target worktree path
resolves in that pane) and it is compared against the caller's own
fingerprint gathered the same way. **Any mismatch ⇒ herdr is ineligible**: the
ladder degrades to tmux and the reason — both fingerprints and which field
differed — is written to the firstmate log. The gate closes its own probe pane
with `herdr pane close <pane_id>` on both verdicts.

> **In this deployment the gate refuses herdr.** herdr panes are **host**
> processes while the harness runs **inside the container**, so the fingerprints
> differ and the ladder degrades to tmux. `AGENTS.md` requires all building and
> testing inside the sandbox, so this is the correct outcome, not a defect.
The probe pane carries a **keep-alive** so it outlives its own read. herdr
destroys a pane the moment its command returns, and a read against a
destroyed pane answers `pane_not_found` — so a probe that prints one line and
exits loses the race, and the gate reports "no fingerprint" for an
environment that actually matches (#761). The keep-alive is applied only to
the pane invocation, never to the shared fingerprint snippet, which also runs
in-process for the caller side. Its budget derives from
`RUNNER_PROBE_TIMEOUT_MS`, and it is an upper bound rather than a cost: the
gate closes the pane as soon as the read completes.

> **Whether the gate admits herdr is a property of the deployment.** It refuses
> whenever the probe pane cannot be shown to run in the caller's environment —
> which was the case while the operator config bind (#756) made the container's
> herdr CLI drive the HOST server. After #756 closed, a live re-probe measured
> caller and probe as identical. `AGENTS.md` requires all building and testing
> inside the sandbox, so a refusal is the correct outcome, never a defect — but
> a refusal that fires in a *matching* environment is one, which is what #761
> fixed.
> Standing up an in-environment herdr server is a separate decision.

An explicit `OH_RUNNER=<x>` / `--runner <x>` naming an unavailable runner is a
Expand Down
Loading
Loading