From 755cc88ea1ac8f4881c822300ad7bcee5802a2a6 Mon Sep 17 00:00:00 2001 From: Vikas Singhal Date: Fri, 18 Sep 2026 10:37:11 +0530 Subject: [PATCH] =?UTF-8?q?fix(deploy):=20make-live.sh=20takes=20a=20lock?= =?UTF-8?q?=20=E2=80=94=20one=20deploy=20at=20a=20time=20per=20box=20(v0.4?= =?UTF-8?q?48.4)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every live checkout is shared by whoever runs the deploy, so two concurrent runs are two `git reset --hard`s racing each other's builds: the second moves a checkout out from under the first one's `npm run build`, and the first then restarts a service with a binary built from a commit it never resolved — while reporting success, because it verifies /health against the version it was TOLD to expect. On 2026-09-17 two sessions ran this concurrently against all four tenants and it was harmless only because both happened to be deploying the same sha. The lock is an atomic `mkdir` (macOS ships no `flock`) holding a meta file. A second run fails fast, naming the holder's pid/user/start time, rather than queueing — a deploy that waits ten minutes and then runs against whatever `main` has become is its own surprise. A lock that cannot be released is worse than no lock, so every wedge has an exit: a dead holder is cleared automatically, one past `AOS_LIVE_LOCK_MAX_AGE` (default 2h) is cleared even with a live pid (covers a wedged run and pid reuse), an unreadable meta is cleared, and `--force-lock` breaks one on purpose. The loser never deletes the winner's lock, and the holder releases on EXIT — including on failure, so a crashed deploy can't block the next one. `--dry-run` is deliberately not gated (it changes nothing, and it is what you reach for to see what the other run is doing) but warns it is reading checkouts another run is moving; `--help` is untouched. One trap found while testing: under `set -euo pipefail` a `sed missing-file | head` pipeline exits non-zero and killed the script outright — silently, with the lock still in place. That is exactly the interrupted-writer case the staleness branch exists to clear, so the meta is read into a variable first. Pinned by scripts/make-live-lock-test.cjs (24 checks, runs the real script against a scratch lock and a bogus target so nothing deploys). Verified end to end against the real lock path: a real invocation was refused before touching any checkout, and the holder's lock survived. Co-Authored-By: Claude Opus 5 (1M context) Second fix in the same change: BSD and GNU `stat` disagree on what `-f` MEANS (format string vs --file-system), so chaining them with || inside one $() captured GNU's `File: "…"` dump alongside the answer. The multi-line result reached $(( … )), which evaluated `File` as a variable and, under set -u, killed the script mid-lock. Green on macOS, dead on every Linux box — CI caught it. Each stat is now tried separately and accepted only if it is all digits. Verified 24/24 on a real Linux box. --- CHANGELOG.md | 23 ++++++ CLAUDE.md | 8 +- package-lock.json | 4 +- package.json | 4 +- scripts/make-live-lock-test.cjs | 130 ++++++++++++++++++++++++++++++++ scripts/make-live.sh | 104 ++++++++++++++++++++++++- 6 files changed, 264 insertions(+), 9 deletions(-) create mode 100644 scripts/make-live-lock-test.cjs diff --git a/CHANGELOG.md b/CHANGELOG.md index cb93ae7c..9c696e61 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,29 @@ new version heading in the same commit. ## [Unreleased] +## [0.448.4] - 2026-09-18 +### Fixed +- **`make-live.sh` takes a lock: one deploy at a time per box.** Every live checkout is shared by whoever + runs the deploy, so two concurrent runs are two `git reset --hard`s racing each other's builds — the + second moves a checkout out from under the first one's `npm run build`, and the first then restarts a + service with a binary built from a commit it never resolved, reporting success because it verifies + `/health` against the version it was *told* to expect. On 2026-09-17 two sessions ran this concurrently + against all four tenants; it was harmless only because both happened to be deploying the same sha. + The lock is an atomic `mkdir` (no `flock`, which macOS doesn't ship) holding a meta file, and a second + run fails fast naming the holder's pid, user and start time rather than queueing behind it. A lock that + can't be released would be worse than none, so: a dead holder is cleared automatically, one past an age + ceiling (`AOS_LIVE_LOCK_MAX_AGE`, default 2h) is cleared even with a live pid, an unreadable meta is + cleared, `--force-lock` breaks one on purpose, and the loser never deletes the winner's lock. + `--dry-run` and `--help` still work while a deploy is in flight (a dry run warns that it is reading + checkouts another run is moving). Pinned by `scripts/make-live-lock-test.cjs`. + ⚠ BSD and GNU `stat` disagree on what `-f` MEANS (format string vs `--file-system`), so chaining the + two with `||` inside one `$(…)` captured GNU's `File: "…"` dump alongside the answer — the multi-line + result reached `$(( … ))`, which under `set -u` killed the script mid-lock with `File: unbound + variable`. Green on macOS, dead on every Linux box; caught by CI and fixed by validating each + candidate is all digits. + **For admins:** Two deploys can no longer run at once on the same box — the second is refused with the + first one's pid, rather than silently corrupting its build. + ## [0.448.2] - 2026-09-18 ### Fixed - **The waiting note no longer teaches a pattern the harness blocks.** It told every agent to wait by diff --git a/CLAUDE.md b/CLAUDE.md index f8ae4bff..177106c3 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -92,7 +92,13 @@ terminal in a test". Leftovers show as `ttyd … attach.sh /tmp/aos-*-test-*/tmu untracked `~/.agentric-live.env`) — a second tenant on the box silently keeping old code because the script only kicked one launchd label was a real recurring bug. All builds run BEFORE any restart, so a bad commit leaves every server untouched; `--only ` narrows it, `--dry-run` shows what would - deploy. The manual + deploy. It also takes a **box-wide lock** (`~/.agentric-live.lock`): two concurrent runs share every + live checkout, so the second one's `git reset --hard` lands inside the first one's build and the first + restarts a service on a binary it never resolved — while reporting success, because it verifies + `/health` against the version it *expected*. Two sessions did exactly this on 2026-09-17 and got away + with it only because both were on the same sha. A second run now fails fast naming the holder; a dead + or ancient lock clears itself, and `--force-lock` breaks one deliberately. `--dry-run`/`--help` are + never blocked. The manual equivalent is `npm run build && launchctl kickstart -k gui/$(id -u)/com.agentos.northwind`; logs at `~/agent-os-data/northwind/server.log`; load/unload with `launchctl load -w|unload `.) - **Agent-facing MCP tools (`src/memory/memory-mcp.ts` — `recall`/`remember`/`revise`/`forget`, the diff --git a/package-lock.json b/package-lock.json index 7ed0b926..dbef313c 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "agent-os", - "version": "0.448.2", + "version": "0.448.4", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "agent-os", - "version": "0.448.2", + "version": "0.448.4", "license": "MIT", "bin": { "agent-os": "bin/agent-os" diff --git a/package.json b/package.json index 194b0ca9..5f0bee6a 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "agent-os", - "version": "0.448.2", + "version": "0.448.4", "description": "A generic, governed operating system for running autonomous agents safely across brands. Ships with a local web console.", "license": "MIT", "type": "commonjs", @@ -27,7 +27,7 @@ "check-deps": "bash scripts/install-deps.sh --check", "dev": "ts-node src/cli.ts serve", "demo:dev": "ts-node src/demo.ts", - "test:governance": "node scripts/version-sync-test.cjs && node scripts/governance-conformance.cjs && node scripts/tier-a-policy-test.cjs && node scripts/policy-baseline-test.cjs && node scripts/heredoc-intent-test.cjs && node scripts/capability-registry-test.cjs && node scripts/composio-envelope-test.cjs && node scripts/composio-identity-test.cjs && node scripts/idle-reaper-test.cjs && node scripts/dm-continuity-test.cjs && node scripts/telegram-dm-lane-test.cjs && node scripts/cli-link-origin-test.cjs && node scripts/alert-staleness-test.cjs && node scripts/run-as-identity-test.cjs && node scripts/email-identity-guard-test.cjs && node scripts/deps-freshness-test.cjs && node scripts/runtime-account-test.cjs && node scripts/runtime-account-misattribution-test.cjs && node scripts/runtime-usage-refresh-test.cjs && node scripts/keychain-credential-test.cjs && node scripts/credential-preflight-test.cjs && node scripts/capacity-queue-test.cjs && node scripts/runtime-login-test.cjs && node scripts/rotate-on-reload-test.cjs && node scripts/headless-resumable-test.cjs && node scripts/session-revive-gates-test.cjs && node scripts/session-pause-test.cjs && node scripts/claude-config-seed-test.cjs && node scripts/claude-config-isolation-test.cjs && node scripts/output-style-test.cjs && node scripts/session-cost-test.cjs && node scripts/chain-model-test.cjs && node scripts/task-workers-test.cjs && node scripts/tuning-patch-test.cjs && node scripts/task-runs-test.cjs && node scripts/task-pr-links-test.cjs && node scripts/task-draft-delete-test.cjs && node scripts/task-discussion-delivery-test.cjs && node scripts/task-resume-test.cjs && node scripts/task-unblock-test.cjs && node scripts/audience-session-access-test.cjs && node scripts/warm-chat-test.cjs && node scripts/poke-warm-caller-test.cjs && node scripts/wakeup-queue-test.cjs && node scripts/stranded-human-stop-test.cjs && node scripts/inject-submit-test.cjs && node scripts/blocked-routing-test.cjs && node scripts/self-dispatch-guard-test.cjs && node scripts/task-proposals-test.cjs && node scripts/npm-boundary-test.cjs && node scripts/agent-edit-guard-test.cjs && node scripts/per-agent-context-test.cjs && node scripts/goal-update-guard-test.cjs && node scripts/insights-signal-test.cjs && node scripts/outcome-derivation-test.cjs && node scripts/episode-quality-test.cjs && node scripts/memory-upkeep-test.cjs && node scripts/automem-health-test.cjs && node scripts/memory-store-switch-test.cjs && node scripts/memory-preload-test.cjs && node scripts/turn-lifecycle-test.cjs && node scripts/resume-seed-test.cjs && node scripts/outcome-vocabulary-test.cjs && node scripts/skill-presets-test.cjs && node scripts/skill-edit-proposal-test.cjs && node scripts/notify-hook-route-test.cjs && node scripts/review-notify-test.cjs && node scripts/turn-idle-background-guard-test.cjs && node scripts/waiting-brief-test.cjs && node scripts/runtime-death-alert-test.cjs && node scripts/github-per-member-test.cjs && node scripts/github-multi-org-test.cjs && node scripts/card-measurement-test.cjs && node scripts/scheduler-admission-test.cjs && node scripts/tick-liveness-test.cjs && node scripts/audit-mirror-test.cjs && node scripts/request-metrics-test.cjs && node scripts/tool-usage-test.cjs && node scripts/sessions-list-perf-test.cjs && node scripts/summarizer-degradation-test.cjs && node scripts/agent-history-scope-test.cjs && node scripts/webhook-ingress-test.cjs && node scripts/slack-content-filter-test.cjs && node scripts/slack-ingress-test.cjs && node scripts/discord-ingress-test.cjs && node scripts/chat-attachments-test.cjs && node scripts/clickup-task-bridge-test.cjs && node scripts/agentric-commands-test.cjs && node scripts/whats-new-test.cjs && node scripts/opencode-gate-test.cjs && node scripts/protected-path-guard-test.cjs && node scripts/attach-grace-test.cjs && node scripts/attach-file-liveness-test.cjs && node scripts/feed-smoke.cjs && node scripts/activity-classify-test.cjs && node scripts/goal-room-test.cjs && node scripts/secret-rotation-test.cjs && node scripts/update-watch-test.cjs && node scripts/runtime-update-watch-test.cjs && node scripts/setup-wizard-test.cjs && node scripts/md-pdf-test.cjs && node scripts/proposal-surfacing-test.cjs && node scripts/process-janitor-test.cjs && node scripts/detached-work-steer-test.cjs && node scripts/statusline-install-test.cjs && node scripts/docs-create-agent-test.cjs && node scripts/agent-stats-rollup-test.cjs && node scripts/task-discussion-rollup-test.cjs && node scripts/session-insights-stamp-test.cjs && node scripts/loop-stall-attribution-test.cjs && node scripts/session-progress-test.cjs && node scripts/drift-nudge-test.cjs && node scripts/goal-metric-review-test.cjs && node scripts/capability-gap-test.cjs && node scripts/workflow-proposal-test.cjs && node scripts/automation-edit-proposal-test.cjs && node scripts/app-card-close-test.cjs", + "test:governance": "node scripts/version-sync-test.cjs && node scripts/governance-conformance.cjs && node scripts/tier-a-policy-test.cjs && node scripts/policy-baseline-test.cjs && node scripts/heredoc-intent-test.cjs && node scripts/capability-registry-test.cjs && node scripts/composio-envelope-test.cjs && node scripts/composio-identity-test.cjs && node scripts/idle-reaper-test.cjs && node scripts/dm-continuity-test.cjs && node scripts/telegram-dm-lane-test.cjs && node scripts/cli-link-origin-test.cjs && node scripts/alert-staleness-test.cjs && node scripts/run-as-identity-test.cjs && node scripts/email-identity-guard-test.cjs && node scripts/deps-freshness-test.cjs && node scripts/runtime-account-test.cjs && node scripts/runtime-account-misattribution-test.cjs && node scripts/runtime-usage-refresh-test.cjs && node scripts/keychain-credential-test.cjs && node scripts/credential-preflight-test.cjs && node scripts/capacity-queue-test.cjs && node scripts/runtime-login-test.cjs && node scripts/rotate-on-reload-test.cjs && node scripts/headless-resumable-test.cjs && node scripts/session-revive-gates-test.cjs && node scripts/session-pause-test.cjs && node scripts/make-live-lock-test.cjs && node scripts/claude-config-seed-test.cjs && node scripts/claude-config-isolation-test.cjs && node scripts/output-style-test.cjs && node scripts/session-cost-test.cjs && node scripts/chain-model-test.cjs && node scripts/task-workers-test.cjs && node scripts/tuning-patch-test.cjs && node scripts/task-runs-test.cjs && node scripts/task-pr-links-test.cjs && node scripts/task-draft-delete-test.cjs && node scripts/task-discussion-delivery-test.cjs && node scripts/task-resume-test.cjs && node scripts/task-unblock-test.cjs && node scripts/audience-session-access-test.cjs && node scripts/warm-chat-test.cjs && node scripts/poke-warm-caller-test.cjs && node scripts/wakeup-queue-test.cjs && node scripts/stranded-human-stop-test.cjs && node scripts/inject-submit-test.cjs && node scripts/blocked-routing-test.cjs && node scripts/self-dispatch-guard-test.cjs && node scripts/task-proposals-test.cjs && node scripts/npm-boundary-test.cjs && node scripts/agent-edit-guard-test.cjs && node scripts/per-agent-context-test.cjs && node scripts/goal-update-guard-test.cjs && node scripts/insights-signal-test.cjs && node scripts/outcome-derivation-test.cjs && node scripts/episode-quality-test.cjs && node scripts/memory-upkeep-test.cjs && node scripts/automem-health-test.cjs && node scripts/memory-store-switch-test.cjs && node scripts/memory-preload-test.cjs && node scripts/turn-lifecycle-test.cjs && node scripts/resume-seed-test.cjs && node scripts/outcome-vocabulary-test.cjs && node scripts/skill-presets-test.cjs && node scripts/skill-edit-proposal-test.cjs && node scripts/notify-hook-route-test.cjs && node scripts/review-notify-test.cjs && node scripts/turn-idle-background-guard-test.cjs && node scripts/waiting-brief-test.cjs && node scripts/runtime-death-alert-test.cjs && node scripts/github-per-member-test.cjs && node scripts/github-multi-org-test.cjs && node scripts/card-measurement-test.cjs && node scripts/scheduler-admission-test.cjs && node scripts/tick-liveness-test.cjs && node scripts/audit-mirror-test.cjs && node scripts/request-metrics-test.cjs && node scripts/tool-usage-test.cjs && node scripts/sessions-list-perf-test.cjs && node scripts/summarizer-degradation-test.cjs && node scripts/agent-history-scope-test.cjs && node scripts/webhook-ingress-test.cjs && node scripts/slack-content-filter-test.cjs && node scripts/slack-ingress-test.cjs && node scripts/discord-ingress-test.cjs && node scripts/chat-attachments-test.cjs && node scripts/clickup-task-bridge-test.cjs && node scripts/agentric-commands-test.cjs && node scripts/whats-new-test.cjs && node scripts/opencode-gate-test.cjs && node scripts/protected-path-guard-test.cjs && node scripts/attach-grace-test.cjs && node scripts/attach-file-liveness-test.cjs && node scripts/feed-smoke.cjs && node scripts/activity-classify-test.cjs && node scripts/goal-room-test.cjs && node scripts/secret-rotation-test.cjs && node scripts/update-watch-test.cjs && node scripts/runtime-update-watch-test.cjs && node scripts/setup-wizard-test.cjs && node scripts/md-pdf-test.cjs && node scripts/proposal-surfacing-test.cjs && node scripts/process-janitor-test.cjs && node scripts/detached-work-steer-test.cjs && node scripts/statusline-install-test.cjs && node scripts/docs-create-agent-test.cjs && node scripts/agent-stats-rollup-test.cjs && node scripts/task-discussion-rollup-test.cjs && node scripts/session-insights-stamp-test.cjs && node scripts/loop-stall-attribution-test.cjs && node scripts/session-progress-test.cjs && node scripts/drift-nudge-test.cjs && node scripts/goal-metric-review-test.cjs && node scripts/capability-gap-test.cjs && node scripts/workflow-proposal-test.cjs && node scripts/automation-edit-proposal-test.cjs && node scripts/app-card-close-test.cjs", "test:alert-staleness": "node scripts/alert-staleness-test.cjs", "test:deps": "node scripts/deps-freshness-test.cjs && node scripts/runtime-account-test.cjs && node scripts/runtime-account-misattribution-test.cjs && node scripts/runtime-login-test.cjs && node scripts/claude-config-seed-test.cjs && node scripts/claude-config-isolation-test.cjs", "test:dm-continuity": "node scripts/dm-continuity-test.cjs", diff --git a/scripts/make-live-lock-test.cjs b/scripts/make-live-lock-test.cjs new file mode 100644 index 00000000..170a121c --- /dev/null +++ b/scripts/make-live-lock-test.cjs @@ -0,0 +1,130 @@ +#!/usr/bin/env node +/* The deploy lock — one `make-live.sh` at a time per box. + * + * Every live checkout is shared by whoever runs the deploy, so two concurrent runs are two + * `git reset --hard`s racing each other's builds: the second one moves a checkout out from under the + * first one's `npm run build`, and the first then restarts a service with a binary built from a commit + * it never resolved — while reporting success, because it verifies /health against the version it was + * told to expect. On 2026-09-17 two sessions ran this concurrently against all four tenants and it was + * harmless ONLY because both happened to be deploying the same sha. Luck, not design. + * + * A lock that cannot be released is worse than no lock, so the other half of this is the escape hatches: + * a dead holder is cleared automatically, an ancient one is cleared on the age ceiling, and --force-lock + * breaks one on purpose. Each is pinned here, because each is the thing that would wedge every future + * deploy if it regressed. + * + * Runs the real script with a scratch lock path and a deliberately bogus target, so every case fails + * fast in preflight — after the lock has been taken and released. Nothing is deployed and no checkout is + * touched. */ +const fs = require('fs'); +const os = require('os'); +const path = require('path'); +const { spawnSync } = require('child_process'); + +const ROOT = path.resolve(__dirname, '..'); +const SCRIPT = path.join(ROOT, 'scripts/make-live.sh'); +const TMP = fs.mkdtempSync(path.join(os.tmpdir(), 'aos-deploy-lock-')); +const LOCK = path.join(TMP, 'deploy.lock'); + +let pass = 0, fail = 0; +const assert = (c, name, d) => c ? (pass++, console.log(` \x1b[32m✓\x1b[0m ${name}`)) : (fail++, console.log(` \x1b[31m✗ ${name}\x1b[0m${d !== undefined ? ' — ' + String(d).slice(0, 300) : ''}`)); + +// An env file naming one local tenant whose checkout does not exist: preflight fails within milliseconds, +// which is exactly what we want — the lock is acquired and released around a run that deploys nothing. +const ENV_FILE = path.join(TMP, 'live.env'); +fs.writeFileSync(ENV_FILE, `AOS_LIVE_TARGETS="faketenant:${path.join(TMP, 'no-such-checkout')}:3999"\n`); + +const run = (args = []) => spawnSync('bash', [SCRIPT, ...args], { + encoding: 'utf8', + env: { ...process.env, AOS_LIVE_ENV: ENV_FILE, AOS_LIVE_LOCK: LOCK }, +}); +const held = () => fs.existsSync(LOCK); +/** Plant a lock as if another run owned it. */ +const plant = (pid, startedAgoSec = 0) => { + fs.rmSync(LOCK, { recursive: true, force: true }); + fs.mkdirSync(LOCK); + fs.writeFileSync(path.join(LOCK, 'meta'), `pid=${pid}\nuser=someone@otherbox\nstarted=planted\nargs=\n`); + if (startedAgoSec) { + const t = new Date(Date.now() - startedAgoSec * 1000); + fs.utimesSync(LOCK, t, t); + } +}; +/** A PID that is alive for the duration of this test but is NOT a deploy. */ +const sleeper = spawnSync === null ? null : require('child_process').spawn('sleep', ['120'], { stdio: 'ignore' }); +const LIVE_PID = sleeper.pid; +const DEAD_PID = 999999; // far above any live pid on a desktop box + +console.log('\n\x1b[1m1) an ordinary run takes the lock and always gives it back\x1b[0m'); +{ + fs.rmSync(LOCK, { recursive: true, force: true }); + const r = run(); + // It must fail in PREFLIGHT (no checkout / no launchctl), never at the lock. + assert(r.status !== 0, 'the run fails fast on the bogus target', r.status); + assert(!/another deploy is already running/.test(r.stderr), 'and not because of the lock', r.stderr.slice(0, 200)); + assert(!held(), 'the lock is released even though the run FAILED — a crashed deploy must not wedge the next one'); +} + +console.log('\n\x1b[1m2) a second run is refused while a live holder has it\x1b[0m'); +{ + plant(LIVE_PID); + const r = run(); + assert(r.status === 1, 'it exits non-zero', r.status); + assert(/another deploy is already running/.test(r.stderr), 'it says a deploy is already running', r.stderr.slice(0, 200)); + assert(r.stderr.includes(String(LIVE_PID)), 'it names the holding pid so you can go look at it'); + assert(/--force-lock/.test(r.stderr), 'and it prints the way out'); + assert(held(), 'the LOSER does not delete the winner\'s lock'); + // The refusal must come before any deploy work — the bogus-target error proves we never got to preflight. + assert(!/no live checkout/.test(r.stderr), 'and it refuses BEFORE touching any target'); +} + +console.log('\n\x1b[1m3) a lock nobody holds never wedges the next deploy\x1b[0m'); +{ + plant(DEAD_PID); + const r = run(); + assert(/stale deploy lock/.test(r.stderr), 'a dead holder is cleared automatically', r.stderr.slice(0, 200)); + assert(!/another deploy is already running/.test(r.stderr), 'and the run proceeds past the lock'); + assert(!held(), 'and it is released again at exit'); +} +{ + // A live PID can still be debris — it may be a REUSED pid, or a run wedged on an ssh that never + // returns. The age ceiling is what stops either from blocking deploys forever. + plant(LIVE_PID, 3 * 3600); + const r = run(); + assert(/stale deploy lock/.test(r.stderr), 'a lock past the age ceiling is cleared even with a live pid', r.stderr.slice(0, 200)); + assert(!held(), 'and released at exit'); +} +{ + // A meta we cannot read at all (an interrupted writer) must not be a permanent wedge either. + fs.rmSync(LOCK, { recursive: true, force: true }); + fs.mkdirSync(LOCK); + const r = run(); + assert(/stale deploy lock/.test(r.stderr), 'a lock with no readable meta is cleared', r.stderr.slice(0, 200)); +} + +console.log('\n\x1b[1m4) --force-lock breaks a live holder on purpose\x1b[0m'); +{ + plant(LIVE_PID); + const r = run(['--force-lock']); + assert(/breaking the lock/.test(r.stderr), 'it says what it is doing', r.stderr.slice(0, 200)); + assert(!/another deploy is already running/.test(r.stderr), 'it is not refused'); + assert(!held(), 'and the forcing run releases the lock it took'); +} + +console.log('\n\x1b[1m5) the read-only paths are not blocked by a held lock\x1b[0m'); +{ + plant(LIVE_PID); + const r = run(['--dry-run']); + assert(!/another deploy is already running/.test(r.stderr), 'a dry run is not refused — it changes nothing, and it is the tool you reach for to see what the other run is doing'); + assert(/deploy is in progress/.test(r.stderr), 'but it warns that it is reading checkouts another run is moving', r.stderr.slice(0, 200)); + assert(held(), 'and it leaves the holder\'s lock alone'); + + const h = run(['--help']); + assert(h.status === 0, '--help still works with a lock held', h.status); + assert(/--force-lock/.test(h.stdout), 'and documents --force-lock'); + assert(held(), 'and touches nothing'); +} + +try { sleeper.kill(); } catch { /* best effort */ } +fs.rmSync(TMP, { recursive: true, force: true }); +console.log(`\n${fail ? '\x1b[31m' : '\x1b[32m'}${pass} passed, ${fail} failed\x1b[0m`); +process.exit(fail ? 1 : 0); diff --git a/scripts/make-live.sh b/scripts/make-live.sh index a782f047..2906a424 100755 --- a/scripts/make-live.sh +++ b/scripts/make-live.sh @@ -11,6 +11,7 @@ # scripts/make-live.sh --dry-run # show what WOULD deploy, change nothing # scripts/make-live.sh --skip-tests # skip the governance suite gate # scripts/make-live.sh --force # discard local changes in a live checkout +# scripts/make-live.sh --force-lock # break another run's lock (see below) and deploy anyway # # WHICH tenants belong to your box is your deployment's business, not this repo's — it comes from an # untracked env file (see below). Either form works: @@ -54,6 +55,11 @@ # - EVERY checkout lands on the SAME commit: $REF is resolved once, up front, and each checkout is # pinned to that sha. Resolving per-checkout raced whoever was merging — a box that fetched a second # later deployed a different commit, and the run still reported success. +# - ONE deploy at a time per box (the lock below). Two runs at once share every live checkout, so the +# second one's `git reset --hard` lands in the middle of the first one's build: the first then +# restarts a service with a binary built from a commit it never resolved, and reports success +# against the version it was told to expect. On 2026-09-17 two sessions ran this concurrently and it +# was harmless ONLY because both happened to be deploying the same sha — luck, not design. set -euo pipefail ENV_FILE="${AOS_LIVE_ENV:-$HOME/.agentric-live.env}" @@ -62,15 +68,17 @@ ENV_FILE="${AOS_LIVE_ENV:-$HOME/.agentric-live.env}" REF="${AOS_LIVE_REF:-origin/main}" -DRY=0; SKIP_TESTS=0; FORCE=0; ONLY="" +DRY=0; SKIP_TESTS=0; FORCE=0; FORCE_LOCK=0; ONLY="" +ORIG_ARGS="$*" # recorded in the lock meta, so a refusal can say what the holder is doing while [ $# -gt 0 ]; do case "$1" in --dry-run) DRY=1 ;; --skip-tests) SKIP_TESTS=1 ;; --force) FORCE=1 ;; + --force-lock) FORCE_LOCK=1 ;; --only) shift; ONLY="${1:?--only needs a tenant slug}" ;; --only=*) ONLY="${1#--only=}" ;; - -h|--help) sed -n '2,48p' "$0"; exit 0 ;; + -h|--help) sed -n '2,62p' "$0"; exit 0 ;; *) echo "unknown option: $1 (try --help)" >&2; exit 2 ;; esac shift @@ -80,6 +88,95 @@ say() { printf '\033[36m%s\033[0m\n' "$*"; } warn() { printf '\033[33m%s\033[0m\n' "$*" >&2; } fail() { printf '\033[31m%s\033[0m\n' "$*" >&2; exit 1; } +# ── one deploy at a time on this box ───────────────────────────────────────────── +# Every live checkout is shared by whoever runs this, so two concurrent runs are two `git reset --hard`s +# racing each other's builds. `mkdir` is the lock because it is atomic on every POSIX filesystem and needs +# no `flock` (macOS ships none) — the directory IS the mutex, and the meta file inside only explains who +# holds it. +# +# Fail fast rather than queue: a deploy that waits ten minutes and then runs against whatever `main` has +# become by then is its own surprise. The refusal names the holder and prints the two ways out. +# +# Staleness is a live PID plus a ceiling. `kill -0` catches the common case (a run that was Ctrl-C'd or +# whose terminal died), and the age ceiling catches the rest — a wedged run, or the PID being reused by +# something else. No deploy has ever taken an hour, so a lock older than the ceiling is debris. +# +# Scope: this guards one BOX. Two different machines deploying to the same REMOTE tenant still race; that +# needs a lock on the remote side and is not what bit us. +LOCK="${AOS_LIVE_LOCK:-$HOME/.agentric-live.lock}" +LOCK_MAX_AGE="${AOS_LIVE_LOCK_MAX_AGE:-7200}" # seconds; 0 disables the ceiling +LOCK_HELD=0 # 1 once WE own it — the loser must never release it + +# Seconds since the lock dir was created, or 0 when that can't be read. +# +# BSD and GNU `stat` do not merely differ in FORMAT, they disagree on what `-f` MEANS: on BSD it +# introduces the format string, on GNU it is `--file-system` and `%m` is read as another FILE operand. +# So on Linux the BSD attempt prints a filesystem dump beginning `File: "…"` to stdout and THEN fails — +# and chaining the two with `||` inside one `$(…)` captures that dump along with the GNU answer. The +# multi-line result reached `$(( … ))`, which evaluated `File` as a variable and, under `set -u`, killed +# the script mid-lock with `File: unbound variable`. Caught by CI, invisible on the macOS box. +# +# So: try each separately, and accept a candidate only if it is all digits. That also makes any other +# userland surprise degrade to "age unknown" instead of a crash. +lock_age() { + local t="" + t="$(stat -f %m "$LOCK" 2>/dev/null || true)" # BSD/macOS + case "$t" in ''|*[!0-9]*) t="$(stat -c %Y "$LOCK" 2>/dev/null || true)" ;; esac # GNU/Linux + case "$t" in ''|*[!0-9]*) echo 0; return ;; esac + echo $(( $(date +%s) - t )) +} + +release_lock() { [ "$LOCK_HELD" = 1 ] && rm -rf "$LOCK"; LOCK_HELD=0; } +# ONE exit trap for the whole script — the scratch-state dir below is created later, so cleanup has to +# tolerate it not existing yet. +cleanup() { [ -n "${STATE:-}" ] && rm -rf "$STATE"; release_lock; return 0; } + +acquire_lock() { + local age holder pid started + if mkdir "$LOCK" 2>/dev/null; then + LOCK_HELD=1 + printf 'pid=%s\nuser=%s\nstarted=%s\nargs=%s\n' "$$" "${USER:-?}@$(hostname -s 2>/dev/null || echo '?')" "$(date '+%Y-%m-%d %H:%M:%S')" "${ORIG_ARGS:-}" > "$LOCK/meta" 2>/dev/null || true + return 0 + fi + # Held. Decide whether the holder is real before refusing — a lock nothing can release is worse than + # no lock at all. + # + # Read the meta into a variable FIRST rather than sed-ing the file per field. Under `set -euo pipefail` + # a `sed missing-file | head` pipeline exits non-zero, which killed the script outright — silently, with + # the lock still in place. The one case that reaches it is an interrupted writer (dir created, meta not + # yet written), i.e. precisely the debris this branch exists to clear. + local meta="" + if [ -r "$LOCK/meta" ]; then meta="$(cat "$LOCK/meta" 2>/dev/null || true)"; fi + pid="$(printf '%s\n' "$meta" | sed -n 's/^pid=//p' | head -1)" + started="$(printf '%s\n' "$meta" | sed -n 's/^started=//p' | head -1)" + holder="$(printf '%s\n' "$meta" | sed -n 's/^user=//p' | head -1)" + age="$(lock_age)" + if [ -n "$pid" ] && kill -0 "$pid" 2>/dev/null \ + && { [ "$LOCK_MAX_AGE" = 0 ] || [ "$age" -lt "$LOCK_MAX_AGE" ]; }; then + [ "$FORCE_LOCK" = 1 ] || fail "another deploy is already running on this box (pid $pid, ${holder:-?}, started ${started:-?}). + Deploys share every live checkout, so running two at once can restart a service on a half-synced build. + Wait for it to finish, or — if you are certain it is dead — re-run with --force-lock." + warn "--force-lock: breaking the lock held by pid $pid (${holder:-?}, started ${started:-?})" + else + # Nobody home: a dead PID, a lock past the ceiling, or a meta we could not read at all. + warn "clearing a stale deploy lock at $LOCK (pid ${pid:-unknown}, age ${age}s) — no live holder" + fi + rm -rf "$LOCK" + mkdir "$LOCK" 2>/dev/null || fail "could not take the deploy lock at $LOCK" + LOCK_HELD=1 + printf 'pid=%s\nuser=%s\nstarted=%s\nargs=%s\n' "$$" "${USER:-?}@$(hostname -s 2>/dev/null || echo '?')" "$(date '+%Y-%m-%d %H:%M:%S')" "${ORIG_ARGS:-}" > "$LOCK/meta" 2>/dev/null || true +} + +trap cleanup EXIT +if [ "$DRY" = 1 ]; then + # A dry run changes nothing, so it is never the thing that corrupts a checkout — refusing it would only + # take away the tool you reach for to find out what the other run is doing. Say the deploy is in flight + # (its report will be read against a moving checkout) and carry on. + [ -d "$LOCK" ] && warn "note: a deploy is in progress on this box — this dry run reads checkouts it is moving" +else + acquire_lock +fi + # ── the target list ────────────────────────────────────────────────────────────── # Normalised to one `tenant:checkout:port:label` line per tenant, so everything below iterates over a # single shape whichever env form configured it. @@ -148,8 +245,7 @@ fi # Per-checkout scratch state (bash 3.2 on macOS has no associative arrays): one file per checkout, # named after its path, holding the commit it was on before the sync — that's what a rollback needs. -STATE="$(mktemp -d)" -trap 'rm -rf "$STATE"' EXIT +STATE="$(mktemp -d)" # cleaned up by the `cleanup` EXIT trap installed with the deploy lock above key_for() { printf '%s' "$1" | tr -c 'A-Za-z0-9' '_'; } log_for() { # tenant → its server.log (AOS_LIVE_LOG only names the single-tenant one)