From da3590c7b78df5b98b277b9e363deaf2c403856c Mon Sep 17 00:00:00 2001 From: Nikolai Emil Damm Date: Wed, 11 Nov 2026 15:01:32 +0100 Subject: [PATCH 1/5] feat(agentic-engineer): take a single-writer lease before a run touches memory kagent starts each day's run whether or not the last has finished, and a run has no time limit, so two runs can overlap and overwrite each other's memory. The run message now takes an exclusive WebDAV LOCK on the lane store's lease before it reads memory, refreshes it before it uploads (the refresh must name the run as the holder, since a store restart can reissue a token), and releases it last. A run refused the lease stops and uploads nothing, as the durable-memory guide requires. A MKCOL cannot be the lease: the store answers 201 for a folder that already exists. A new CI test runs the store's pinned rclone image with the Deployment's own arguments and proves the lease's guarantees on it. Refs devantler-tech/monorepo#4910 Co-Authored-By: Claude Opus 5.5 --- .github/workflows/ci.yaml | 11 + .../deployment-lane-store.yaml | 6 +- .../apps/agentic-engineer/job-schedule.yaml | 47 +++- .../test-agentic-engineer-lane-store-lease.sh | 223 ++++++++++++++++++ 4 files changed, 274 insertions(+), 13 deletions(-) create mode 100755 scripts/tests/test-agentic-engineer-lane-store-lease.sh diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index af0ca9c475..612aca5d6c 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -612,6 +612,7 @@ jobs: - 'scripts/tests/test-coroot-helper-image-pull-policy.sh' - 'scripts/tests/test-agentic-engineer-schedule.sh' - 'scripts/tests/test-agentic-engineer-lane-store.sh' + - 'scripts/tests/test-agentic-engineer-lane-store-lease.sh' - 'scripts/tests/test-kagent-claude-harness-wrapper.sh' - 'scripts/kagent-claude-harness/**' - 'scripts/tests/test-anthropic-proxy-envoy-config.sh' @@ -2886,6 +2887,16 @@ jobs: shellcheck scripts/tests/test-agentic-engineer-lane-store.sh bash scripts/tests/test-agentic-engineer-lane-store.sh + - name: 🔒 Validate the Agentic Engineer lane store lease + if: needs.changes.outputs.k8s == 'true' + # Runs the store's pinned rclone image with its own arguments: the lease a + # run takes before it touches memory refuses every other run, even many at + # once, releases only for its holder, and a store restart cannot pass one + # run's lease off as another's. + run: | + shellcheck scripts/tests/test-agentic-engineer-lane-store-lease.sh + bash scripts/tests/test-agentic-engineer-lane-store-lease.sh + - name: 🧠 Validate the kagent harness plugin wrapper if: needs.changes.outputs.k8s == 'true' # Runs the image's real `claude` wrapper against a stub Claude Code: a diff --git a/k8s/providers/hetzner/apps/agentic-engineer/deployment-lane-store.yaml b/k8s/providers/hetzner/apps/agentic-engineer/deployment-lane-store.yaml index 3eb088a49a..2d33e3e3e5 100644 --- a/k8s/providers/hetzner/apps/agentic-engineer/deployment-lane-store.yaml +++ b/k8s/providers/hetzner/apps/agentic-engineer/deployment-lane-store.yaml @@ -5,7 +5,11 @@ # # - webdav (8080): read and write, behind the lane-store credential. Only # Substrate's egress gateway reaches it, sending that credential in place of -# the agent's placeholder (egress-policy-lane-store.yaml). +# the agent's placeholder (egress-policy-lane-store.yaml). It also holds +# the lane's single-writer lease, a WebDAV LOCK on /lease kept only in its +# memory (devantler-tech/monorepo#4910). A run that dies before releasing +# the lease leaves it held, and every later run stops at it; restarting +# this Deployment frees it. # - reader (8081): read only, with no credential of its own. Only the # Kubernetes API server reaches it, so a reader needs the # services/proxy grant in role-lane-store-reader.yaml. diff --git a/k8s/providers/hetzner/apps/agentic-engineer/job-schedule.yaml b/k8s/providers/hetzner/apps/agentic-engineer/job-schedule.yaml index 5b830a172a..71cb08152b 100644 --- a/k8s/providers/hetzner/apps/agentic-engineer/job-schedule.yaml +++ b/k8s/providers/hetzner/apps/agentic-engineer/job-schedule.yaml @@ -97,26 +97,49 @@ spec: Scheduled run of the devantler-tech Agentic Engineer on its kagent lane: instance `kagent-cluster`, writer namespace `kagent/*`, one run a day. - 1. Restore your native memory from the lane store. List memory/ with a + 1. Take the lane store's single-writer lease, so that overlapping runs + never overwrite each other's memory (devantler-tech/monorepo#4910). + Send a WebDAV LOCK to lease (the store URL plus /lease) with the + store header, `Timeout: Infinite` and `Content-Type: application/xml`, + and this body with OWNER replaced by `agentic-engineer`, your UTC + start time and a random 16-hex-digit nonce, which is your owner: + `OWNER` + Read the status with `curl -sS -D -`, not -f, which hides it. A 200 + or 201 means you hold the lease: keep the reply's Lock-Token value, + angle brackets included. A 423 means another run holds it, or a run + that crashed left it behind: stop, upload nothing and report it. + Never remove a lease you did not take. Any other reply also stops + the run before it touches memory. + 2. Restore your native memory from the lane store. List memory/ with a WebDAV PROPFIND (`curl -fsS -X PROPFIND -H 'Depth: 1'` plus the store header) and download every file into ~/lane-memory/. Only a 404 for memory/ means a first run, so start with an empty ~/lane-memory/. Any - other failure means the store is unreachable: stop and report it. - 2. Shallow-clone devantler-tech/monorepo, read its AGENTS.md, load the + other failure, or a download you cannot complete, stops the run: + release the lease with the UNLOCK of step 4, upload nothing and + report it. + 3. Shallow-clone devantler-tech/monorepo, read its AGENTS.md, load the agentic-engineering plugin and invoke its agentic-engineer entrypoint as this scheduled run. ~/lane-memory/ is your native memory (the legacy layout), as the durable-memory guide says for this lane. Follow the consumer contract and fail closed on missing deployment facts. Do not duplicate the role here. - 3. Before you stop, however the run ended, upload ~/lane-memory/ back to - memory/ on the lane store, and upload this session's transcript (the - newest Claude Code session JSONL under your Claude config directory) - and your run report to runs//transcript.jsonl and - runs//report.md. If an earlier run that day already wrote - them, never replace them: write yours beside them as - transcript-.jsonl and report-.md. - Create a missing folder with MKCOL before writing into it, and check - that every upload succeeded. + 4. Before you stop, however the run ended, refresh the lease: a LOCK to + lease with the store header, `Timeout: Infinite`, no body, and your + Lock-Token value in parentheses as the If header (`If: ()` for + `Lock-Token: `). You still hold the lease only if the reply is 200 + and its owner is yours: a store restart drops every lease and can + hand your token to another run. On any other reply, upload nothing, + leave the lease alone and report it. Otherwise upload ~/lane-memory/ + back to memory/, + and upload this session's transcript (the newest Claude Code session + JSONL under your Claude config directory) and your run report to + runs//transcript.jsonl and runs//report.md. If + an earlier run that day already wrote them, never replace them: + write yours beside them as transcript-.jsonl and + report-.md. Create a missing folder with MKCOL + before writing into it, and check that every upload succeeded. Then + release the lease: an UNLOCK to lease with the store header and your + Lock-Token value as the Lock-Token header. This run may open, update and review devantler-tech pull requests and issues, per AGENTS.md. It never merges. Only when genuinely blocked on a diff --git a/scripts/tests/test-agentic-engineer-lane-store-lease.sh b/scripts/tests/test-agentic-engineer-lane-store-lease.sh new file mode 100755 index 0000000000..46197c8707 --- /dev/null +++ b/scripts/tests/test-agentic-engineer-lane-store-lease.sh @@ -0,0 +1,223 @@ +#!/usr/bin/env bash +# Prove the kagent lane's single-writer lease holds on the store the lane runs against. +# +# kagent starts each day's run whether or not the last one has finished, and a run has no time +# limit, so two runs can overlap (devantler-tech/monorepo#4910). Before a run touches its memory it +# takes an exclusive WebDAV LOCK on the lane store's lease, refreshes it before it uploads, and +# releases it afterwards (job-schedule.yaml). This starts the store's own pinned rclone image with +# the Deployment's own arguments, minus the credential the egress gateway adds, and proves on it: +# - a held lease refuses every other run, including many at once; +# - the lease never blocks the holder's own uploads; +# - a refresh succeeds only with the holder's token and names the holder; +# - only the holder's token releases it; +# - a store restart frees it, and a run that held it before can never pass a later run's lease off +# as its own. +# It also shows why the lease is not a MKCOL: the store accepts a MKCOL of a folder that exists. +set -euo pipefail + +root="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)" +dir="${root}/k8s/providers/hetzner/apps/agentic-engineer" +deployment="${dir}/deployment-lane-store.yaml" +work="$(mktemp -d)" +container='' +completed=0 +# Remove the store and the work directory on exit, and fail any exit that did not reach the PASS. +cleanup() { + if [[ -n "${container}" ]]; then + docker rm -f "${container}" >/dev/null 2>&1 || true + fi + rm -rf "${work}" + if [[ "${completed}" != 1 ]]; then + printf 'FAIL: agentic-engineer lane store lease: aborted before completion\n' >&2 + exit 1 + fi +} +trap cleanup EXIT +# Print one failure reason and stop. +fail() { + printf 'FAIL: agentic-engineer lane store lease: %s\n' "$1" >&2 + exit 1 +} +for tool in curl docker yq; do + command -v "${tool}" >/dev/null || fail "${tool} is required" +done + +# The run message takes the lease before it reads memory, refreshes it before it uploads, and +# releases it last. The body this test locks with is the one the run message gives. +prompt="$(yq '.spec.template.spec.containers[] | select(.name == "schedule") | .env[] | + select(.name == "PROMPT") | .value' "${dir}/job-schedule.yaml")" +# at : how many characters of the run message come before text, or -1 when it is missing. +at() { + local before="${prompt%%"$1"*}" + if [[ "${before}" == "${prompt}" ]]; then printf '%s' -1; else printf '%s' "${#before}"; fi +} +previous=-1 +for step in 'Send a WebDAV LOCK to lease' 'PROPFIND' 'refresh the lease' 'upload ~/lane-memory/' \ + 'UNLOCK to lease'; do + position="$(at "${step}")" + [[ "${position}" -gt "${previous}" ]] || fail "the run message does not say, in order: ${step}" + previous="${position}" +done +for rule in 'Timeout: Infinite' 'A 423 means another run holds it' 'Never remove a lease you did not take' \ + 'its owner is yours' 'leave the lease alone'; do + [[ "$(at "${rule}")" -ge 0 ]] || fail "the run message no longer says: ${rule}" +done +pattern='(<[?]xml [^`]*)' +[[ "${prompt}" =~ ${pattern} ]] || fail 'the run message gives no lock body' +lockinfo="${BASH_REMATCH[1]}" +for part in '' '' 'OWNER'; do + [[ "${lockinfo}" == *"${part}"* ]] || fail "the run message's lock body has no ${part}: ${lockinfo}" +done + +# The store's image and arguments, without the credential: the egress gateway adds that, and it +# has no part in the lease. +image="$(yq '.spec.template.spec.containers[] | select(.name == "webdav") | .image' "${deployment}")" +[[ "${image}" =~ ^docker\.io/rclone/rclone:[^@]+@sha256:[0-9a-f]{64}$ ]] || + fail "the store does not run a digest-pinned rclone: ${image}" +args=() +dropped=0 +while IFS= read -r arg; do + case "${arg}" in + --htpasswd=*) dropped=$((dropped + 1)) ;; + *) args+=("${arg}") ;; + esac +done < <(yq '.spec.template.spec.containers[] | select(.name == "webdav") | .args[]' "${deployment}") +[[ "${dropped}" == 1 ]] || fail "the store has ${dropped} --htpasswd arguments, not one" +[[ "${args[*]:0:4}" == 'serve webdav /data --addr=:8080' ]] || + fail "the store no longer serves WebDAV from /data on :8080: ${args[*]}" + +mkdir -p "${work}/data" +container="lane-store-lease-$$-${RANDOM}" +docker run -d --name "${container}" --user "$(id -u):$(id -g)" --read-only --tmpfs /tmp \ + --cap-drop ALL --security-opt no-new-privileges \ + --mount "type=bind,src=${work}/data,dst=/data" \ + -p 127.0.0.1::8080 "${image}" "${args[@]}" >/dev/null +url='' +# connect: waits until the store answers, then points url at it. A restart can move its port. +connect() { + local ports address attempt + for attempt in $(seq 1 30); do + ports="$(docker port "${container}" 8080/tcp 2>/dev/null || true)" + address="${ports%%$'\n'*}" + if [[ -n "${address}" ]] && + curl -fsS -o /dev/null -X PROPFIND -H 'Depth: 0' "http://${address}/" 2>/dev/null; then + url="http://${address}" + return 0 + fi + sleep 1 + done + fail "the store did not answer after ${attempt} tries: $(docker logs "${container}" 2>&1 | tail -c 600)" +} +connect + +# dav [curl arguments...]: sends one request, keeps its headers and body +# as .headers and .body, and prints its status. +dav() { + local name="$1" method="$2" path="$3" + shift 3 + curl -sS -o "${work}/${name}.body" -D "${work}/${name}.headers" -w '%{http_code}' \ + -X "${method}" "$@" "${url}${path}" +} +# lock : takes the lease with the run message's body, as owner. +lock() { + dav "$1" LOCK /lease -H 'Timeout: Infinite' -H 'Content-Type: application/xml' \ + --data-binary "${lockinfo/OWNER/$2}" +} +# refresh : refreshes the lease as the run message does, with no body. +refresh() { dav "$1" LOCK /lease -H 'Timeout: Infinite' -H "If: ($2)"; } +# unlock : releases the lease. +unlock() { dav "$1" UNLOCK /lease -H "Lock-Token: $2"; } +# token : the Lock-Token a reply carried, angle brackets included, or nothing. +token() { + local value + value="$(tr -d '\r' <"${work}/$1.headers" | sed -n 's/^[Ll][Oo][Cc][Kk]-[Tt][Oo][Kk][Ee][Nn]:[[:space:]]*//p')" + if [[ "${value}" =~ ^\<[^\<\>]+\>$ ]]; then printf '%s' "${value}"; fi +} +# names : whether a lock reply names owner as the lease's holder. +names() { grep -Fq ">$2<" "${work}/$1.body"; } +# body : the start of a reply's body, for a failure message. +body() { tr -d '\r\n' <"${work}/$1.body" | head -c 400; } +# owner : a run's owner in the shape the run message gives. +owner() { printf 'agentic-engineer 2026-11-11T05:15:%02dZ %016x' "$(($1 % 60))" "$1"; } + +# A MKCOL is no lease: the store answers 201 for a folder that already exists. +[[ "$(dav mkcol-new MKCOL /memory)" == 201 ]] || fail "the store refused a MKCOL: $(body mkcol-new)" +status="$(dav mkcol-again MKCOL /memory)" +[[ "${status}" == 201 ]] || + fail "the store now refuses a MKCOL of an existing folder (${status}): it could serve as a lease, so revisit why the lease is a LOCK" + +# A first run takes the lease, and the reply names it as the holder. +status="$(lock first "$(owner 1)")" +[[ "${status}" == 201 ]] || fail "the first run did not take the lease: ${status} $(body first)" +first="$(token first)" +[[ -n "${first}" ]] || fail 'the lease reply carried no Lock-Token' +names first "$(owner 1)" || fail "the lease reply does not name its holder: $(body first)" + +# While it holds the lease, another run is refused, and the holder's own uploads still land. +status="$(lock second "$(owner 2)")" +[[ "${status}" == 423 ]] || fail "a second run was not refused the held lease: ${status} $(body second)" +status="$(dav upload PUT /memory/MEMORY.md --data-binary 'memory')" +[[ "${status}" == 201 ]] || fail "the lease blocked an upload to memory/: ${status} $(body upload)" + +# Only the holder's token refreshes the lease, and the refresh names the holder. +status="$(refresh refreshed "${first}")" +[[ "${status}" == 200 ]] || fail "the holder could not refresh the lease: ${status} $(body refreshed)" +names refreshed "$(owner 1)" || fail "the refresh does not name the holder: $(body refreshed)" +status="$(refresh forged '')" +[[ "${status}" != 200 ]] || fail 'a refresh with a token nobody holds succeeded' + +# Only the holder's token releases it. +status="$(unlock forged-unlock '')" +[[ "${status}" != 204 ]] || fail 'an UNLOCK with a token nobody holds succeeded' +[[ "$(lock still-held "$(owner 3)")" == 423 ]] || fail 'an UNLOCK with a wrong token released the lease' +status="$(unlock released "${first}")" +[[ "${status}" == 204 ]] || fail "the holder could not release the lease: ${status} $(body released)" + +# Of many runs that try at once, exactly one takes the lease. +racers=20 +for racer in $(seq 1 "${racers}"); do + (lock "race-${racer}" "$(owner "$((100 + racer))")" >"${work}/race-${racer}.status" || true) & +done +wait +winners=() +refused=0 +for racer in $(seq 1 "${racers}"); do + case "$(cat "${work}/race-${racer}.status")" in + 200 | 201) winners+=("${racer}") ;; + 423) refused=$((refused + 1)) ;; + *) fail "racer ${racer} got neither the lease nor a 423: $(cat "${work}/race-${racer}.status")" ;; + esac +done +[[ "${#winners[@]}" == 1 && "${refused}" == $((racers - 1)) ]] || + fail "${#winners[@]} of ${racers} simultaneous runs took the lease, not exactly one" +winner="$(token "race-${winners[0]}")" +names "race-${winners[0]}" "$(owner "$((100 + winners[0]))")" || fail 'the winning reply does not name the winner' +[[ "$(unlock race-released "${winner}")" == 204 ]] || fail 'the winner could not release the lease' + +# A store restart frees the lease, and can hand an earlier run's token to a later run. So a run +# that held the lease before the restart stops at its refresh: the refresh fails, or names the +# later run as the holder, never the earlier one. Both runs here take the first lease after a +# restart, the case where a token could repeat. +docker restart -t 5 "${container}" >/dev/null +connect +[[ "$(lock before-restart "$(owner 4)")" == 200 ]] || fail 'a run could not take the lease before the restart' +before="$(token before-restart)" +docker restart -t 5 "${container}" >/dev/null +connect +status="$(lock after-restart "$(owner 5)")" +[[ "${status}" == 200 ]] || fail "the restart did not free the lease: ${status} $(body after-restart)" +after="$(token after-restart)" +status="$(refresh stale "${before}")" +if [[ "${status}" == 200 ]] && names stale "$(owner 4)"; then + fail 'after a restart, the earlier run passed the refresh as the holder of a lease it lost' +fi +if [[ "${before}" == "${after}" ]]; then + printf 'note: the restarted store reissued token %s, so only the owner shows who holds the lease\n' "${after}" +fi +[[ "$(lock after-stale "$(owner 6)")" == 423 ]] || fail "the earlier run's refresh released the later run's lease" + +grep -Fq 'bash scripts/tests/test-agentic-engineer-lane-store-lease.sh' "${root}/.github/workflows/ci.yaml" || + fail 'CI does not run this test' + +completed=1 +printf 'agentic-engineer lane store lease: PASS\n' From f31acc109fd0bb54964cec81074f4dc3683cbc77 Mon Sep 17 00:00:00 2001 From: Nikolai Emil Damm Date: Wed, 11 Nov 2026 15:18:49 +0100 Subject: [PATCH 2/5] test(agentic-engineer): build the restart case where a lease token repeats The store numbers its lock tokens up from the clock at its start (x/net/webdav's memLS), so after a burst of writes a restarted store hands out a token an earlier run still holds. The test now builds that case on purpose and proves the earlier run's refresh succeeds on the later run's lease, so only the owner check in the run message keeps the earlier run from uploading. Refs devantler-tech/monorepo#4910 Co-Authored-By: Claude Opus 5.5 --- .../test-agentic-engineer-lane-store-lease.sh | 47 +++++++++++++------ 1 file changed, 33 insertions(+), 14 deletions(-) diff --git a/scripts/tests/test-agentic-engineer-lane-store-lease.sh b/scripts/tests/test-agentic-engineer-lane-store-lease.sh index 46197c8707..b9bd2c86b7 100755 --- a/scripts/tests/test-agentic-engineer-lane-store-lease.sh +++ b/scripts/tests/test-agentic-engineer-lane-store-lease.sh @@ -10,8 +10,8 @@ # - the lease never blocks the holder's own uploads; # - a refresh succeeds only with the holder's token and names the holder; # - only the holder's token releases it; -# - a store restart frees it, and a run that held it before can never pass a later run's lease off -# as its own. +# - a store restart frees it and can hand an earlier run's token to a later run's lease, so only a +# refresh that names the run as the holder shows the lease is still its own. # It also shows why the lease is not a MKCOL: the store accepts a MKCOL of a folder that exists. set -euo pipefail @@ -194,25 +194,44 @@ winner="$(token "race-${winners[0]}")" names "race-${winners[0]}" "$(owner "$((100 + winners[0]))")" || fail 'the winning reply does not name the winner' [[ "$(unlock race-released "${winner}")" == 204 ]] || fail 'the winner could not release the lease' -# A store restart frees the lease, and can hand an earlier run's token to a later run. So a run -# that held the lease before the restart stops at its refresh: the refresh fails, or names the -# later run as the holder, never the earlier one. Both runs here take the first lease after a -# restart, the case where a token could repeat. +# A store restart frees every lease, and can hand a token a run took before it to a later run's +# lease: the store numbers its tokens up from the clock at its start, so a burst of writes after a +# start runs the count ahead of the clock, and the restarted store counts through the same +# numbers again. This builds exactly that case. The earlier run's refresh then succeeds on the +# later run's lease, and only the owner shows that the lease is no longer its own. docker restart -t 5 "${container}" >/dev/null connect +for burst in $(seq 1 200); do + [[ "$(lock burst "$(owner 7)")" == 200 ]] || fail "burst ${burst} could not take the lease: $(body burst)" + [[ "$(unlock burst-released "$(token burst)")" == 204 ]] || fail "burst ${burst} could not release the lease" +done [[ "$(lock before-restart "$(owner 4)")" == 200 ]] || fail 'a run could not take the lease before the restart' before="$(token before-restart)" +[[ "${before}" =~ ^\<[0-9]+\>$ ]] || + fail "the store's lock tokens are no longer numbers (${before}): revisit whether a restart can reissue one" docker restart -t 5 "${container}" >/dev/null connect -status="$(lock after-restart "$(owner 5)")" -[[ "${status}" == 200 ]] || fail "the restart did not free the lease: ${status} $(body after-restart)" -after="$(token after-restart)" +# Later runs take and release the lease until the store hands out the earlier run's token. +after='' +for attempt in $(seq 1 400); do + status="$(lock after-restart "$(owner 5)")" + [[ "${status}" == 200 ]] || fail "the restart did not free the lease: ${status} $(body after-restart)" + after="$(token after-restart)" + [[ "${after}" =~ ^\<[0-9]+\>$ ]] || fail "the restarted store's token is not a number: ${after}" + reached="${after//[<>]/}" + target="${before//[<>]/}" + if ((reached >= target)); then + break + fi + [[ "$(unlock after-released "${after}")" == 204 ]] || fail 'a later run could not release the lease' +done +[[ "${after}" == "${before}" ]] || + fail "the restarted store did not reissue the earlier token ${before} (reached ${after} after ${attempt} leases)" status="$(refresh stale "${before}")" -if [[ "${status}" == 200 ]] && names stale "$(owner 4)"; then - fail 'after a restart, the earlier run passed the refresh as the holder of a lease it lost' -fi -if [[ "${before}" == "${after}" ]]; then - printf 'note: the restarted store reissued token %s, so only the owner shows who holds the lease\n' "${after}" +[[ "${status}" == 200 ]] || + fail "the earlier run's token no longer refreshes the later run's lease (${status}), so this no longer shows why the owner matters" +if names stale "$(owner 4)" || ! names stale "$(owner 5)"; then + fail "the refresh on a reissued token does not name the later run as the holder: $(body stale)" fi [[ "$(lock after-stale "$(owner 6)")" == 423 ]] || fail "the earlier run's refresh released the later run's lease" From c522da18ca24984054a51da8596bb31be7a23fa3 Mon Sep 17 00:00:00 2001 From: Nikolai Emil Damm Date: Wed, 11 Nov 2026 15:47:25 +0100 Subject: [PATCH 3/5] fix(agentic-engineer): fence memory writes with the lease and refuse reissuable tokens Review found that the lease on /lease guarded nothing at the store: a run that lost it to a store restart could keep writing memory/, and because the store numbers lock tokens up from the second it started, a restarted store could hand that run's token to a later run, so a stale UNLOCK could release the later run's lease. The lease is now a LOCK on memory/ itself, and the run writes there one file at a time with the lock's token, so the store refuses a write without it, and refuses the former holder's writes after a restart. A run also refuses a token that is not at least 10 below the lock reply's Date: a token at or below the store's clock can never be handed out again, so no stale UNLOCK can match a later run's lease. The test now proves the fence, the restart case, and the burst case where a token above the Date is reissued and passes the fence, which is why the run refuses one. Refs devantler-tech/monorepo#4910 Co-Authored-By: Claude Opus 5.5 --- .github/workflows/ci.yaml | 6 +- .../deployment-lane-store.yaml | 9 +- .../apps/agentic-engineer/job-schedule.yaml | 78 ++++---- .../test-agentic-engineer-lane-store-lease.sh | 178 ++++++++++++------ 4 files changed, 169 insertions(+), 102 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 612aca5d6c..9d3716959b 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -2890,9 +2890,9 @@ jobs: - name: 🔒 Validate the Agentic Engineer lane store lease if: needs.changes.outputs.k8s == 'true' # Runs the store's pinned rclone image with its own arguments: the lease a - # run takes before it touches memory refuses every other run, even many at - # once, releases only for its holder, and a store restart cannot pass one - # run's lease off as another's. + # run takes on memory/ refuses every other run, even many at once, the + # store refuses a write there without its token, and after a restart the + # former holder can neither write nor release. run: | shellcheck scripts/tests/test-agentic-engineer-lane-store-lease.sh bash scripts/tests/test-agentic-engineer-lane-store-lease.sh diff --git a/k8s/providers/hetzner/apps/agentic-engineer/deployment-lane-store.yaml b/k8s/providers/hetzner/apps/agentic-engineer/deployment-lane-store.yaml index 2d33e3e3e5..d669a48b43 100644 --- a/k8s/providers/hetzner/apps/agentic-engineer/deployment-lane-store.yaml +++ b/k8s/providers/hetzner/apps/agentic-engineer/deployment-lane-store.yaml @@ -6,10 +6,11 @@ # - webdav (8080): read and write, behind the lane-store credential. Only # Substrate's egress gateway reaches it, sending that credential in place of # the agent's placeholder (egress-policy-lane-store.yaml). It also holds -# the lane's single-writer lease, a WebDAV LOCK on /lease kept only in its -# memory (devantler-tech/monorepo#4910). A run that dies before releasing -# the lease leaves it held, and every later run stops at it; restarting -# this Deployment frees it. +# the lane's single-writer lease, a WebDAV LOCK on memory/ kept only in +# its memory, and refuses a write under memory/ without that lock's token +# (devantler-tech/monorepo#4910). A run that dies before releasing the +# lease leaves it held, and every later run stops at it; restarting this +# Deployment frees it. # - reader (8081): read only, with no credential of its own. Only the # Kubernetes API server reaches it, so a reader needs the # services/proxy grant in role-lane-store-reader.yaml. diff --git a/k8s/providers/hetzner/apps/agentic-engineer/job-schedule.yaml b/k8s/providers/hetzner/apps/agentic-engineer/job-schedule.yaml index 71cb08152b..b977866b73 100644 --- a/k8s/providers/hetzner/apps/agentic-engineer/job-schedule.yaml +++ b/k8s/providers/hetzner/apps/agentic-engineer/job-schedule.yaml @@ -97,26 +97,33 @@ spec: Scheduled run of the devantler-tech Agentic Engineer on its kagent lane: instance `kagent-cluster`, writer namespace `kagent/*`, one run a day. - 1. Take the lane store's single-writer lease, so that overlapping runs - never overwrite each other's memory (devantler-tech/monorepo#4910). - Send a WebDAV LOCK to lease (the store URL plus /lease) with the - store header, `Timeout: Infinite` and `Content-Type: application/xml`, - and this body with OWNER replaced by `agentic-engineer`, your UTC - start time and a random 16-hex-digit nonce, which is your owner: - `OWNER` - Read the status with `curl -sS -D -`, not -f, which hides it. A 200 - or 201 means you hold the lease: keep the reply's Lock-Token value, - angle brackets included. A 423 means another run holds it, or a run - that crashed left it behind: stop, upload nothing and report it. - Never remove a lease you did not take. Any other reply also stops - the run before it touches memory. - 2. Restore your native memory from the lane store. List memory/ with a - WebDAV PROPFIND (`curl -fsS -X PROPFIND -H 'Depth: 1'` plus the store - header) and download every file into ~/lane-memory/. Only a 404 for - memory/ means a first run, so start with an empty ~/lane-memory/. Any - other failure, or a download you cannot complete, stops the run: - release the lease with the UNLOCK of step 4, upload nothing and - report it. + 1. Take the lane store's single-writer lease: an exclusive WebDAV lock + on memory/ that also fences every write under it, so that overlapping + runs never overwrite each other's memory (devantler-tech/monorepo#4910). + Read every status with `curl -sS -D -`, not -f, which hides it. + a. Send a PROPFIND to memory/ with the store header and `Depth: 0`. + A 404 means a first run: create memory/ with a MKCOL. Any reply + but 207 or that 404 stops the run. + b. Send a LOCK to memory/ with the store header, `Timeout: Infinite` + and `Content-Type: application/xml`, and this body with OWNER + replaced by `agentic-engineer`, your UTC start time and a random + 16-hex-digit nonce, which is your owner: + `OWNER` + A 200 means you hold the lease: keep the reply's Lock-Token value, + angle brackets included. A 423 means another run holds it, or a + run that crashed left it behind: stop, upload nothing and report + it. Never remove a lease you did not take. Any other reply also + stops the run before it touches memory. + c. The store numbers its tokens up from the second it started, so a + restarted store can hand a number out again. Your token's number + must be at least 10 below the reply's Date header in Unix seconds, + so that no restart can reissue it. Otherwise release the lease + with the UNLOCK of step 4, upload nothing and report it. + 2. Restore your native memory from the lane store: list memory/ with a + PROPFIND (`-X PROPFIND -H 'Depth: 1'` plus the store header) and + download every file into ~/lane-memory/, which stays empty on a first + run. A listing or download you cannot complete stops the run: release + the lease with the UNLOCK of step 4, upload nothing and report it. 3. Shallow-clone devantler-tech/monorepo, read its AGENTS.md, load the agentic-engineering plugin and invoke its agentic-engineer entrypoint as this scheduled run. ~/lane-memory/ is your native memory (the @@ -124,22 +131,25 @@ spec: Follow the consumer contract and fail closed on missing deployment facts. Do not duplicate the role here. 4. Before you stop, however the run ended, refresh the lease: a LOCK to - lease with the store header, `Timeout: Infinite`, no body, and your + memory/ with the store header, `Timeout: Infinite`, no body, and your Lock-Token value in parentheses as the If header (`If: ()` for `Lock-Token: `). You still hold the lease only if the reply is 200 - and its owner is yours: a store restart drops every lease and can - hand your token to another run. On any other reply, upload nothing, - leave the lease alone and report it. Otherwise upload ~/lane-memory/ - back to memory/, - and upload this session's transcript (the newest Claude Code session - JSONL under your Claude config directory) and your run report to - runs//transcript.jsonl and runs//report.md. If - an earlier run that day already wrote them, never replace them: - write yours beside them as transcript-.jsonl and - report-.md. Create a missing folder with MKCOL - before writing into it, and check that every upload succeeded. Then - release the lease: an UNLOCK to lease with the store header and your - Lock-Token value as the Lock-Token header. + and its owner is yours. On any other reply, upload nothing, leave the + lease alone and report it. Otherwise upload ~/lane-memory/ back to + memory/ one file at a time, each PUT with that same If header: the + store refuses a write under memory/ that lacks the lease's token, and + a second write with one token while the first is still running. A 412 + or 423 means you lost the lease: stop uploading, leave the lease alone + and report it. Then upload this session's transcript (the newest + Claude Code session JSONL under your Claude config directory) and your + run report, with no If header, to runs//transcript.jsonl and + runs//report.md. If an earlier run that day already wrote + them, never replace them: write yours beside them as + transcript-.jsonl and report-.md. + Create a missing folder with MKCOL before writing into it, and check + that every upload succeeded. Last, when every write under memory/ + succeeded, release the lease: an UNLOCK to memory/ with the store + header and your Lock-Token value as the Lock-Token header. This run may open, update and review devantler-tech pull requests and issues, per AGENTS.md. It never merges. Only when genuinely blocked on a diff --git a/scripts/tests/test-agentic-engineer-lane-store-lease.sh b/scripts/tests/test-agentic-engineer-lane-store-lease.sh index b9bd2c86b7..11755ead5f 100755 --- a/scripts/tests/test-agentic-engineer-lane-store-lease.sh +++ b/scripts/tests/test-agentic-engineer-lane-store-lease.sh @@ -3,15 +3,18 @@ # # kagent starts each day's run whether or not the last one has finished, and a run has no time # limit, so two runs can overlap (devantler-tech/monorepo#4910). Before a run touches its memory it -# takes an exclusive WebDAV LOCK on the lane store's lease, refreshes it before it uploads, and -# releases it afterwards (job-schedule.yaml). This starts the store's own pinned rclone image with -# the Deployment's own arguments, minus the credential the egress gateway adds, and proves on it: +# takes an exclusive WebDAV LOCK on the lane store's memory/, writes there only with that lock's +# token, and releases it last (job-schedule.yaml). This starts the store's own pinned rclone image +# with the Deployment's own arguments, minus the credential the egress gateway adds, and proves: # - a held lease refuses every other run, including many at once; -# - the lease never blocks the holder's own uploads; +# - the store fences memory/: it refuses a write there without the lease's token, and lets the +# holder's writes, and every write outside memory/, through; # - a refresh succeeds only with the holder's token and names the holder; -# - only the holder's token releases it; -# - a store restart frees it and can hand an earlier run's token to a later run's lease, so only a -# refresh that names the run as the holder shows the lease is still its own. +# - only the holder's token releases the lease; +# - a store restart frees the lease and refuses the former holder's writes and release; +# - the store numbers its tokens up from the second it started, so a token at least 10 below the +# reply's Date is never handed out again after a restart, while a token above it can be: that +# is why the run refuses a token above it. # It also shows why the lease is not a MKCOL: the store accepts a MKCOL of a folder that exists. set -euo pipefail @@ -42,8 +45,9 @@ for tool in curl docker yq; do command -v "${tool}" >/dev/null || fail "${tool} is required" done -# The run message takes the lease before it reads memory, refreshes it before it uploads, and -# releases it last. The body this test locks with is the one the run message gives. +# The run message checks for memory/, takes the lease, checks its token against the clock, +# downloads, refreshes, uploads one file at a time with the token, and releases the lease last. +# The body this test locks with is the one the run message gives. prompt="$(yq '.spec.template.spec.containers[] | select(.name == "schedule") | .env[] | select(.name == "PROMPT") | .value' "${dir}/job-schedule.yaml")" # at : how many characters of the run message come before text, or -1 when it is missing. @@ -52,14 +56,14 @@ at() { if [[ "${before}" == "${prompt}" ]]; then printf '%s' -1; else printf '%s' "${#before}"; fi } previous=-1 -for step in 'Send a WebDAV LOCK to lease' 'PROPFIND' 'refresh the lease' 'upload ~/lane-memory/' \ - 'UNLOCK to lease'; do +for step in 'Send a PROPFIND to memory/' 'Send a LOCK to memory/' 'at least 10 below the reply' \ + 'download every file' 'refresh the lease' 'one file at a time' 'UNLOCK to memory/'; do position="$(at "${step}")" [[ "${position}" -gt "${previous}" ]] || fail "the run message does not say, in order: ${step}" previous="${position}" done for rule in 'Timeout: Infinite' 'A 423 means another run holds it' 'Never remove a lease you did not take' \ - 'its owner is yours' 'leave the lease alone'; do + 'its owner is yours' 'each PUT with that same If header' 'leave the lease alone' 'A 412'; do [[ "$(at "${rule}")" -ge 0 ]] || fail "the run message no longer says: ${rule}" done pattern='(<[?]xml [^`]*)' @@ -108,6 +112,11 @@ connect() { done fail "the store did not answer after ${attempt} tries: $(docker logs "${container}" 2>&1 | tail -c 600)" } +# restart: restarts the store, which drops every lease, and waits for it. +restart() { + docker restart -t 5 "${container}" >/dev/null + connect +} connect # dav [curl arguments...]: sends one request, keeps its headers and body @@ -118,20 +127,41 @@ dav() { curl -sS -o "${work}/${name}.body" -D "${work}/${name}.headers" -w '%{http_code}' \ -X "${method}" "$@" "${url}${path}" } -# lock : takes the lease with the run message's body, as owner. +# lock : takes the lease on memory/ with the run message's body, as owner. lock() { - dav "$1" LOCK /lease -H 'Timeout: Infinite' -H 'Content-Type: application/xml' \ + dav "$1" LOCK /memory/ -H 'Timeout: Infinite' -H 'Content-Type: application/xml' \ --data-binary "${lockinfo/OWNER/$2}" } # refresh : refreshes the lease as the run message does, with no body. -refresh() { dav "$1" LOCK /lease -H 'Timeout: Infinite' -H "If: ($2)"; } +refresh() { dav "$1" LOCK /memory/ -H 'Timeout: Infinite' -H "If: ($2)"; } # unlock : releases the lease. -unlock() { dav "$1" UNLOCK /lease -H "Lock-Token: $2"; } +unlock() { dav "$1" UNLOCK /memory/ -H "Lock-Token: $2"; } +# put [token]: writes a file, with the lease's token when one is given. +put() { + if [[ -n "${3:-}" ]]; then + dav "$1" PUT "$2" --data-binary "$1" -H "If: ($3)" + else + dav "$1" PUT "$2" --data-binary "$1" + fi +} +# header
: one header of a reply, or nothing. +header() { + tr -d '\r' <"${work}/$1.headers" | awk -v h="$2" 'BEGIN {h = tolower(h) ":"} + tolower(substr($0, 1, length(h))) == h {sub(/^[^:]*:[[:space:]]*/, ""); print; exit}' +} # token : the Lock-Token a reply carried, angle brackets included, or nothing. token() { local value - value="$(tr -d '\r' <"${work}/$1.headers" | sed -n 's/^[Ll][Oo][Cc][Kk]-[Tt][Oo][Kk][Ee][Nn]:[[:space:]]*//p')" - if [[ "${value}" =~ ^\<[^\<\>]+\>$ ]]; then printf '%s' "${value}"; fi + value="$(header "$1" Lock-Token)" + if [[ "${value}" =~ ^\<[0-9]+\>$ ]]; then printf '%s' "${value}"; fi +} +# number : the number inside a token. +number() { printf '%s' "${1//[<>]/}"; } +# clock : a reply's Date header in Unix seconds. +clock() { + local value + value="$(header "$1" Date)" + date -u -d "${value}" +%s 2>/dev/null || date -u -j -f '%a, %d %b %Y %H:%M:%S GMT' "${value}" +%s } # names : whether a lock reply names owner as the lease's holder. names() { grep -Fq ">$2<" "${work}/$1.body"; } @@ -141,37 +171,50 @@ body() { tr -d '\r\n' <"${work}/$1.body" | head -c 400; } owner() { printf 'agentic-engineer 2026-11-11T05:15:%02dZ %016x' "$(($1 % 60))" "$1"; } # A MKCOL is no lease: the store answers 201 for a folder that already exists. -[[ "$(dav mkcol-new MKCOL /memory)" == 201 ]] || fail "the store refused a MKCOL: $(body mkcol-new)" -status="$(dav mkcol-again MKCOL /memory)" +[[ "$(dav mkcol-new MKCOL /runs/)" == 201 ]] || fail "the store refused a MKCOL: $(body mkcol-new)" +status="$(dav mkcol-again MKCOL /runs/)" [[ "${status}" == 201 ]] || fail "the store now refuses a MKCOL of an existing folder (${status}): it could serve as a lease, so revisit why the lease is a LOCK" -# A first run takes the lease, and the reply names it as the holder. +# A first run finds no memory/, creates it, and takes the lease, which names it as the holder. +[[ "$(dav first-check PROPFIND /memory/ -H 'Depth: 0')" == 404 ]] || fail 'a fresh store already has memory/' +[[ "$(dav first-mkcol MKCOL /memory/)" == 201 ]] || fail "a first run could not create memory/: $(body first-mkcol)" status="$(lock first "$(owner 1)")" -[[ "${status}" == 201 ]] || fail "the first run did not take the lease: ${status} $(body first)" +[[ "${status}" == 200 ]] || fail "the first run did not take the lease: ${status} $(body first)" first="$(token first)" -[[ -n "${first}" ]] || fail 'the lease reply carried no Lock-Token' +[[ -n "${first}" ]] || fail "the lease reply carried no numbered Lock-Token: $(header first Lock-Token)" names first "$(owner 1)" || fail "the lease reply does not name its holder: $(body first)" +[[ "$(dav first-list PROPFIND /memory/ -H 'Depth: 1')" == 207 ]] || fail 'the lease blocked a listing of memory/' -# While it holds the lease, another run is refused, and the holder's own uploads still land. +# While it holds the lease, another run is refused it. status="$(lock second "$(owner 2)")" [[ "${status}" == 423 ]] || fail "a second run was not refused the held lease: ${status} $(body second)" -status="$(dav upload PUT /memory/MEMORY.md --data-binary 'memory')" -[[ "${status}" == 201 ]] || fail "the lease blocked an upload to memory/: ${status} $(body upload)" + +# The store fences memory/: the holder's writes with its token land, a write without it or with +# another token is refused, and writes outside memory/ are not held up. +status="$(put fenced /memory/MEMORY.md "${first}")" +[[ "${status}" == 201 ]] || fail "the holder could not write memory/ with its token: ${status} $(body fenced)" +status="$(put unfenced /memory/topic.md)" +[[ "${status}" == 423 ]] || fail "the store let a write without the lease's token into memory/: ${status}" +status="$(put forged-write /memory/topic.md '<999>')" +[[ "${status}" == 412 ]] || fail "the store let a write with another token into memory/: ${status}" +status="$(put transcript /runs/transcript.jsonl)" +[[ "${status}" == 201 ]] || fail "the lease held up a write outside memory/: ${status} $(body transcript)" # Only the holder's token refreshes the lease, and the refresh names the holder. status="$(refresh refreshed "${first}")" [[ "${status}" == 200 ]] || fail "the holder could not refresh the lease: ${status} $(body refreshed)" names refreshed "$(owner 1)" || fail "the refresh does not name the holder: $(body refreshed)" -status="$(refresh forged '')" +status="$(refresh forged '<999>')" [[ "${status}" != 200 ]] || fail 'a refresh with a token nobody holds succeeded' -# Only the holder's token releases it. -status="$(unlock forged-unlock '')" +# Only the holder's token releases it, and a release opens memory/ again. +status="$(unlock forged-unlock '<999>')" [[ "${status}" != 204 ]] || fail 'an UNLOCK with a token nobody holds succeeded' [[ "$(lock still-held "$(owner 3)")" == 423 ]] || fail 'an UNLOCK with a wrong token released the lease' status="$(unlock released "${first}")" [[ "${status}" == 204 ]] || fail "the holder could not release the lease: ${status} $(body released)" +[[ "$(put after-release /memory/topic.md)" == 201 ]] || fail 'memory/ stayed fenced after the release' # Of many runs that try at once, exactly one takes the lease. racers=20 @@ -183,7 +226,7 @@ winners=() refused=0 for racer in $(seq 1 "${racers}"); do case "$(cat "${work}/race-${racer}.status")" in - 200 | 201) winners+=("${racer}") ;; + 200) winners+=("${racer}") ;; 423) refused=$((refused + 1)) ;; *) fail "racer ${racer} got neither the lease nor a 423: $(cat "${work}/race-${racer}.status")" ;; esac @@ -194,46 +237,59 @@ winner="$(token "race-${winners[0]}")" names "race-${winners[0]}" "$(owner "$((100 + winners[0]))")" || fail 'the winning reply does not name the winner' [[ "$(unlock race-released "${winner}")" == 204 ]] || fail 'the winner could not release the lease' -# A store restart frees every lease, and can hand a token a run took before it to a later run's -# lease: the store numbers its tokens up from the clock at its start, so a burst of writes after a -# start runs the count ahead of the clock, and the restarted store counts through the same -# numbers again. This builds exactly that case. The earlier run's refresh then succeeds on the -# later run's lease, and only the owner shows that the lease is no longer its own. -docker restart -t 5 "${container}" >/dev/null -connect +# A lease whose token is at least 10 below the reply's Date, as the run message requires, outlives +# no restart and is never handed out again: the restart frees it, the store refuses the former +# holder's writes and release, and the next run's lease stands. +restart +sleep 12 +[[ "$(lock settled "$(owner 4)")" == 200 ]] || fail 'a run could not take the lease on a settled store' +held="$(token settled)" +(($(number "${held}") <= $(clock settled) - 10)) || + fail "a settled store's token ${held} is not 10 below its Date $(clock settled): the run would refuse every lease" +restart +status="$(put stale-write /memory/MEMORY.md "${held}")" +[[ "${status}" == 412 ]] || fail "after a restart, the store took the former holder's write: ${status}" +[[ "$(lock next "$(owner 5)")" == 200 ]] || fail 'the restart did not free the lease' +next="$(token next)" +(($(number "${next}") > $(number "${held}"))) || + fail "the restarted store handed out ${next}, not above the earlier ${held}" +[[ "$(put stale-write-again /memory/MEMORY.md "${held}")" == 412 ]] || + fail "the store took the former holder's write under the next run's lease" +[[ "$(unlock stale-unlock "${held}")" != 204 ]] || fail "the former holder's UNLOCK succeeded after a restart" +[[ "$(lock still-next "$(owner 6)")" == 423 ]] || fail "the former holder's UNLOCK released the next run's lease" +[[ "$(unlock next-released "${next}")" == 204 ]] || fail 'the next run could not release the lease' + +# Why the run refuses a token above the Date: a burst of writes after a start runs the store's +# count ahead of the clock, and a restarted store then counts through the same numbers again. The +# later run's lease gets the earlier token, the earlier run's writes pass the fence under it, and +# only the owner in a refresh still tells the two apart. +restart for burst in $(seq 1 200); do [[ "$(lock burst "$(owner 7)")" == 200 ]] || fail "burst ${burst} could not take the lease: $(body burst)" [[ "$(unlock burst-released "$(token burst)")" == 204 ]] || fail "burst ${burst} could not release the lease" done -[[ "$(lock before-restart "$(owner 4)")" == 200 ]] || fail 'a run could not take the lease before the restart' -before="$(token before-restart)" -[[ "${before}" =~ ^\<[0-9]+\>$ ]] || - fail "the store's lock tokens are no longer numbers (${before}): revisit whether a restart can reissue one" -docker restart -t 5 "${container}" >/dev/null -connect -# Later runs take and release the lease until the store hands out the earlier run's token. -after='' +[[ "$(lock ahead "$(owner 8)")" == 200 ]] || fail 'a run could not take the lease after the burst' +ahead="$(token ahead)" +(($(number "${ahead}") > $(clock ahead) - 10)) || + fail "after the burst the token ${ahead} is still 10 below the Date $(clock ahead), so this case shows nothing" +restart +reached='' for attempt in $(seq 1 400); do - status="$(lock after-restart "$(owner 5)")" - [[ "${status}" == 200 ]] || fail "the restart did not free the lease: ${status} $(body after-restart)" - after="$(token after-restart)" - [[ "${after}" =~ ^\<[0-9]+\>$ ]] || fail "the restarted store's token is not a number: ${after}" - reached="${after//[<>]/}" - target="${before//[<>]/}" - if ((reached >= target)); then + [[ "$(lock reissued "$(owner 9)")" == 200 ]] || fail "the restart did not free the lease: $(body reissued)" + reached="$(token reissued)" + if (($(number "${reached}") >= $(number "${ahead}"))); then break fi - [[ "$(unlock after-released "${after}")" == 204 ]] || fail 'a later run could not release the lease' + [[ "$(unlock reissued-released "${reached}")" == 204 ]] || fail 'a later run could not release the lease' done -[[ "${after}" == "${before}" ]] || - fail "the restarted store did not reissue the earlier token ${before} (reached ${after} after ${attempt} leases)" -status="$(refresh stale "${before}")" -[[ "${status}" == 200 ]] || - fail "the earlier run's token no longer refreshes the later run's lease (${status}), so this no longer shows why the owner matters" -if names stale "$(owner 4)" || ! names stale "$(owner 5)"; then - fail "the refresh on a reissued token does not name the later run as the holder: $(body stale)" +[[ "${reached}" == "${ahead}" ]] || + fail "the restarted store did not reissue ${ahead} (reached ${reached} after ${attempt} leases), so the Date check guards nothing" +[[ "$(put reused-write /memory/MEMORY.md "${ahead}")" == 201 ]] || + fail "the earlier run's reissued token no longer passes the fence, so this no longer shows why the Date check matters" +status="$(refresh reused "${ahead}")" +if [[ "${status}" != 200 ]] || ! names reused "$(owner 9)" || names reused "$(owner 8)"; then + fail "a refresh on a reissued token does not name the later run: ${status} $(body reused)" fi -[[ "$(lock after-stale "$(owner 6)")" == 423 ]] || fail "the earlier run's refresh released the later run's lease" grep -Fq 'bash scripts/tests/test-agentic-engineer-lane-store-lease.sh' "${root}/.github/workflows/ci.yaml" || fail 'CI does not run this test' From 99233cad624a053a653fb10e440645692a0a20fc Mon Sep 17 00:00:00 2001 From: Nikolai Emil Damm Date: Wed, 11 Nov 2026 16:10:38 +0100 Subject: [PATCH 4/5] fix(agentic-engineer): never release a lease token that is not yet safe Review found that a run which rejected a reissuable token still sent an UNLOCK with it. If the store restarted and handed that token to another run first, the delayed UNLOCK would release the other run's lease, because the store checks only the token. A run now never releases a token until it is safe. A lease taken moments after the store started waits until the store's clock is past the token, refreshes, and goes on only if the refresh still names it; otherwise it leaves the lease alone. The safety margin is now 60 seconds and states what it relies on: node clocks that agree within a minute. The test now takes the lease on a fresh store and goes through that wait. It also shows the hazard itself: after a restart reissues a token, an UNLOCK with it releases the later run's lease. Refs devantler-tech/monorepo#4910 Co-Authored-By: Claude Opus 5.5 --- .../deployment-lane-store.yaml | 8 +-- .../apps/agentic-engineer/job-schedule.yaml | 17 ++++-- .../test-agentic-engineer-lane-store-lease.sh | 54 ++++++++++++------- 3 files changed, 52 insertions(+), 27 deletions(-) diff --git a/k8s/providers/hetzner/apps/agentic-engineer/deployment-lane-store.yaml b/k8s/providers/hetzner/apps/agentic-engineer/deployment-lane-store.yaml index d669a48b43..46a0558e53 100644 --- a/k8s/providers/hetzner/apps/agentic-engineer/deployment-lane-store.yaml +++ b/k8s/providers/hetzner/apps/agentic-engineer/deployment-lane-store.yaml @@ -8,9 +8,11 @@ # the agent's placeholder (egress-policy-lane-store.yaml). It also holds # the lane's single-writer lease, a WebDAV LOCK on memory/ kept only in # its memory, and refuses a write under memory/ without that lock's token -# (devantler-tech/monorepo#4910). A run that dies before releasing the -# lease leaves it held, and every later run stops at it; restarting this -# Deployment frees it. +# (devantler-tech/monorepo#4910). Lock tokens count up from the second the +# store started, so a run trusts one only once it is a minute behind the +# store's clock; this relies on node clocks agreeing within a minute. A run +# that dies before releasing the lease leaves it held, and every later run +# stops at it; restarting this Deployment frees it. # - reader (8081): read only, with no credential of its own. Only the # Kubernetes API server reaches it, so a reader needs the # services/proxy grant in role-lane-store-reader.yaml. diff --git a/k8s/providers/hetzner/apps/agentic-engineer/job-schedule.yaml b/k8s/providers/hetzner/apps/agentic-engineer/job-schedule.yaml index b977866b73..2c274d79b7 100644 --- a/k8s/providers/hetzner/apps/agentic-engineer/job-schedule.yaml +++ b/k8s/providers/hetzner/apps/agentic-engineer/job-schedule.yaml @@ -114,11 +114,18 @@ spec: run that crashed left it behind: stop, upload nothing and report it. Never remove a lease you did not take. Any other reply also stops the run before it touches memory. - c. The store numbers its tokens up from the second it started, so a - restarted store can hand a number out again. Your token's number - must be at least 10 below the reply's Date header in Unix seconds, - so that no restart can reissue it. Otherwise release the lease - with the UNLOCK of step 4, upload nothing and report it. + c. The store numbers its tokens up from the second it started, and an + UNLOCK checks only the token, so a restarted store could hand your + token to another run whose lease your UNLOCK would then release. + Your token is safe once its number is at least 60 below a reply's + Date header in Unix seconds: no restart can reissue it then, since + the cluster keeps its nodes' clocks within a minute of each other. + Until it is safe, never UNLOCK it. If the LOCK reply shows it is not yet + safe, the store started moments ago: wait out the difference plus + a second, if that is under 10 minutes, then refresh the lease as in + step 4 and check again. If it is still not safe, or the refresh + fails or names another owner, upload nothing, leave the lease + alone and report it. 2. Restore your native memory from the lane store: list memory/ with a PROPFIND (`-X PROPFIND -H 'Depth: 1'` plus the store header) and download every file into ~/lane-memory/, which stays empty on a first diff --git a/scripts/tests/test-agentic-engineer-lane-store-lease.sh b/scripts/tests/test-agentic-engineer-lane-store-lease.sh index 11755ead5f..eeec59a087 100755 --- a/scripts/tests/test-agentic-engineer-lane-store-lease.sh +++ b/scripts/tests/test-agentic-engineer-lane-store-lease.sh @@ -12,9 +12,10 @@ # - a refresh succeeds only with the holder's token and names the holder; # - only the holder's token releases the lease; # - a store restart frees the lease and refuses the former holder's writes and release; -# - the store numbers its tokens up from the second it started, so a token at least 10 below the -# reply's Date is never handed out again after a restart, while a token above it can be: that -# is why the run refuses a token above it. +# - the store numbers its tokens up from the second it started, so a token at least 60 below a +# reply's Date is never handed out again after a restart. A token above it can be, and an +# UNLOCK of it could then release a later run's lease: that is why a run that takes the lease +# on a fresh store waits until its token is safe, and never releases one that is not. # It also shows why the lease is not a MKCOL: the store accepts a MKCOL of a folder that exists. set -euo pipefail @@ -56,14 +57,15 @@ at() { if [[ "${before}" == "${prompt}" ]]; then printf '%s' -1; else printf '%s' "${#before}"; fi } previous=-1 -for step in 'Send a PROPFIND to memory/' 'Send a LOCK to memory/' 'at least 10 below the reply' \ - 'download every file' 'refresh the lease' 'one file at a time' 'UNLOCK to memory/'; do +for step in 'Send a PROPFIND to memory/' 'Send a LOCK to memory/' 'at least 60 below a reply' \ + 'download every file' 'however the run ended, refresh the lease' 'one file at a time' 'UNLOCK to memory/'; do position="$(at "${step}")" [[ "${position}" -gt "${previous}" ]] || fail "the run message does not say, in order: ${step}" previous="${position}" done for rule in 'Timeout: Infinite' 'A 423 means another run holds it' 'Never remove a lease you did not take' \ - 'its owner is yours' 'each PUT with that same If header' 'leave the lease alone' 'A 412'; do + 'its owner is yours' 'each PUT with that same If header' 'leave the lease alone' 'A 412' \ + 'never UNLOCK it' 'wait out the difference'; do [[ "$(at "${rule}")" -ge 0 ]] || fail "the run message no longer says: ${rule}" done pattern='(<[?]xml [^`]*)' @@ -237,15 +239,24 @@ winner="$(token "race-${winners[0]}")" names "race-${winners[0]}" "$(owner "$((100 + winners[0]))")" || fail 'the winning reply does not name the winner' [[ "$(unlock race-released "${winner}")" == 204 ]] || fail 'the winner could not release the lease' -# A lease whose token is at least 10 below the reply's Date, as the run message requires, outlives -# no restart and is never handed out again: the restart frees it, the store refuses the former +# A run that takes the lease moments after the store started holds a token that is not yet safe. +# As the run message says, it never releases that token: it waits out the difference and +# refreshes, and the token is then at least 60 below the Date. A safe token outlives no restart +# and is never handed out again: the restart frees the lease, the store refuses the former # holder's writes and release, and the next run's lease stands. restart -sleep 12 -[[ "$(lock settled "$(owner 4)")" == 200 ]] || fail 'a run could not take the lease on a settled store' -held="$(token settled)" -(($(number "${held}") <= $(clock settled) - 10)) || - fail "a settled store's token ${held} is not 10 below its Date $(clock settled): the run would refuse every lease" +[[ "$(lock fresh "$(owner 4)")" == 200 ]] || fail 'a run could not take the lease on a fresh store' +held="$(token fresh)" +wait_for=$(($(number "${held}") + 60 - $(clock fresh))) +if ((wait_for > 0)); then + sleep "$((wait_for + 1))" +fi +status="$(refresh settled "${held}")" +if [[ "${status}" != 200 ]] || ! names settled "$(owner 4)"; then + fail "the run's refresh after waiting did not hold the lease: ${status} $(body settled)" +fi +(($(number "${held}") <= $(clock settled) - 60)) || + fail "after the wait, the token ${held} is still not 60 below the Date $(clock settled)" restart status="$(put stale-write /memory/MEMORY.md "${held}")" [[ "${status}" == 412 ]] || fail "after a restart, the store took the former holder's write: ${status}" @@ -259,10 +270,11 @@ next="$(token next)" [[ "$(lock still-next "$(owner 6)")" == 423 ]] || fail "the former holder's UNLOCK released the next run's lease" [[ "$(unlock next-released "${next}")" == 204 ]] || fail 'the next run could not release the lease' -# Why the run refuses a token above the Date: a burst of writes after a start runs the store's -# count ahead of the clock, and a restarted store then counts through the same numbers again. The -# later run's lease gets the earlier token, the earlier run's writes pass the fence under it, and -# only the owner in a refresh still tells the two apart. +# Why a token that is not yet safe is never used or released: a burst of writes after a start runs +# the store's count ahead of the clock, and a restarted store then counts through the same numbers +# again. The later run's lease gets the earlier token, the earlier run's writes pass the fence +# under it, only the owner in a refresh still tells the two apart, and the earlier run's UNLOCK +# would release the later run's lease. restart for burst in $(seq 1 200); do [[ "$(lock burst "$(owner 7)")" == 200 ]] || fail "burst ${burst} could not take the lease: $(body burst)" @@ -270,8 +282,8 @@ for burst in $(seq 1 200); do done [[ "$(lock ahead "$(owner 8)")" == 200 ]] || fail 'a run could not take the lease after the burst' ahead="$(token ahead)" -(($(number "${ahead}") > $(clock ahead) - 10)) || - fail "after the burst the token ${ahead} is still 10 below the Date $(clock ahead), so this case shows nothing" +(($(number "${ahead}") > $(clock ahead) - 60)) || + fail "after the burst the token ${ahead} is still 60 below the Date $(clock ahead), so this case shows nothing" restart reached='' for attempt in $(seq 1 400); do @@ -290,6 +302,10 @@ status="$(refresh reused "${ahead}")" if [[ "${status}" != 200 ]] || ! names reused "$(owner 9)" || names reused "$(owner 8)"; then fail "a refresh on a reissued token does not name the later run: ${status} $(body reused)" fi +[[ "$(unlock reused-unlock "${ahead}")" == 204 ]] || + fail "the earlier run's UNLOCK no longer releases a lease that reissued its token, so this no longer shows why the run never releases a token that is not yet safe" +[[ "$(lock taken-over "$(owner 10)")" == 200 ]] || + fail "a third run could not take the lease the earlier run's UNLOCK released" grep -Fq 'bash scripts/tests/test-agentic-engineer-lane-store-lease.sh' "${root}/.github/workflows/ci.yaml" || fail 'CI does not run this test' From 25c2a854e5ae7b0a8fe8f81d4b7fc3226129bb15 Mon Sep 17 00:00:00 2001 From: Nikolai Emil Damm Date: Wed, 11 Nov 2026 16:38:37 +0100 Subject: [PATCH 5/5] fix(agentic-engineer): send the lease token with a folder's MKCOL too Codex review: the store fences every write under the locked memory/, not just PUT, so a run that restores a folder inside memory/ would get a 423 for its MKCOL and wrongly conclude it lost the lease. The run message now sends the lease's If header with that MKCOL as well, and lists each folder when it restores memory, not only the top level. The test proves a MKCOL without the token is refused and one with it, and a file inside it, land. Refs devantler-tech/monorepo#4910 Co-Authored-By: Claude Opus 5.5 --- .../apps/agentic-engineer/job-schedule.yaml | 20 ++++++++++--------- .../test-agentic-engineer-lane-store-lease.sh | 10 +++++++++- 2 files changed, 20 insertions(+), 10 deletions(-) diff --git a/k8s/providers/hetzner/apps/agentic-engineer/job-schedule.yaml b/k8s/providers/hetzner/apps/agentic-engineer/job-schedule.yaml index 2c274d79b7..fd2db6dcff 100644 --- a/k8s/providers/hetzner/apps/agentic-engineer/job-schedule.yaml +++ b/k8s/providers/hetzner/apps/agentic-engineer/job-schedule.yaml @@ -127,10 +127,11 @@ spec: fails or names another owner, upload nothing, leave the lease alone and report it. 2. Restore your native memory from the lane store: list memory/ with a - PROPFIND (`-X PROPFIND -H 'Depth: 1'` plus the store header) and - download every file into ~/lane-memory/, which stays empty on a first - run. A listing or download you cannot complete stops the run: release - the lease with the UNLOCK of step 4, upload nothing and report it. + PROPFIND (`-X PROPFIND -H 'Depth: 1'` plus the store header), list + each folder it shows the same way, and download every file into + ~/lane-memory/, which stays empty on a first run. A listing or + download you cannot complete stops the run: release the lease with + the UNLOCK of step 4, upload nothing and report it. 3. Shallow-clone devantler-tech/monorepo, read its AGENTS.md, load the agentic-engineering plugin and invoke its agentic-engineer entrypoint as this scheduled run. ~/lane-memory/ is your native memory (the @@ -143,11 +144,12 @@ spec: `Lock-Token: `). You still hold the lease only if the reply is 200 and its owner is yours. On any other reply, upload nothing, leave the lease alone and report it. Otherwise upload ~/lane-memory/ back to - memory/ one file at a time, each PUT with that same If header: the - store refuses a write under memory/ that lacks the lease's token, and - a second write with one token while the first is still running. A 412 - or 423 means you lost the lease: stop uploading, leave the lease alone - and report it. Then upload this session's transcript (the newest + memory/ one file at a time, with that same If header on every PUT and + on the MKCOL of any folder inside memory/: the store refuses a write + under memory/ that lacks the lease's token, and a second write with + one token while the first is still running. A 412 or 423 means you + lost the lease: stop uploading, leave the lease alone and report it. + Then upload this session's transcript (the newest Claude Code session JSONL under your Claude config directory) and your run report, with no If header, to runs//transcript.jsonl and runs//report.md. If an earlier run that day already wrote diff --git a/scripts/tests/test-agentic-engineer-lane-store-lease.sh b/scripts/tests/test-agentic-engineer-lane-store-lease.sh index eeec59a087..ccf664fe58 100755 --- a/scripts/tests/test-agentic-engineer-lane-store-lease.sh +++ b/scripts/tests/test-agentic-engineer-lane-store-lease.sh @@ -64,7 +64,8 @@ for step in 'Send a PROPFIND to memory/' 'Send a LOCK to memory/' 'at least 60 b previous="${position}" done for rule in 'Timeout: Infinite' 'A 423 means another run holds it' 'Never remove a lease you did not take' \ - 'its owner is yours' 'each PUT with that same If header' 'leave the lease alone' 'A 412' \ + 'its owner is yours' 'If header on every PUT' 'on the MKCOL of any folder inside memory/' \ + 'each folder it shows the same way' 'leave the lease alone' 'A 412' \ 'never UNLOCK it' 'wait out the difference'; do [[ "$(at "${rule}")" -ge 0 ]] || fail "the run message no longer says: ${rule}" done @@ -200,6 +201,13 @@ status="$(put unfenced /memory/topic.md)" [[ "${status}" == 423 ]] || fail "the store let a write without the lease's token into memory/: ${status}" status="$(put forged-write /memory/topic.md '<999>')" [[ "${status}" == 412 ]] || fail "the store let a write with another token into memory/: ${status}" +# A folder inside memory/ is a write too: its MKCOL needs the token, and a file in it then lands. +status="$(dav unfenced-folder MKCOL /memory/archive/)" +[[ "${status}" == 423 ]] || fail "the store let a MKCOL without the lease's token into memory/: ${status}" +status="$(dav fenced-folder MKCOL /memory/archive/ -H "If: (${first})")" +[[ "${status}" == 201 ]] || fail "the holder could not create a folder in memory/ with its token: ${status} $(body fenced-folder)" +status="$(put fenced-nested /memory/archive/old.md "${first}")" +[[ "${status}" == 201 ]] || fail "the holder could not write inside a folder of memory/: ${status} $(body fenced-nested)" status="$(put transcript /runs/transcript.jsonl)" [[ "${status}" == 201 ]] || fail "the lease held up a write outside memory/: ${status} $(body transcript)"