From 333d4353a2be21afb34d016ea197a239eb2bcf95 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 13 Oct 2026 15:03:05 +0000 Subject: [PATCH 1/5] Guard against a leftover mxcli test injection; verify app-restart's pid; serialise test-scratch takeovers; lint-a-copy recipe #244 (BUG-157): test-scratch.sh and app-restart.sh read AfterStartupMicroflow from the settings unit's bytes (no mxcli) and refuse to start while it is MxTest.RegisterEndpoint or MxTest.TestRunner, printing the git / alter-settings / Studio Pro repair with HEAD's value. app-restart lets a live --test-endpoint app through and re-checks after the stop (new exit 5). Golden: a settings unit captured from mxcli's own test model, re-encoded byte-faithfully per case. Second field case credited to AlvinnYeo (PR #309 inbox note). Review of #304: app-restart verifies the recorded pid is itself an mxcli run before stopping (a reused pid with --cmd was killed); test-scratch's stale-lock takeover is serialised by an O_EXCL .takeover marker and re-read, restored with ln, not mv -f. #240 (BUG-158): skills/lint-that-actually-runs.md gains the copy-the-rules-first recipe and the rule-count comparison before any diff. Refs #244 #240 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01VJgWP5vEoAsNsJYqCDMGNw --- CHANGELOG.md | 4 + bug-logs/mxcli-bugs.md | 72 +++++++++++ project-bin/app-restart.sh | 119 +++++++++++++++++- project-bin/test-scratch.sh | 105 +++++++++++++++- skills/lint-that-actually-runs.md | 46 +++++++ .../mxtest-guard/set-after-startup.js | 34 +++++ .../mxtest-guard/settings-unit.mxunit | Bin 0 -> 2684 bytes tests/wave2/test-app-restart.sh | 64 +++++++++- tests/wave2/test-test-scratch.sh | 68 ++++++++++ 9 files changed, 503 insertions(+), 9 deletions(-) create mode 100644 tests/wave2/fixtures/mxtest-guard/set-after-startup.js create mode 100644 tests/wave2/fixtures/mxtest-guard/settings-unit.mxunit diff --git a/CHANGELOG.md b/CHANGELOG.md index 20342404..f514aba5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,10 @@ Sections dated before 2026-09-19 predate the cycle and stay as they are. ## Unreleased +- fix(test-scratch, app-restart): **both refuse to start on a model whose after-startup is still an mxcli test injection (`MxTest.RegisterEndpoint` / `MxTest.TestRunner`), and print the repair** ([#244](https://github.com/MendixMau/mxcli-project-toolkit/issues/244), BUG-157). A `mxcli test` or `run --test-endpoint` stopped by a signal skips its restore, and the next injection chains `RegisterEndpoint` to itself: StackOverflowError, app down. It happened twice in one day on one machine, and again on a second project. The setting is read from the settings unit's bytes, so no mxcli is needed. The fixture uses a settings unit captured from mxcli's own test model. `test-scratch.sh` exits 3 before copying. `app-restart.sh` exits 3 before stopping, except for a live `--test-endpoint` app, which holds the injection legitimately; if stopping that app leaves the setting behind, it exits 5 (new): stopped, not started. Projects pick this up on sync; a clean model sees no change. — MendixMau, second case AlvinnYeo +- fix(app-restart): **the recorded pid is verified to still be an `mxcli run` before anything is stopped** (review of #304). With `--cmd` / `MXTK_APP_START_CMD`, the identity check read the override, not the process. A record left by a SIGKILL whose pid the OS had reused sent TERM/KILL to that unrelated process and its children. The pid's own command line (`/proc`, `ps`; on Windows the image name `mxcli.exe`) must now name mxcli and `run`. Otherwise the app counts as not running and the process is not touched. — MendixMau +- fix(test-scratch): **taking over a stale lock is serialised, so two runs can no longer both hold it** (review of #304). The takeover holds an O_EXCL `.takeover` marker (cleared after 30s), re-reads the lock, and removes it only if it still holds the line judged stale. A changed lock is put back with `ln`, never `mv -f`. A deterministic replay of the race fails on the old code. — MendixMau +- learn(lint-that-actually-runs): **lint a copy of the model only after copying `.claude/lint-rules` (and `lint-config.yaml`) beside it, and compare rule counts before diffing** ([#240](https://github.com/MendixMau/mxcli-project-toolkit/issues/240), BUG-158). A snapshot in `/tmp` gave 51 findings from 8 rules, against 721 from 29 for the project: no notice, exit 0. mxcli's source also shows `lint-config.yaml` is read only from beside the `.mpr`, so an in-repo snapshot loses it as well. — MendixMau - learn(testing-shape, test-result-audit): **three test-strength rules, each with a denominator: refusals probed on the server, every undeclared 4xx a failure, one clean-database final run** ([#172](https://github.com/MendixMau/mxcli-project-toolkit/issues/172)). A field comparison of two builds against one frozen spec found the toolkit build checked permission refusals only in the client (a hidden menu item), logged unexpected 400/422 responses as information, and reached 20/20 by stitching three runs. `testing-shape.md` §7 gains three report lines (`N of N restricted pairs probed server-side`, `undeclared 4xx/5xx: 0 of M requests — each fails its step`, `one clean-DB run : N of N scenarios incl. timers, not stitched`); `test-result-audit.md` gains check 2e, which carries the reasons and the field evidence and asks for those numbers. Both baseline skills stay inside the 80,000-word baseline budget (79,997): the reasons live in the on-demand audit skill, and two redundant sentences in `testing-shape.md` §4's verdict paragraphs were folded (the empty-set corollary is stated in the Trace row, `harness-architecture.md` and `report-schema.md`). The backing inbox note is trimmed to its unpromoted "Keep" items. — MendixMau - learn(learned-detection-gaps, module-review): **two UI defects every model gate passes: a lazy-loaded combo box filter hides its preset, and a theme that pins row heights by position lets the next row cover an inserted widget** ([#299](https://github.com/MendixMau/mxcli-project-toolkit/issues/299)). Both passed check, mxbuild, lint and the scripted checks on a 127-module production app and were found only in the running app. Two register rows with the first oracle and the pre-build check (pluggable properties that change *when* data loads; grep the theme for fixed `height` on positional row classes before an insert), and two LOOK lines in `module-review.md` §4c with denominators (`N of N filters show their preset on first load`, `N of N widgets inserted into an existing container fully visible`). — jopterhorst - fix(app-restart, test-stack-up): **a port held by another process before `run --local` is named and reported as "instrument blocked" (exit 4), never killed** ([#262](https://github.com/MendixMau/mxcli-project-toolkit/issues/262)). Studio Pro had started its own runtime on the app/admin ports a restart needed; `mxcli run --local` then failed with "Authentication failed" (the admin API answering was Studio Pro's), the next journey ran against Studio Pro's runtime, and four more runs were NOT RUN. `app-restart.sh` now checks the app port and the admin port (record's `adminPort`, else app + 10) before stopping anything — a holder outside the recorded process tree exits 4 with its name, pid and parent — and again after the stop, where anything still holding a port is by definition not ours. `test-stack-up.sh` runs the same check wherever it would tell you to start `run --local` (`--no-docker`, no Docker daemon, `--check` with a leftover record), on the record's ports, `RUN_LOCAL_APP_PORT`/`RUN_LOCAL_ADMIN_PORT`, or mxcli's defaults 8080/8090. Holder lookup: lsof, then ss, then netstat + tasklist on Windows; unnamed holders still block when nothing of ours is running. Fixtures: `test-app-restart.sh` +2 cases (14 assertions, all fail on the old script), `test-stack-up-local.sh` T7 (4 of 6 fail on the old script). — AlvinnYeo diff --git a/bug-logs/mxcli-bugs.md b/bug-logs/mxcli-bugs.md index 5bdcbc70..6e80b6e0 100644 --- a/bug-logs/mxcli-bugs.md +++ b/bug-logs/mxcli-bugs.md @@ -6851,3 +6851,75 @@ passes `mxcli check`, and `mx check` then fails with CE1613. **Workaround:** never treat `describe` output as a source to replay without an `mx check`; qualify the caption parameter (as BUG-23 / BUG-142 describe) before re-executing. + +## BUG-157: `mxcli test` / `run --test-endpoint` stopped by a signal leave AfterStartupMicroflow on `MxTest.RegisterEndpoint`; the next injection chains it to itself (StackOverflowError, the app never starts) + +**Severity:** High. An app goes down with no model change by the user, and the cause (a settings unit) is not where anyone looks. +**mxcli version:** v0.25.0 (field). Same code in v0.26.0 source (`cmd/mxcli/testrunner`, `12ce281`). **Mendix version:** 11.15.0 and 11.14.0 Beta +**Discovered:** 2026-10, three apps built side by side (#244): two hits in one day, one app down. A second field case on another project, 2026-10-13, reported by AlvinnYeo (open PR #309, inbox note) +**Reproducible:** yes. Probe from the second report: start `mxcli run --local --test-endpoint` on a scratch copy, `kill -TERM` it, then `describe settings`. After-startup is still `MxTest.RegisterEndpoint` on v0.25.0. +**Not filed upstream** (draft prepared, issue #244) + +**How it happens.** `mxcli test` and `run --test-endpoint` read the current after-startup +microflow, generate `MxTest.RegisterEndpoint` (plain `mxcli test`: `MxTest.TestRunner`) that calls +it after registering the endpoint, and point after-startup at the generated flow. Only a clean exit +runs the restore (`ALTER SETTINGS RUNTIME (AfterStartupMicroflow: '')`). A run stopped by +a signal (an agent's background-task stop, a kill) skips it. The setting stays on the injected flow +in the on-disk model, where every other mxcli process sees it. The next injection then reads +`MxTest.RegisterEndpoint` as "the original" and generates a flow that calls itself: + +``` +ERROR - Core: An exception occurred while running the after-startup-action. +com.mendix.modules.microflowengine.MicroflowException: java.lang.StackOverflowError + at MxTest.RegisterEndpoint (SubMicroflow : 'RegisterEndpoint') + at MxTest.RegisterEndpoint (SubMicroflow : 'RegisterEndpoint') +ERROR - M2EE: Starting Mendix Runtime failed. +``` + +A partial clean-up makes it worse. If the `MxTest` module is removed but the setting is not, the +after-startup names a flow that does not exist: CE1613 at build, which `exec.sh`'s delta gate then +carries as a "pre-existing" error on every later script. + +**Toolkit guard (#244).** `project-bin/test-scratch.sh` and `project-bin/app-restart.sh` read the +setting from the model bytes before starting. The setting is the BSON string `AfterStartupMicroflow` +in the project-settings unit (`mprcontents/4e/70/4e70063d-….mxunit`, or inside a v1 `.mpr`). This +needs no mxcli. When the value is `MxTest.RegisterEndpoint` or `MxTest.TestRunner`, both scripts +refuse and print the repair. `app-restart.sh` lets a live `--test-endpoint` app through, because it +holds the injection legitimately, and checks again after the stop. + +**Repair** (any one of): +- `git checkout -- mprcontents/4e/70/`, then `git status`: the run may also have left an `MxTest` module. +- Through `./bin/exec.sh` (an `ALTER SETTINGS`, so say so in chat; it is the statement mxcli's own cleanup runs, and it was field-proven on v0.25.0): + `mdl 1;` `alter settings runtime (AfterStartupMicroflow: 'MyModule.ASU_Startup');` (`''` for none). +- Studio Pro: App Settings → Runtime → After startup. + +**Avoid it:** stop `run --test-endpoint` with Ctrl+C in its own terminal. Never kill it or stop it +as a background task. Afterwards `git status` on the model must show the Settings unit unchanged. + +## BUG-158: `mxcli lint` on a copy of the model outside the repository silently drops the project's Starlark rules (8 of 29 rules ran, no notice, exit 0) + +**Severity:** Medium. The result is wrong, not refused: a before/after diff over it is off by hundreds. +**mxcli version:** v0.25.0 (field). Discovery code unchanged in v0.26.0 source (`mdl/linter/lintrules_discovery.go`, `12ce281`) +**Discovered:** 2026-10, attributing a lint ratchet rise to one script by linting the pre-exec snapshot copied to `/tmp` (#240) +**Reproducible:** yes. Copy the `.mpr` + `mprcontents/` to a folder outside the repository and lint it. +**Not filed upstream** (draft prepared, issue #240) + +`mxcli lint -p /tmp/pre/App.mpr --format json` gave **51 findings across 8 rules**. The same model +in the project folder gave **721 across 29**. The 21 missing rules are the Starlark rules in +`.claude/lint-rules/`. Discovery walks up from the `.mpr`'s folder and stops at a `.git`, so a +copy outside the repository finds none. Only the built-in rules ran (MPR\*, CONV013, SEC003, +MDL-FLOW01), with no notice and exit 0. The "no `.claude/lint-rules/` directory was found" line +exists in the source, but it prints only when `--rules` names an unknown id. + +Two related findings from reading the v0.26.0 source (not field-run): +- `lint-config.yaml` is read only from beside the `.mpr` (`FindConfigFile`), never from above it. + A snapshot inside the repository finds the rules by walking up but loses the config's module + excludes and rule overrides. +- With no `.git` above it, the walk continues to `/`. A stray `~/.claude/lint-rules` would then + supply rules to the copy. + +**Workaround (worked in the field):** `cp -R .claude/lint-rules /.claude/` (plus +`.claude/lint-config.yaml`) before linting. The copy then gave 721 findings across 29 rules, +matching the project. Recipe and the rule-set comparison to run before any diff: +`skills/lint-that-actually-runs.md` → "Linting a copy of the model". `lint-gate.sh`'s collapse +guard catches this inside the gate. Nothing catches it when lint is run directly. diff --git a/project-bin/app-restart.sh b/project-bin/app-restart.sh index b18ffe19..c9529a51 100755 --- a/project-bin/app-restart.sh +++ b/project-bin/app-restart.sh @@ -7,9 +7,12 @@ # ./bin/app-restart.sh --timeout readiness bound (default 240, MXTK_RESTART_TIMEOUT) # # Exit: 0 up and serving · 1 restarted but not proven up (timeout, no bundle) · 2 usage · -# 3 refused — nothing was stopped (no record, record belongs elsewhere, start command unknown) · +# 3 refused — nothing was stopped (no record, record belongs elsewhere, start command unknown, +# the model's after-startup left on an mxcli test injection — #244) · # 4 instrument blocked — the app or admin port is held by a process this script did not record # (named, with its pid; never stopped). A run summary records it as blocked, not as FAIL. +# 5 stopped, not started — stopping a `run --test-endpoint` app left its after-startup on +# MxTest.RegisterEndpoint, and a start would recurse (#244; the repair is printed). # # WHY (#289 item 1, field run 2026-10-12). A 20-task UX-fix run over three apps, one lane per app: # implementers were told never to start or stop apps, because a broad kill in one lane had once @@ -52,6 +55,12 @@ # the admin port (record's adminPort, else app port + 10) must be free or held by the recorded # process tree; otherwise it exits 4 naming the holder (lsof / ss / netstat) and kills nothing. # +# LEFTOVER TEST INJECTION (#244, BUG-157): `mxcli test` / `run --test-endpoint` stopped by a signal +# leave the after-startup on MxTest.RegisterEndpoint, and the next start recurses (StackOverflowError) +# or fails at CE1613. It is read from the model bytes (no mxcli) before anything is stopped — refused, +# exit 3, unless the recorded app is a live --test-endpoint run, which holds it legitimately — and +# again after the stop, before the start (exit 5). The repair is printed; this script never writes it. +# # Never touches Studio Pro, the model, Docker, or another port. Docker stack: bin/test-stack-up.sh # --restart (scoped to this project's container). Bash 3.2 / Git Bash / macOS / Linux. set -uo pipefail @@ -179,6 +188,65 @@ blocked() { exit 4 } # in_our_tree — the recorded pid itself, or a descendant of it (the runtime mxcli started). +# ── After-startup left on an mxcli test injection (#244, BUG-157) ─────────── +# `mxcli test` and `mxcli run --test-endpoint` point the model's after-startup microflow at +# MxTest.RegisterEndpoint (MxTest.TestRunner for a plain test run) and chain the original; only a +# clean exit puts it back. A run stopped by a signal leaves it, and the next injection then chains +# RegisterEndpoint to itself: StackOverflowError in the after-startup, the runtime never starts. +# Read straight from the model bytes — no mxcli, no running app: the setting is the BSON string +# element "AfterStartupMicroflow" of the project-settings unit (mprcontents/**/*.mxunit; inside the +# .mpr itself on a v1 model). Layout checked against real settings units (mxcli's own test models). +mxtest_after_startup() { # $1 = file, or - for stdin. Prints the value ('' = none); 1 = no such element + { if [ "$1" = - ]; then od -An -v -tx1; else od -An -v -tx1 "$1"; fi; } 2>/dev/null | awk ' + BEGIN { for (i = 0; i < 256; i++) hv[sprintf("%02x", i)] = i + for (i = 32; i < 127; i++) { c = sprintf("%c", i); ch[sprintf("%02x", i)] = c; hx[c] = sprintf("%02x", i) } + s = "AfterStartupMicroflow"; T = length(s) + 2; t[1] = "02"; t[T] = "00" + for (i = 1; i <= length(s); i++) t[i + 1] = hx[substr(s, i, 1)] + k = 0; st = 0; found = 0 } + { for (f = 1; f <= NF; f++) { b = $f + if (st == 0) { if (b == t[k + 1]) k++; else k = (b == t[1]) ? 1 : 0 + if (k == T) { st = 1; L = 0; m = 1; n = 0; v = "" } } + else if (st == 1) { L += hv[b] * m; m *= 256 + if (++n == 4) { if (L < 1 || L > 1024) { st = 0; k = 0 } else if (L == 1) { found = 1; exit } else { st = 2; n = 0 } } } + else { if (++n < L) v = v ((b in ch) ? ch[b] : "?"); else { found = 1; exit } } } } + END { if (!found) exit 1; print v }' +} +MXTEST_LEFT=""; MXTEST_UNIT=""; MXTEST_HEAD="" +mxtest_left() { # $1 = model dir, $2 = .mpr. 0 = after-startup is an mxcli test injection (sets MXTEST_*) + local unit val + MXTEST_LEFT=""; MXTEST_UNIT=""; MXTEST_HEAD="" + if [ -d "$1/mprcontents" ]; then + unit="$(LC_ALL=C grep -rla --include='*.mxunit' 'AfterStartupMicroflow' "$1/mprcontents" 2>/dev/null | head -1)" + else unit="$2"; fi + [ -n "$unit" ] && [ -f "$unit" ] || return 1 + val="$(mxtest_after_startup "$unit")" || return 1 + case "$val" in MxTest.RegisterEndpoint|MxTest.TestRunner) ;; *) return 1 ;; esac + MXTEST_LEFT="$val"; MXTEST_UNIT="$unit" + case "$unit" in *.mxunit) # what HEAD has, so the repair can name it; "?" = not tracked / no git + MXTEST_HEAD="$(git -C "$(dirname "$unit")" show "HEAD:./$(basename "$unit")" 2>/dev/null | mxtest_after_startup -)" || MXTEST_HEAD="?" ;; + *) MXTEST_HEAD="?" ;; esac + return 0 +} +mxtest_refusal() { # $1 = tool name. Prints the refusal and the repair to stderr. + local was="$MXTEST_HEAD" rel="${MXTEST_UNIT#"$PROJECT_ROOT"/}" + echo "✗ $1: this model's after-startup microflow is $MXTEST_LEFT — left behind by an mxcli test or" >&2 + echo " run --test-endpoint that did not exit cleanly (stopped by a signal or killed; BUG-157)." >&2 + echo " Starting on it recurses (the next injection chains $MXTEST_LEFT to itself: StackOverflowError)," >&2 + echo " or fails the build with CE1613 if the MxTest module is already gone. Either way the app never starts." >&2 + echo " set in: $rel" >&2 + case "$was" in '?') was="" + echo " HEAD: not readable (no git, or the unit is untracked)" >&2 ;; + '') echo " HEAD: no after-startup microflow" >&2 ;; + *) echo " HEAD: $was" >&2 ;; esac + echo " → If this project's app is running with --test-endpoint right now, this is its live injection:" >&2 + echo " stop it with Ctrl+C in its own terminal (a clean stop restores the setting), then retry." >&2 + echo " → Otherwise restore it, then retry:" >&2 + echo " git checkout -- '$rel' (then git status: the run may have left an MxTest module too)" >&2 + echo " or through ./bin/exec.sh (an ALTER SETTINGS — say so in chat; it is what mxcli's own cleanup runs):" >&2 + echo " mdl 1;" >&2 + echo " alter settings runtime (AfterStartupMicroflow: '$was');" >&2 + echo " or Studio Pro: App Settings → Runtime → After startup." >&2 +} in_our_tree() { local p="$1" i=0 [ "$ALIVE" = 1 ] || return 1 @@ -207,7 +275,31 @@ if [ "$HS_REAL" != "$MPR_REAL" ] && [ "$HS_REAL" != "$MPR_TARGET" ] \ "record: $HS_PROJECT" "this: $MPR_TARGET" \ "A copied project carries the original's record; stopping its pid would stop someone else's app." fi -ALIVE=0; pid_alive "$PID" && ALIVE=1 +# The recorded pid must still BE an mxcli run (#304 review). A SIGKILLed mxcli leaves its record +# behind, and the OS hands its pid to an unrelated process; `kill -0` cannot tell the difference, +# and the start-command check below cannot either when --cmd / MXTK_APP_START_CMD supplies the +# command — so without this a stale record plus a reused pid sent TERM/KILL to that process and +# its children. Checked on the pid itself, whichever way the start command is learned: its +# command line (/proc//cmdline, else `ps -o args=`) names mxcli and `run`; on Windows the +# image name is mxcli.exe. Anything else is treated as not running: never stopped, and if it +# holds one of the ports the port check below names it and stops. +pid_is_mxcli_run() { + local a="" + if [ "$(mxtk_platform)" = windows ]; then + a="$(tasklist //FI "PID eq $1" //FO CSV //NH 2>/dev/null | tr -d '\r' | head -1 | cut -d'"' -f2)" + case "$(printf '%s' "$a" | tr '[:upper:]' '[:lower:]')" in mxcli.exe|mxcli) return 0 ;; esac + return 1 + fi + if [ -r "/proc/$1/cmdline" ]; then a="$(tr '\0' ' ' < "/proc/$1/cmdline" 2>/dev/null)" + else a="$(ps -o args= -p "$1" 2>/dev/null)"; fi + case " $a " in *mxcli*" run "*) return 0 ;; esac + return 1 +} +ALIVE=0 +if pid_alive "$PID"; then + if pid_is_mxcli_run "$PID"; then ALIVE=1 + else echo " ! recorded pid $PID is alive but is not an mxcli run (the pid was reused after the app died) — treating the app as not running; that process is not touched"; fi +fi ADMIN_PORT="$(hs_num adminPort "$HS")"; [ -n "$ADMIN_PORT" ] || ADMIN_PORT=$((PORT + 10)) # ── 2. How it was started ─────────────────────────────────────────────────── @@ -241,6 +333,19 @@ case " ${ARGV[*]} " in "This script restarts only that; it will not start anything else." ;; esac + +# ── 2a. The model must not carry a leftover mxcli test injection (#244) ───── +# A live `run --test-endpoint` legitimately holds the injection while it runs; its stop is supposed +# to restore it, which 3b re-checks before anything starts. In every other case the start would +# recurse in the after-startup, so refuse before stopping anything. +LIVE_ENDPOINT=0 +if [ "$ALIVE" = 1 ]; then case " ${ARGV[*]} " in *" --test-endpoint "*|*" --test-endpoint="*) LIVE_ENDPOINT=1 ;; esac; fi +if mxtest_left "$MODEL_DIR" "$MPR_TARGET" && [ "$LIVE_ENDPOINT" = 0 ]; then + mxtest_refusal "app-restart" + echo " Nothing was stopped." >&2 + exit 3 +fi + echo "→ app-restart: $(basename "$MPR") on :$PORT" if [ "$ALIVE" = 1 ]; then echo " stop: pid $PID (from $HS) and its own child processes" else echo " stop: nothing — recorded pid $PID is not running"; fi @@ -292,6 +397,16 @@ elif port_answers "$PORT"; then blocked "$PORT" "?|an unidentified process" "The recorded app was stopped; nothing was started" fi + +# ── 3b. Did the stop leave the test injection behind? (#244) ─────────────── +# A `run --test-endpoint` stopped by a signal can skip its restore; starting it again would chain +# MxTest.RegisterEndpoint to itself. The app is down now — say so plainly, and start nothing. +if mxtest_left "$MODEL_DIR" "$MPR_TARGET"; then + mxtest_refusal "app-restart" + echo " The recorded app was stopped; nothing was started (exit 5)." >&2 + exit 5 +fi + # ── 4. Start it the same way, detached, and remember how ──────────────────── LOG="${LOGDIR:-${TMPDIR:-/tmp}}/$(basename "$MPR" .mpr)-run-local.log" rm -f "$HS" 2>/dev/null || true # a SIGKILLed mxcli leaves its record behind; the new one must be new diff --git a/project-bin/test-scratch.sh b/project-bin/test-scratch.sh index b45d134a..dc3055a6 100755 --- a/project-bin/test-scratch.sh +++ b/project-bin/test-scratch.sh @@ -23,6 +23,9 @@ # holder has it and taking over a dead one; # 4. runs `mxcli test -p .mpr` in the scratch dir, then releases both and deletes # the copy (--keep leaves it, and says where). +# Refused (exit 3, nothing copied) when the live model's after-startup is still an mxcli test injection +# (MxTest.RegisterEndpoint / MxTest.TestRunner, left by a run stopped by a signal — #244, BUG-157): +# the copy would carry it and the test runtime would recurse in its after-startup. The repair is printed. # --attach is refused: it hot-applies into the live app and runs against its database, which is # the opposite of a scratch run. Never touches the live model, the live app, or Studio Pro. # Bash 3.2 / Git Bash / macOS / Linux; no flock (absent on macOS and Git Bash). @@ -57,6 +60,74 @@ elif [ -x "$MODEL_DIR/mxcli" ]; then MXCLI="$MODEL_DIR/mxcli" elif command -v mxcli >/dev/null 2>&1; then MXCLI="$(command -v mxcli)" else echo "✗ test-scratch: no mxcli (project root or PATH)." >&2; exit 3; fi +# ── After-startup left on an mxcli test injection (#244, BUG-157) ─────────── +# `mxcli test` and `mxcli run --test-endpoint` point the model's after-startup microflow at +# MxTest.RegisterEndpoint (MxTest.TestRunner for a plain test run) and chain the original; only a +# clean exit puts it back. A run stopped by a signal leaves it, and the next injection then chains +# RegisterEndpoint to itself: StackOverflowError in the after-startup, the runtime never starts. +# Read straight from the model bytes — no mxcli, no running app: the setting is the BSON string +# element "AfterStartupMicroflow" of the project-settings unit (mprcontents/**/*.mxunit; inside the +# .mpr itself on a v1 model). Layout checked against real settings units (mxcli's own test models). +mxtest_after_startup() { # $1 = file, or - for stdin. Prints the value ('' = none); 1 = no such element + { if [ "$1" = - ]; then od -An -v -tx1; else od -An -v -tx1 "$1"; fi; } 2>/dev/null | awk ' + BEGIN { for (i = 0; i < 256; i++) hv[sprintf("%02x", i)] = i + for (i = 32; i < 127; i++) { c = sprintf("%c", i); ch[sprintf("%02x", i)] = c; hx[c] = sprintf("%02x", i) } + s = "AfterStartupMicroflow"; T = length(s) + 2; t[1] = "02"; t[T] = "00" + for (i = 1; i <= length(s); i++) t[i + 1] = hx[substr(s, i, 1)] + k = 0; st = 0; found = 0 } + { for (f = 1; f <= NF; f++) { b = $f + if (st == 0) { if (b == t[k + 1]) k++; else k = (b == t[1]) ? 1 : 0 + if (k == T) { st = 1; L = 0; m = 1; n = 0; v = "" } } + else if (st == 1) { L += hv[b] * m; m *= 256 + if (++n == 4) { if (L < 1 || L > 1024) { st = 0; k = 0 } else if (L == 1) { found = 1; exit } else { st = 2; n = 0 } } } + else { if (++n < L) v = v ((b in ch) ? ch[b] : "?"); else { found = 1; exit } } } } + END { if (!found) exit 1; print v }' +} +MXTEST_LEFT=""; MXTEST_UNIT=""; MXTEST_HEAD="" +mxtest_left() { # $1 = model dir, $2 = .mpr. 0 = after-startup is an mxcli test injection (sets MXTEST_*) + local unit val + MXTEST_LEFT=""; MXTEST_UNIT=""; MXTEST_HEAD="" + if [ -d "$1/mprcontents" ]; then + unit="$(LC_ALL=C grep -rla --include='*.mxunit' 'AfterStartupMicroflow' "$1/mprcontents" 2>/dev/null | head -1)" + else unit="$2"; fi + [ -n "$unit" ] && [ -f "$unit" ] || return 1 + val="$(mxtest_after_startup "$unit")" || return 1 + case "$val" in MxTest.RegisterEndpoint|MxTest.TestRunner) ;; *) return 1 ;; esac + MXTEST_LEFT="$val"; MXTEST_UNIT="$unit" + case "$unit" in *.mxunit) # what HEAD has, so the repair can name it; "?" = not tracked / no git + MXTEST_HEAD="$(git -C "$(dirname "$unit")" show "HEAD:./$(basename "$unit")" 2>/dev/null | mxtest_after_startup -)" || MXTEST_HEAD="?" ;; + *) MXTEST_HEAD="?" ;; esac + return 0 +} +mxtest_refusal() { # $1 = tool name. Prints the refusal and the repair to stderr. + local was="$MXTEST_HEAD" rel="${MXTEST_UNIT#"$PROJECT_ROOT"/}" + echo "✗ $1: this model's after-startup microflow is $MXTEST_LEFT — left behind by an mxcli test or" >&2 + echo " run --test-endpoint that did not exit cleanly (stopped by a signal or killed; BUG-157)." >&2 + echo " Starting on it recurses (the next injection chains $MXTEST_LEFT to itself: StackOverflowError)," >&2 + echo " or fails the build with CE1613 if the MxTest module is already gone. Either way the app never starts." >&2 + echo " set in: $rel" >&2 + case "$was" in '?') was="" + echo " HEAD: not readable (no git, or the unit is untracked)" >&2 ;; + '') echo " HEAD: no after-startup microflow" >&2 ;; + *) echo " HEAD: $was" >&2 ;; esac + echo " → If this project's app is running with --test-endpoint right now, this is its live injection:" >&2 + echo " stop it with Ctrl+C in its own terminal (a clean stop restores the setting), then retry." >&2 + echo " → Otherwise restore it, then retry:" >&2 + echo " git checkout -- '$rel' (then git status: the run may have left an MxTest module too)" >&2 + echo " or through ./bin/exec.sh (an ALTER SETTINGS — say so in chat; it is what mxcli's own cleanup runs):" >&2 + echo " mdl 1;" >&2 + echo " alter settings runtime (AfterStartupMicroflow: '$was');" >&2 + echo " or Studio Pro: App Settings → Runtime → After startup." >&2 +} + +# A model whose after-startup is an mxcli test injection must not be copied and tested: the copy +# carries it, and mxcli test then chains the injection to itself (#244 observation 1). +if mxtest_left "$MODEL_DIR" "$MODEL_DIR/$MPR_NAME"; then + mxtest_refusal "test-scratch" + echo " Nothing was copied or run." >&2 + exit 3 +fi + # Same liveness rule as exec.sh's lock: alive, not a zombie, and (where ps can say) one of ours. holder_live() { [ -n "${1:-}" ] || return 1 @@ -67,9 +138,32 @@ holder_live() { case "$_args" in ""|*exec.sh*|*test-scratch.sh*) return 0 ;; esac return 1 } +# Taking over a stale lock is itself serialised (#304 review): two runs that both read the same +# dead holder must not both remove "the stale lock" — the second would remove the lock the first +# had just taken, and both would run. So a takeover first wins "$f.takeover" (noclobber: O_EXCL; +# one older than 30s is a takeover that died midway and is cleared), re-reads the lock, and +# removes it only if it still holds the stale line it judged. It moves the lock aside before +# checking; a lock that changed in between is put back with `ln`, which refuses to overwrite, +# never `mv -f`, which would clobber a lock taken meanwhile. +take_over() { # $1 = lock file, $2 = the stale line. 0 = handled (retry the take), 1 = another takeover runs + local f="$1" held="$2" t="$1.takeover" ts + if ! ( set -o noclobber; printf '%s %s\n' "$$" "$(date +%s)" > "$t" ) 2>/dev/null; then + ts="$(cut -d' ' -f2 "$t" 2>/dev/null | head -1)" + case "$ts" in ''|*[!0-9]*) ;; *) [ $(( $(date +%s) - ts )) -gt 30 ] && rm -f "$t" ;; esac + return 1 + fi + if [ "$(head -1 "$f" 2>/dev/null || true)" = "$held" ] && mv "$f" "$f.stale.$$" 2>/dev/null; then + if [ "$(head -1 "$f.stale.$$" 2>/dev/null || true)" != "$held" ]; then + ln "$f.stale.$$" "$f" 2>/dev/null || true # not the line we judged: put it back, never over a newer one + fi + rm -f "$f.stale.$$" + fi + rm -f "$t" + return 0 +} take_lock() { # $1 = lock file, $2 = what it guards (for messages). 0 = taken, 1 = live holder local f="$1" held hpid i - for i in 1 2 3 4 5; do + for i in 1 2 3 4 5 6 7 8; do if ( set -o noclobber; printf '%s %s %s\n' "$$" "$(date +%s)" "test-scratch.sh" > "$f" ) 2>/dev/null; then return 0; fi held="$(head -1 "$f" 2>/dev/null || true)" if [ -z "$held" ] && [ "$i" -lt 3 ]; then sleep 1; continue; fi @@ -78,10 +172,13 @@ take_lock() { # $1 = lock file, $2 = what it guards (for messages). 0 = taken, echo "✗ test-scratch: $2 is held by PID $hpid ($(printf '%s' "$held" | cut -d' ' -f3-)) — wait for it to finish." >&2 return 1 fi - echo " (stale lock on $2 from PID ${hpid:-?}, not running — taking it over)" - mv -f "$f" "$f.stale.$$" 2>/dev/null && rm -f "$f.stale.$$" + if take_over "$f" "$held"; then + echo " (stale lock on $2 from PID ${hpid:-?}, not running — taking it over)" + else + sleep 1 # another run is taking the same stale lock over; see who won + fi done - echo "✗ test-scratch: could not take the lock on $2 after 5 attempts." >&2; return 1 + echo "✗ test-scratch: could not take the lock on $2 after 8 attempts." >&2; return 1 } release() { local f="$1" own; own="$(head -1 "$f" 2>/dev/null || true)"; [ "${own%% *}" = "$$" ] && rm -f "$f"; return 0; } diff --git a/skills/lint-that-actually-runs.md b/skills/lint-that-actually-runs.md index 64b1b179..ba4d5b6a 100644 --- a/skills/lint-that-actually-runs.md +++ b/skills/lint-that-actually-runs.md @@ -153,6 +153,52 @@ Both were found by testing the gate, not by reading it: rules improved."* If the catalog is empty or the rules fail to load, every count drops to zero and the gate celebrates. Guard: a broad drop is a malfunction until proven otherwise. +## Linting a copy of the model: copy the rules in first + +`mxcli lint` looks for the Starlark rules in `.claude/lint-rules/` from the `.mpr`'s folder +upward, and stops at the repository root. A copy of the model **outside the repository** (a +pre-exec snapshot copied to `/tmp` to attribute a ratchet rise, a scratch copy for a +before/after diff) therefore lints with the built-in rules only. Nothing says so and the exit +code is 0. + +Field run, v0.25.0, 2026-10 (BUG-158, #240). The same model gave: + +| Where the `.mpr` sat | Findings | Rules that ran | +|---|---|---| +| project folder | 721 | 29 | +| copy in `/tmp`, no rules beside it | 51 | 8 (MPR\*, CONV013, SEC003, MDL-FLOW01) | + +A before/after diff over those two outputs reports 670 "new" findings that are not new. + +**Recipe.** Copy the rules and the lint config beside the copy, then lint it: + +```sh +COPY=/tmp/pre # the folder that holds the copied App.mpr +mkdir -p "$COPY/.claude" && cp -R .claude/lint-rules "$COPY/.claude/" +[ -f .claude/lint-config.yaml ] && cp .claude/lint-config.yaml "$COPY/.claude/" +./mxcli lint -p "$COPY/App.mpr" --format json > pre.json +``` + +**Before you diff two lint outputs, compare their rule sets.** Both runs must report the same +number of distinct `ruleId`s (or the copy's set must be a subset you can explain), counted over +the two files. If the copy shows only the built-in rules above, it did not load the project rules +and the diff is meaningless. "51 findings" from a copy is not a result until its rule count +matches. + +- A copy **inside** the repository (`.mpr-snapshots//`, left where `exec.sh` puts it) + finds the rules by walking up. It does **not** find `lint-config.yaml`: in mxcli's source + (v0.26.0) that file is read only from beside the `.mpr`, never above it. Its module excludes + and rule overrides then do not apply, so copy both files every time. +- A copy outside any repository is searched all the way up to `/`. In the v0.26.0 source the + walk stops only at a `.git`, so a stray `~/.claude/lint-rules` or `/tmp/.claude/lint-rules` + would supply its rules to the copy. Copying the project's rules in puts the nearest match + beside the `.mpr`, which wins. +- There is no `--rules-dir` option on v0.25.0. Check `mxcli lint --help` on your binary before + you rely on one. +- `project-bin/lint-gate.sh` guards against the collapse inside the gate (see "A collapse looks + like a triumph" above). Running lint directly has no such guard, and this recipe stands in for + it. + ## What does NOT generalise — do not build this **A vendor-module exclusion list.** `mxcli lint` **already skips Marketplace-sourced modules and diff --git a/tests/wave2/fixtures/mxtest-guard/set-after-startup.js b/tests/wave2/fixtures/mxtest-guard/set-after-startup.js new file mode 100644 index 00000000..83182623 --- /dev/null +++ b/tests/wave2/fixtures/mxtest-guard/set-after-startup.js @@ -0,0 +1,34 @@ +#!/usr/bin/env node +// set-after-startup.js — fixture helper: re-encode a captured Mendix settings unit (BSON) with +// AfterStartupMicroflow = , every enclosing length rewritten. Re-encoding with the original +// value ('') reproduces the capture byte for byte (checked by test-mxtest-guard.sh). +const fs = require('fs'); +const [inp, out, value] = process.argv.slice(2); +const buf = fs.readFileSync(inp); +function cstr(b, o) { const e = b.indexOf(0, o); return [b.toString('utf8', o, e), e + 1]; } +function size(t, b, o) { + switch (t) { + case 0x01: case 0x09: case 0x11: case 0x12: return 8; + case 0x02: case 0x0d: case 0x0e: return 4 + b.readInt32LE(o); + case 0x03: case 0x04: return b.readInt32LE(o); + case 0x05: return 5 + b.readInt32LE(o); + case 0x07: return 12; case 0x08: return 1; case 0x0a: case 0x06: case 0x7f: case 0xff: return 0; + case 0x10: return 4; case 0x13: return 16; + default: throw new Error('unknown BSON type 0x' + t.toString(16) + ' at ' + o); + } +} +function enc(b, o) { // re-encode document at o, rewriting the target + const len = b.readInt32LE(o), end = o + len - 1; let p = o + 4; const parts = []; + while (p < end) { + const t = b[p]; const [k, vo] = cstr(b, p + 1); const n = size(t, b, vo); + let val = b.subarray(vo, vo + n); + if (t === 0x03 || t === 0x04) val = enc(b, vo); + else if (t === 0x02 && k === 'AfterStartupMicroflow') { + const s = Buffer.from(value + '\0', 'utf8'); const l = Buffer.alloc(4); l.writeInt32LE(s.length); val = Buffer.concat([l, s]); + } + parts.push(b.subarray(p, vo), val); p = vo + n; + } + const body = Buffer.concat(parts); const h = Buffer.alloc(4); h.writeInt32LE(body.length + 5); + return Buffer.concat([h, body, Buffer.from([0])]); +} +fs.writeFileSync(out, enc(buf, 0)); diff --git a/tests/wave2/fixtures/mxtest-guard/settings-unit.mxunit b/tests/wave2/fixtures/mxtest-guard/settings-unit.mxunit new file mode 100644 index 0000000000000000000000000000000000000000..6984c6120b3fcb840f3f796292b852808f9429d3 GIT binary patch literal 2684 zcmZuzU2IfE6rSGQ(v=t_jR{~tO;}Jdv4x6Mg6RGfTIhDy-fb&jVy1V`?hc)M=W=Im zw_8CY;X#lnK}}2_P$LqfMllLcKB@7+V0hp~A51jS5Px1Yn)uK}&)lE=Y1oIocg~zS zzu)=h9$!sJZ+a|G))M?Zw#uFO$z0t3!TGuQp}{X+HPYp#3uFUsnV=MNYBF8I$R zye7%k+W^{HJ4g)FPHZQedO9tC{{8XyXm!`&w~NE;&b{{EACcvIPc*{QJ= zwoBAgBsJpDSq=qed7{ciV-l!Ur?ZSR)yz1QHzkv#C(@dLhN(P^GYfS(&xA+f^SCp^ zDm761!2ypdCY;u(N$FB+F6%oL4($`6dLTuEge1##u%RF-K8K=LLCxDSdQ3Qh4`NJt zyyQWZEd&K{Q6Qn{DrU1qsN^^UN0B5d9s-H=*+C}pyla1Q7Nu(ddyK!3q!wEp-5tg6tMVhqu(XJy*@DX$mH~u_zv9Z0Q=q;l;;Z#}8 z56#9>lI#*o@;5d!$`o7sOIK?jKfQc4^ZwP$yL|zyKE(<#i#|?%~1V!GRzosi>o-Dfh94oyAY)e5r)p3cqvD z$WujUse}n?14*HS3RJ1j6>061sFV%iRS4#Vos16z*zZHhs656iv+WHXX`q^lRk7D` zcc5Z|<$H+E4pX@A1^z=1BMZu-2j*=AI0mJ_Lj-Hr{iB$BNqB0)w`UPx=k$DNd4Vn* zp!0MTsB0pgLMi?mt)%i%QmLW~PNrfrN83S&>ve3Fkkxuw298KpUX%2Pjp=Z?6uCs> ziHFJ7EwW*3r619$2_U>v{|gq0L%sGi3@FvQd!UN zn!r}qZK83K&FP?tfo%IS|aXHkXA(hOFcxH<8r1Xy7~YM5|@f z)MNVU2~vE7Y{n67r}*Wk+upj-eCf!CW4m8DQ$JIRQoO4(#c}G?d_-0&!8oFLBY@*7 zJOa)$Qzl8qa!IbBuuZs^aMgBw>HN~ahWppSk8k$gHh$*na^(6BR4QYOWI8rOQUL~Y z!p7lJc<)I}#aQ9g2)PrH0s1DU$Qft|)dEV@VvV_5Y^@aIp~Nmi@zDq#fStXeGHH(eAdw(BC&CS#)BX!Cc1h3^w)E?~Nd~ z@ea^93n%CyEhYV}V@?gR*y&GCYVb=ux}uCHofFTV-Np~U*!#np6aVz=jZ)N9-AeHQ z_443y(X?@@Vkt?EELhwRSv8uVAVmB^8zd+2e7jdyE}i~u&rILUJIm{K-nVCTC~|Ul lr;|KGvS71**Vr0aoSI6}i773>PijoIl=ToHqPq6c>i<8_5c>cC literal 0 HcmV?d00001 diff --git a/tests/wave2/test-app-restart.sh b/tests/wave2/test-app-restart.sh index 75920d7c..c99ee3dd 100755 --- a/tests/wave2/test-app-restart.sh +++ b/tests/wave2/test-app-restart.sh @@ -22,6 +22,12 @@ # #262: a stranger on the admin port while our app runs → exit 4 before anything is stopped, the # stranger named by pid and alive; our app dead and a stranger on the app port → exit 4, nothing # started. (The stranger is a bare TCP listener, as a runtime's admin port is to curl.) +# #304 review: a stale record whose pid was reused by an unrelated process, restarted with --cmd → +# that process survives (the pid is checked to be an mxcli run itself). #244: the settings unit is +# the CAPTURED one (fixtures/mxtest-guard, from mxcli's own test model), re-encoded with the +# after-startup under test: a plain app + leftover injection → exit 3, app untouched; a live +# --test-endpoint app whose stop does not restore → exit 5, nothing started; one whose stop restores +# → restarted. # Needs node and curl; skips (exit 0, says so) without them. set -uo pipefail @@ -84,13 +90,15 @@ http.createServer((req, res) => { fs.mkdirSync(path.dirname(hs), { recursive: true }); setTimeout(() => fs.writeFileSync(hs, g), 300); // written when "ready", as mxcli does }); -const stop = () => { try { fs.unlinkSync(hs); } catch (e) {} try { child.kill('SIGTERM'); } catch (e) {} process.exit(0); }; +// .restore_on_stop (two lines: source, target) stands in for run --test-endpoint's clean-exit restore. +const stop = () => { try { const r = fs.readFileSync(path.join(dir, '.restore_on_stop'), 'utf8').trim().split('\n'); fs.copyFileSync(r[0], r[1]); } catch (e) {} + try { fs.unlinkSync(hs); } catch (e) {} try { child.kill('SIGTERM'); } catch (e) {} process.exit(0); }; process.on('SIGTERM', stop); process.on('SIGINT', stop); JS chmod +x "$P/mxcli" "$P/bin/app-restart.sh" } -start_app() { # $1 = project dir; waits for the record - ( cd "$1" && FAKE_GOLDEN="$GOLDEN" setsid nohup ./mxcli run --local --watch -p Fixture.mpr >"$1/first.log" 2>&1 "$1/first.log" 2>&1 /dev/null; } @@ -157,6 +165,56 @@ has "says there was nothing to stop" "$OUT" "nothing — recorded pid $D is not has "uses the remembered command" "$OUT" "remembered from the last restart" KILL_LIST="$KILL_LIST $(hs_pid "$P") $(cat "$P/.fakechild" 2>/dev/null)" +echo "-- #304 review: a stale record whose pid now belongs to another process → that process is never stopped" +# A SIGKILLed mxcli leaves its record; the OS reuses the pid. With --cmd the start command no longer +# comes from the pid, so only a check on the pid itself can tell. The old code sent it TERM. +R="$WORK/reused"; RPORT=$(( PORT + 200 )); mkapp "$R" "$RPORT" +sleep 300 & REUSED=$!; KILL_LIST="$KILL_LIST $REUSED" +mkdir -p "$R/.mxcli" +sed -e "s|^ \"project\": \"[^\"]*\"| \"project\": $(printf '%s' "$R/Fixture.mpr" | sed 's/[&|]/\\&/g; s/.*/"&"/')|" \ + -e "s/^ \"pid\": [0-9]*/ \"pid\": $REUSED/" -e "s/^ \"appPort\": [0-9]*/ \"appPort\": $RPORT/" \ + -e "s/^ \"adminPort\": [0-9]*/ \"adminPort\": $(( RPORT + 10 ))/" "$GOLDEN" > "$R/.mxcli/run-local.json" +OUT="$(restart "$R" --timeout 30 --cmd './mxcli run --local --watch -p Fixture.mpr')"; RC=$? +alive "$REUSED" && ok "the process holding the reused pid is untouched" || bad "the reused pid $REUSED was killed" +has "says the pid is not an mxcli run" "$OUT" "is not an mxcli run" +[ "$RC" -eq 0 ] && ok "the app is started (exit 0)" || bad "exit $RC: $(printf '%s' "$OUT" | tail -3)" +KILL_LIST="$KILL_LIST $(hs_pid "$R") $(cat "$R/.fakechild" 2>/dev/null)" + +echo "-- #244: an after-startup left on an mxcli test injection" +GU="$TOOLKIT/tests/wave2/fixtures/mxtest-guard/settings-unit.mxunit" +SETAS="$TOOLKIT/tests/wave2/fixtures/mxtest-guard/set-after-startup.js" +UNIT="$P/mprcontents/4e/70/4e70063d-53e1-477a-ab92-6b763330c293.mxunit"; mkdir -p "$(dirname "$UNIT")" +cp "$GU" "$UNIT"; cp "$GU" "$P/.clean.mxunit" +CUR="$(hs_pid "$P")" +node "$SETAS" "$GU" "$UNIT" "MxTest.RegisterEndpoint" +OUT="$(restart "$P" --timeout 10)"; RC=$? +[ "$RC" -eq 3 ] && ok "plain run --local app + leftover injection → refused (exit 3)" || bad "exit $RC: $(printf '%s' "$OUT" | tail -3)" +has "names the leftover" "$OUT" "after-startup microflow is MxTest.RegisterEndpoint" +has "prints the MDL repair" "$OUT" "alter settings runtime (AfterStartupMicroflow:" +has "says nothing was stopped" "$OUT" "Nothing was stopped" +alive "$CUR" && ok "the app was left running" || bad "the app was stopped" +cp "$GU" "$UNIT" + +echo "-- #244: a live --test-endpoint app holds the injection legitimately; a stop that does not restore it → exit 5" +OUT="$(restart "$P" --timeout 30 --cmd './mxcli run --local --watch --test-endpoint -p Fixture.mpr')"; RC=$? +[ "$RC" -eq 0 ] && ok "[setup] app restarted with --test-endpoint" || bad "[setup] exit $RC: $(printf '%s' "$OUT" | tail -3)" +CUR="$(hs_pid "$P")"; KILL_LIST="$KILL_LIST $CUR $(cat "$P/.fakechild" 2>/dev/null)" +node "$SETAS" "$GU" "$UNIT" "MxTest.RegisterEndpoint" # what run --test-endpoint writes while it runs +OUT="$(restart "$P" --timeout 10)"; RC=$? +[ "$RC" -eq 5 ] && ok "exit 5" || bad "exit $RC: $(printf '%s' "$OUT" | tail -3)" +has "says it stopped and started nothing" "$OUT" "The recorded app was stopped; nothing was started" +hasnt "starts nothing" "$OUT" "started (log:" +alive "$CUR" && bad "the app was not stopped" || ok "the --test-endpoint app was stopped" +echo "-- #244: repaired and started by hand (the stop removed the record); a stop that restores restarts normally" +cp "$GU" "$UNIT" +start_app "$P" --test-endpoint && ok "[setup] --test-endpoint app started again" || bad "[setup] app never became ready" +KILL_LIST="$KILL_LIST $(hs_pid "$P") $(cat "$P/.fakechild" 2>/dev/null)" +node "$SETAS" "$GU" "$UNIT" "MxTest.RegisterEndpoint"; printf '%s\n%s\n' "$P/.clean.mxunit" "$UNIT" > "$P/.restore_on_stop" +OUT="$(restart "$P" --timeout 30)"; RC=$? +[ "$RC" -eq 0 ] && ok "a stop that restores → restarted (exit 0)" || bad "exit $RC: $(printf '%s' "$OUT" | tail -3)" +rm -f "$P/.restore_on_stop"; cp "$GU" "$UNIT" +KILL_LIST="$KILL_LIST $(hs_pid "$P") $(cat "$P/.fakechild" 2>/dev/null)" + # A process this script did not start, listening on $1 — the Studio Pro runtime of #262. stranger() { node -e "require('net').createServer(()=>{}).listen($1,'127.0.0.1')" & STR=$!; KILL_LIST="$KILL_LIST $STR" diff --git a/tests/wave2/test-test-scratch.sh b/tests/wave2/test-test-scratch.sh index 9fcfac76..ecb26de0 100755 --- a/tests/wave2/test-test-scratch.sh +++ b/tests/wave2/test-test-scratch.sh @@ -91,6 +91,74 @@ OUT="$(run g -- tests/a.test.mdl --attach)"; RC=$? OUT="$(run h -- tests/a.test.mdl -p Other.mpr)"; RC=$? [ "$RC" -eq 2 ] && ok "-p refused" || bad "-p exit $RC" + +echo "-- #304 review: a stale-lock takeover never removes a lock another run took meanwhile" +# Deterministic replay of the race: the functions are lifted from the subject, and holder_live +# is wrapped so that, while this run judges the dead holder, a second run completes its own +# takeover and writes a live lock. The old code then `mv -f`'d that live lock away and took +# the lock too — two holders. Taking over must re-read and leave a changed lock alone. +H="$WORK/harness.sh"; L="$WORK/race.lock" +awk '/^holder_live\(\) \{/ {p=1} /^release\(\) \{/ {p=0} p' "$SUBJECT" > "$H" +grep -q '^take_lock() {' "$H" && ok "[setup] lifted holder_live..take_lock from the subject" || bad "[setup] could not lift take_lock" +sleep 300 & LIVE=$!; BG="$BG $LIVE" +OUT="$( L="$L" LIVE="$LIVE" bash -c ' + . "$1" + eval "orig_$(declare -f holder_live)" + holder_live() { # the dead holder is being judged: another run finishes ITS takeover now + [ "$1" = 999999 ] && printf "%s %s %s\n" "$LIVE" "$(date +%s)" "test-scratch.sh" > "$L" + [ "$1" = "$LIVE" ] + } + printf "999999 1 test-scratch.sh\n" > "$L" + take_lock "$L" "race lock"; echo "rc=$?" +' _ "$H" 2>&1 )" +has "the second taker is refused (the live lock survives)" "$OUT" "rc=1" +[ "$(cut -d' ' -f1 "$L" 2>/dev/null)" = "$LIVE" ] && ok "the lock still names the live holder" || bad "the live holder's lock was replaced: $(cat "$L" 2>/dev/null)" +[ -e "$L.takeover" ] && bad "takeover marker left behind" || ok "takeover marker cleared" +echo "-- a takeover marker older than 30s (a takeover that died midway) is cleared" +printf '999999 1 test-scratch.sh\n' > "$L"; printf '999998 1\n' > "$L.takeover" +OUT="$( L="$L" bash -c '. "$1"; holder_live() { return 1; }; take_lock "$L" "race lock"; echo "rc=$?"' _ "$H" 2>&1 )" +has "taken after clearing the dead marker" "$OUT" "rc=0" +[ "$(cut -d' ' -f1 "$L" 2>/dev/null)" != 999999 ] && ok "the stale line is gone" || bad "stale line still there" +kill -KILL "$LIVE" 2>/dev/null; wait "$LIVE" 2>/dev/null + +echo "-- #244: an after-startup left on an mxcli test injection is refused before anything is copied" +if command -v node >/dev/null 2>&1; then + GU="$TOOLKIT/tests/wave2/fixtures/mxtest-guard/settings-unit.mxunit" + SETAS="$TOOLKIT/tests/wave2/fixtures/mxtest-guard/set-after-startup.js" + UNIT="$P/mprcontents/4e/70/4e70063d-53e1-477a-ab92-6b763330c293.mxunit"; mkdir -p "$(dirname "$UNIT")" + node "$SETAS" "$GU" "$WORK/rt.mxunit" "" && cmp -s "$GU" "$WORK/rt.mxunit" \ + && ok "[setup] re-encoding the captured unit reproduces it byte for byte" || bad "[setup] the BSON re-encoder is not faithful" + cp "$GU" "$UNIT" + OUT="$(run i -- tests/a.test.mdl)"; RC=$? + [ "$RC" -eq 0 ] && ok "captured settings unit (no after-startup) → runs" || bad "clean captured unit refused (exit $RC): $(printf '%s' "$OUT" | tail -3)" + ( cd "$P" && git init -q && git add mprcontents && git -c user.email=t@t -c user.name=t commit -qm base ) >/dev/null 2>&1 + node "$SETAS" "$GU" "$UNIT" "Orders.ASU_Startup" + ( cd "$P" && git -c user.email=t@t -c user.name=t commit -qam own ) >/dev/null 2>&1 + OUT="$(run j -- tests/a.test.mdl)"; RC=$? + [ "$RC" -eq 0 ] && ok "a project's own after-startup → runs" || bad "own after-startup refused (exit $RC)" + node "$SETAS" "$GU" "$UNIT" "MxTest.RegisterEndpoint" + rm -rf "$WORK/tmp"/Fixture-test.* + OUT="$(run k -- tests/a.test.mdl)"; RC=$? + [ "$RC" -eq 3 ] && ok "MxTest.RegisterEndpoint → refused (exit 3)" || bad "exit $RC: $(printf '%s' "$OUT" | tail -3)" + has "names the leftover" "$OUT" "after-startup microflow is MxTest.RegisterEndpoint" + has "names what HEAD has" "$OUT" "HEAD: Orders.ASU_Startup" + has "prints the git repair" "$OUT" "git checkout -- 'mprcontents/4e/70/4e70063d-53e1-477a-ab92-6b763330c293.mxunit'" + has "prints the MDL repair with HEAD's value" "$OUT" "alter settings runtime (AfterStartupMicroflow: 'Orders.ASU_Startup');" + [ -f "$WORK/rec.k" ] && bad "mxcli ran on the injected model" || ok "mxcli not run" + ls "$WORK/tmp" | grep -q 'Fixture-test' && bad "a copy was made" || ok "nothing copied" + node "$SETAS" "$GU" "$UNIT" "MxTest.TestRunner" + OUT="$(run l -- tests/a.test.mdl)"; RC=$? + [ "$RC" -eq 3 ] && ok "MxTest.TestRunner → refused (exit 3)" || bad "TestRunner exit $RC" + cp "$GU" "$UNIT" + echo "-- #244 on a v1 model (settings inside the .mpr itself)" + V="$WORK/v1"; mkdir -p "$V/bin"; cp "$P/bin/"*.sh "$V/bin/"; cp "$P/mxcli" "$V/" + { printf 'SQLite format 3\0'; node "$SETAS" "$GU" /dev/stdout "MxTest.RegisterEndpoint"; printf '\0\0trailing'; } > "$V/Fixture.mpr" + OUT="$( cd "$V" && REC="$WORK/rec.m" ./bin/test-scratch.sh -- x.test.mdl 2>&1 )"; RC=$? + [ "$RC" -eq 3 ] && has "v1 .mpr → refused, git repair degrades to a placeholder" "$OUT" "HEAD: not readable" || bad "v1 exit $RC: $(printf '%s' "$OUT" | tail -3)" +else + echo " [skip] node missing — the injected settings units are built from the capture with node" +fi + echo "" echo "PASS=$PASS FAIL=$FAIL" [ "$FAIL" -eq 0 ] From dac33df6d1845157447c8a8cd4a1d5e4c110b799 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 13 Oct 2026 15:15:20 +0000 Subject: [PATCH 2/5] test-scratch: take over the shared exec lock with exec.sh's exact protocol (#324) test-scratch.sh shares .mpr-snapshots/.exec.lock with exec.sh, so its stale-lock takeover now mirrors exec_lock_takeover from PR #324 (fix/exec-followups @ ed4efbb): the same O_EXCL .takeover file, stale when its pid is dead or after 10 s (empty: after a minute), re-read and remove only the exact stale line, put a wrongly moved takeover file back with ln. The port lock uses the same protocol. New cross-script fixture tests/wave2/test-shared-lock-takeover.sh: 4 rounds of 5 exec.sh + 5 test-scratch contenders on one dead-holder lock never produce two holders (master's test-scratch: overlaps in 3 of 3 runs). exec.sh's block comes from the real exec.sh once it carries exec_lock_takeover, else from a verbatim copy of #324's block. Refs #244 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01VJgWP5vEoAsNsJYqCDMGNw --- CHANGELOG.md | 2 +- project-bin/test-scratch.sh | 49 ++++++----- .../fixtures/mxtest-guard/exec-lock-block.sh | 81 +++++++++++++++++++ tests/wave2/test-shared-lock-takeover.sh | 75 +++++++++++++++++ tests/wave2/test-test-scratch.sh | 2 +- 5 files changed, 188 insertions(+), 21 deletions(-) create mode 100644 tests/wave2/fixtures/mxtest-guard/exec-lock-block.sh create mode 100644 tests/wave2/test-shared-lock-takeover.sh diff --git a/CHANGELOG.md b/CHANGELOG.md index f514aba5..398efbf3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,7 +19,7 @@ Sections dated before 2026-09-19 predate the cycle and stay as they are. - fix(test-scratch, app-restart): **both refuse to start on a model whose after-startup is still an mxcli test injection (`MxTest.RegisterEndpoint` / `MxTest.TestRunner`), and print the repair** ([#244](https://github.com/MendixMau/mxcli-project-toolkit/issues/244), BUG-157). A `mxcli test` or `run --test-endpoint` stopped by a signal skips its restore, and the next injection chains `RegisterEndpoint` to itself: StackOverflowError, app down. It happened twice in one day on one machine, and again on a second project. The setting is read from the settings unit's bytes, so no mxcli is needed. The fixture uses a settings unit captured from mxcli's own test model. `test-scratch.sh` exits 3 before copying. `app-restart.sh` exits 3 before stopping, except for a live `--test-endpoint` app, which holds the injection legitimately; if stopping that app leaves the setting behind, it exits 5 (new): stopped, not started. Projects pick this up on sync; a clean model sees no change. — MendixMau, second case AlvinnYeo - fix(app-restart): **the recorded pid is verified to still be an `mxcli run` before anything is stopped** (review of #304). With `--cmd` / `MXTK_APP_START_CMD`, the identity check read the override, not the process. A record left by a SIGKILL whose pid the OS had reused sent TERM/KILL to that unrelated process and its children. The pid's own command line (`/proc`, `ps`; on Windows the image name `mxcli.exe`) must now name mxcli and `run`. Otherwise the app counts as not running and the process is not touched. — MendixMau -- fix(test-scratch): **taking over a stale lock is serialised, so two runs can no longer both hold it** (review of #304). The takeover holds an O_EXCL `.takeover` marker (cleared after 30s), re-reads the lock, and removes it only if it still holds the line judged stale. A changed lock is put back with `ln`, never `mv -f`. A deterministic replay of the race fails on the old code. — MendixMau +- fix(test-scratch): **taking over a stale lock is serialised with exactly exec.sh's protocol, so test-scratch and exec.sh never both hold the shared `.exec.lock`** (review of #304). The old takeover moved the lock aside with `mv -f`. It now uses the same O_EXCL `.takeover` file and rules as exec.sh in #324: stale when its pid is dead or after 10s, re-read under it, remove only the exact stale line, put back a wrongly moved takeover file with `ln`. The port lock uses the same protocol. A new cross-script fixture, `test-shared-lock-takeover.sh`, runs 5 exec.sh + 5 test-scratch contenders on a dead holder's lock: two holders at once on the old code, never on the new. — MendixMau - learn(lint-that-actually-runs): **lint a copy of the model only after copying `.claude/lint-rules` (and `lint-config.yaml`) beside it, and compare rule counts before diffing** ([#240](https://github.com/MendixMau/mxcli-project-toolkit/issues/240), BUG-158). A snapshot in `/tmp` gave 51 findings from 8 rules, against 721 from 29 for the project: no notice, exit 0. mxcli's source also shows `lint-config.yaml` is read only from beside the `.mpr`, so an in-repo snapshot loses it as well. — MendixMau - learn(testing-shape, test-result-audit): **three test-strength rules, each with a denominator: refusals probed on the server, every undeclared 4xx a failure, one clean-database final run** ([#172](https://github.com/MendixMau/mxcli-project-toolkit/issues/172)). A field comparison of two builds against one frozen spec found the toolkit build checked permission refusals only in the client (a hidden menu item), logged unexpected 400/422 responses as information, and reached 20/20 by stitching three runs. `testing-shape.md` §7 gains three report lines (`N of N restricted pairs probed server-side`, `undeclared 4xx/5xx: 0 of M requests — each fails its step`, `one clean-DB run : N of N scenarios incl. timers, not stitched`); `test-result-audit.md` gains check 2e, which carries the reasons and the field evidence and asks for those numbers. Both baseline skills stay inside the 80,000-word baseline budget (79,997): the reasons live in the on-demand audit skill, and two redundant sentences in `testing-shape.md` §4's verdict paragraphs were folded (the empty-set corollary is stated in the Trace row, `harness-architecture.md` and `report-schema.md`). The backing inbox note is trimmed to its unpromoted "Keep" items. — MendixMau - learn(learned-detection-gaps, module-review): **two UI defects every model gate passes: a lazy-loaded combo box filter hides its preset, and a theme that pins row heights by position lets the next row cover an inserted widget** ([#299](https://github.com/MendixMau/mxcli-project-toolkit/issues/299)). Both passed check, mxbuild, lint and the scripted checks on a 127-module production app and were found only in the running app. Two register rows with the first oracle and the pre-build check (pluggable properties that change *when* data loads; grep the theme for fixed `height` on positional row classes before an insert), and two LOOK lines in `module-review.md` §4c with denominators (`N of N filters show their preset on first load`, `N of N widgets inserted into an existing container fully visible`). — jopterhorst diff --git a/project-bin/test-scratch.sh b/project-bin/test-scratch.sh index dc3055a6..413a2292 100755 --- a/project-bin/test-scratch.sh +++ b/project-bin/test-scratch.sh @@ -138,27 +138,38 @@ holder_live() { case "$_args" in ""|*exec.sh*|*test-scratch.sh*) return 0 ;; esac return 1 } -# Taking over a stale lock is itself serialised (#304 review): two runs that both read the same -# dead holder must not both remove "the stale lock" — the second would remove the lock the first -# had just taken, and both would run. So a takeover first wins "$f.takeover" (noclobber: O_EXCL; -# one older than 30s is a takeover that died midway and is cleared), re-reads the lock, and -# removes it only if it still holds the stale line it judged. It moves the lock aside before -# checking; a lock that changed in between is put back with `ln`, which refuses to overwrite, -# never `mv -f`, which would clobber a lock taken meanwhile. -take_over() { # $1 = lock file, $2 = the stale line. 0 = handled (retry the take), 1 = another takeover runs - local f="$1" held="$2" t="$1.takeover" ts - if ! ( set -o noclobber; printf '%s %s\n' "$$" "$(date +%s)" > "$t" ) 2>/dev/null; then - ts="$(cut -d' ' -f2 "$t" 2>/dev/null | head -1)" - case "$ts" in ''|*[!0-9]*) ;; *) [ $(( $(date +%s) - ts )) -gt 30 ] && rm -f "$t" ;; esac - return 1 - fi - if [ "$(head -1 "$f" 2>/dev/null || true)" = "$held" ] && mv "$f" "$f.stale.$$" 2>/dev/null; then - if [ "$(head -1 "$f.stale.$$" 2>/dev/null || true)" != "$held" ]; then - ln "$f.stale.$$" "$f" 2>/dev/null || true # not the line we judged: put it back, never over a newer one +# Taking over a stale lock is serialised through a second O_EXCL file, .takeover — the SAME +# file name and protocol as exec.sh's exec_lock_takeover (this script shares .exec.lock with it, so +# the two must exclude each other; keep the bodies in step). Under it the lock is re-read and +# removed ONLY if it still holds exactly the stale line judged; nothing else ever removes a lock +# that is not its own. WHY (independent review of #304): the old takeover here moved the lock +# aside with `mv -f` — check-then-act — so two runs that judged the same dead holder could both +# end up holding it. A takeover file left by a holder killed mid-takeover is stale once its pid is +# dead or it is older than 10 s (an EMPTY one once it is a minute old); it is moved aside, and put +# back with `ln` (fails if a new one exists — never clobbers) if what was moved is not the one +# judged stale. The machine-wide port lock uses the same protocol. +take_over() { # $1 = lock file, $2 = the stale lock line. 0 iff it was removed (or already gone) + local _tk="$1.takeover" _t _tpid _tsince _tstale _now + if ! ( set -o noclobber; printf '%s %s\n' "$$" "$(date +%s)" > "$_tk" ) 2>/dev/null; then + _t="$(head -1 "$_tk" 2>/dev/null || true)"; _tpid="${_t%% *}"; _tsince="${_t#* }" + case "$_tsince" in ''|*[!0-9]*) _tsince=0 ;; esac + _tstale=0 + if [ -n "$_t" ]; then + { ! kill -0 "$_tpid" 2>/dev/null || [ $(( $(date +%s) - _tsince )) -gt 10 ]; } && _tstale=1 + elif [ -n "$(find "$_tk" -mmin +0 2>/dev/null)" ]; then + _tstale=1 # empty for a minute: its creator died between the create and the write fi - rm -f "$f.stale.$$" + if [ "$_tstale" = 1 ]; then + if mv "$_tk" "$_tk.stale.$$" 2>/dev/null; then + [ "$(head -1 "$_tk.stale.$$" 2>/dev/null || true)" = "$_t" ] || ln "$_tk.stale.$$" "$_tk" 2>/dev/null || true + rm -f "$_tk.stale.$$" + fi + fi + return 1 fi - rm -f "$t" + _now="$(head -1 "$1" 2>/dev/null || true)" + [ "$_now" = "$2" ] && rm -f "$1" + rm -f "$_tk" return 0 } take_lock() { # $1 = lock file, $2 = what it guards (for messages). 0 = taken, 1 = live holder diff --git a/tests/wave2/fixtures/mxtest-guard/exec-lock-block.sh b/tests/wave2/fixtures/mxtest-guard/exec-lock-block.sh new file mode 100644 index 00000000..a6a31c34 --- /dev/null +++ b/tests/wave2/fixtures/mxtest-guard/exec-lock-block.sh @@ -0,0 +1,81 @@ +# exec-lock-block.sh — VERBATIM copy of project-bin/exec.sh's lock block (exec_lock_live .. the +# LOCK_TAKEN check) from PR #324, branch fix/exec-followups @ ed4efbb. Used by +# test-shared-lock-takeover.sh until #324 is merged; that fixture prefers the real exec.sh's block +# once it carries exec_lock_takeover. Do not edit by hand — re-copy. +exec_lock_live() { # $1 = pid from the lock; 0 iff that process is alive and plausibly exec.sh + [ -n "$1" ] || return 1 + case "$1" in *[!0-9]*) return 1 ;; esac + kill -0 "$1" 2>/dev/null || return 1 + # Linux: an unreaped zombie still answers kill -0; /proc//stat says Z after the "(comm) ". + case "$(sed 's/.*) //' "/proc/$1/stat" 2>/dev/null | cut -d' ' -f1)" in Z) return 1 ;; esac + _args="$(ps -o args= -p "$1" 2>/dev/null || true)" + case "$_args" in ""|*exec.sh*|*test-scratch.sh*) return 0 ;; esac # test-scratch.sh holds it while copying + return 1 +} +exec_lock_try() { ( set -o noclobber; printf '%s %s %s\n' "$$" "$(date +%s)" "$LOG_NAME" > "$LOCK" ) 2>/dev/null; } +exec_lock_release() { # the trap: remove the lock only while it is still ours + _own="$(cat "$LOCK" 2>/dev/null || true)" + [ "${_own%% *}" = "$$" ] && rm -f "$LOCK" + return 0 +} +# Taking over a stale lock is serialised through a second O_EXCL file, $LOCK.takeover. Under +# it the lock is re-read and removed ONLY if it still holds exactly the stale line judged above; +# nothing else ever removes a lock that is not its own, so the compare-then-rm cannot hit a +# fresh lock. WHY (independent review of #304): the old takeover moved the lock aside and put +# it back with `mv -f` when it found a fresh one there — check-then-act. With 3+ contenders, B +# moved A's fresh lock aside, C's noclobber write landed in the gap, B saw a lock and deleted +# A's: 12 contenders gave up to 3 holders at once (tests/wave2/test-exec-lock-watch.sh). +# A takeover file left by a holder killed inside those few microseconds is stale once its pid is +# dead or it is older than 10 s (an EMPTY one once it is a minute old); it is moved aside, and put back with `ln` (fails if a new one +# exists — never clobbers) if what was moved is not the one judged stale. +exec_lock_takeover() { # $1 = the stale lock line; 0 iff it was removed (or already gone) + _tk="$LOCK.takeover" + if ! ( set -o noclobber; printf '%s %s\n' "$$" "$(date +%s)" > "$_tk" ) 2>/dev/null; then + _t="$(head -1 "$_tk" 2>/dev/null || true)"; _tpid="${_t%% *}"; _tsince="${_t#* }" + case "$_tsince" in ''|*[!0-9]*) _tsince=0 ;; esac + _tstale=0 + if [ -n "$_t" ]; then + { ! kill -0 "$_tpid" 2>/dev/null || [ $(( $(date +%s) - _tsince )) -gt 10 ]; } && _tstale=1 + elif [ -n "$(find "$_tk" -mmin +0 2>/dev/null)" ]; then + _tstale=1 # empty for a minute: its creator died between the create and the write + fi + if [ "$_tstale" = 1 ]; then + if mv "$_tk" "$_tk.stale.$$" 2>/dev/null; then + [ "$(head -1 "$_tk.stale.$$" 2>/dev/null || true)" = "$_t" ] || ln "$_tk.stale.$$" "$_tk" 2>/dev/null || true + rm -f "$_tk.stale.$$" + fi + fi + return 1 + fi + _now="$(head -1 "$LOCK" 2>/dev/null || true)" + [ "$_now" = "$1" ] && rm -f "$LOCK" + rm -f "$_tk" + return 0 +} +LOCK_TAKEN=0 +for _try in 1 2 3 4 5 6; do + if exec_lock_try; then LOCK_TAKEN=1; break; fi + _held="$(head -1 "$LOCK" 2>/dev/null || true)" + if [ -z "$_held" ] && [ "$_try" -lt 3 ]; then sleep 1; continue; fi # being written right now + _hpid="${_held%% *}" + if exec_lock_live "$_hpid"; then + _hrest="${_held#* }"; _hsince="${_hrest%% *}"; _hscript="${_hrest#* }" + _age="" + case "$_hsince" in ''|*[!0-9]*) ;; *) _age=" for $(( $(date +%s) - _hsince ))s" ;; esac + [ "$_hrest" = "$_held" ] && _hscript="?" + echo "✗ Another session holds this project's exec lock (PID $_hpid, running$_age, script: $_hscript)." + echo " → Wait for it to finish, then re-run. Nothing was written." + echo " (A dead holder is detected and taken over automatically — no manual rm is ever needed.)" + exit 1 + fi + if exec_lock_takeover "$_held"; then + echo " (stale exec lock from PID ${_hpid:-?}, which is not running — taking it over)" + else + sleep 1 # another contender is taking it over right now; look again + fi +done +if [ "$LOCK_TAKEN" != 1 ]; then + echo "✗ Could not take the exec lock $LOCK after 6 attempts — another session keeps re-taking it." + echo " → Wait for the other exec to finish, then re-run. Nothing was written." + exit 1 +fi diff --git a/tests/wave2/test-shared-lock-takeover.sh b/tests/wave2/test-shared-lock-takeover.sh new file mode 100644 index 00000000..83a8ef07 --- /dev/null +++ b/tests/wave2/test-shared-lock-takeover.sh @@ -0,0 +1,75 @@ +#!/usr/bin/env bash +# test-shared-lock-takeover.sh — exec.sh and test-scratch.sh share .mpr-snapshots/.exec.lock; contending +# on one DEAD-holder lock, they must never both hold it. +# +# Usage: bash tests/wave2/test-shared-lock-takeover.sh [path-to-test-scratch.sh] [path-to-exec.sh] +# +# WHY (review of #304, PR #324): both scripts take the same per-project lock and both take over a dead +# holder's lock. Each was made safe on its own by a serialised takeover (an O_EXCL `.takeover`, +# re-read, remove only the exact stale line). Two private protocols would not exclude EACH OTHER: the +# takeover file name and the compare-then-remove rule must be the same in both scripts. +# +# How: the lock code is lifted from each script (exec.sh: exec_lock_live .. the LOCK_TAKEN check; +# test-scratch.sh: holder_live .. take_lock) into two small contender scripts whose names contain +# "exec.sh" / "test-scratch.sh", so each script's liveness check recognises the other as a holder. +# Rounds of 5 + 5 contenders start together on a lock whose holder pid is dead; each holder marks +# itself in a shared dir while it holds the lock. Two marks at once = two holders = FAIL. +# When the given exec.sh has no exec_lock_takeover yet (PR #324 unmerged), its block comes from the +# verbatim copy in fixtures/mxtest-guard/exec-lock-block.sh, and says so. +set -uo pipefail +TOOLKIT="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)" +TS="${1:-$TOOLKIT/project-bin/test-scratch.sh}"; EX="${2:-$TOOLKIT/project-bin/exec.sh}" +PASS=0; FAIL=0 +ok() { PASS=$((PASS+1)); echo " ok — $1"; } +bad() { FAIL=$((FAIL+1)); echo " FAIL — $1"; } +WORK="$(mktemp -d "${TMPDIR:-/tmp}/sharedlock.XXXXXX")" || exit 2 +trap 'rm -rf "$WORK"' EXIT +echo "== test-scratch: $TS" + +BLOCK="$WORK/exec-block.sh" +awk '/^exec_lock_live\(\) \{/ {p=1} p {print} p && /^if \[ "\$LOCK_TAKEN" != 1 \]; then$/ {e=1} e && /^fi$/ {exit}' "$EX" > "$BLOCK" +if grep -q '^exec_lock_takeover() {' "$BLOCK"; then echo "== exec.sh lock block: $EX" +else + sed '1,4d' \ + "$TOOLKIT/tests/wave2/fixtures/mxtest-guard/exec-lock-block.sh" > "$BLOCK" + echo "== exec.sh lock block: verbatim copy from PR #324 ($EX has no exec_lock_takeover yet)" +fi +grep -q '^exec_lock_takeover() {' "$BLOCK" && ok "[setup] exec.sh lock block lifted" || { bad "[setup] no exec.sh lock block"; echo "PASS=$PASS FAIL=$FAIL"; exit 1; } +awk '/^holder_live\(\) \{/ {p=1} /^release\(\) \{/ {p=0} p' "$TS" > "$WORK/ts-block.sh" +grep -q '^take_lock() {' "$WORK/ts-block.sh" && ok "[setup] test-scratch take_lock lifted" || { bad "[setup] no take_lock in $TS"; echo "PASS=$PASS FAIL=$FAIL"; exit 1; } + +LOCK="$WORK/.exec.lock"; HOLD="$WORK/holding"; mkdir -p "$HOLD" +hold() { # body shared by both contenders: mark, look for another mark, unmark + cat <<'H' +touch "$HOLD/$$"; n=$(ls "$HOLD" | wc -l | tr -d ' ') +[ "$n" -gt 1 ] && echo "OVERLAP $$ with $(ls "$HOLD" | tr '\n' ' ')" >> "$WORK/overlaps" +echo "$ROLE $$" >> "$WORK/holders"; sleep 0.3; rm -f "$HOLD/$$" +H +} +{ echo 'LOCK="$1"; LOG_NAME="exec-contender"; ROLE=exec'; cat "$BLOCK"; hold + echo '_own="$(head -1 "$LOCK" 2>/dev/null)"; [ "${_own%% *}" = "$$" ] && rm -f "$LOCK"'; } > "$WORK/contender-exec.sh" +{ echo 'LOCK="$1"; ROLE=scratch'; cat "$WORK/ts-block.sh" + echo 'take_lock "$LOCK" "the exec lock" >/dev/null 2>&1 || exit 1'; hold + echo '_own="$(head -1 "$LOCK" 2>/dev/null)"; [ "${_own%% *}" = "$$" ] && rm -f "$LOCK"'; } > "$WORK/contender-test-scratch.sh" + +sleep 0 & DEAD=$!; wait "$DEAD" # a pid that is certainly not running now +ROUNDS=4 +for r in $(seq 1 "$ROUNDS"); do + printf '%s %s exec.sh\n' "$DEAD" "$(date +%s)" > "$LOCK"; rm -f "$LOCK.takeover" + PIDS="" + for i in 1 2 3 4 5; do + WORK="$WORK" HOLD="$HOLD" bash "$WORK/contender-exec.sh" "$LOCK" >/dev/null 2>&1 & PIDS="$PIDS $!" + WORK="$WORK" HOLD="$HOLD" bash "$WORK/contender-test-scratch.sh" "$LOCK" >/dev/null 2>&1 & PIDS="$PIDS $!" + done + for p in $PIDS; do wait "$p"; done +done +N="$(wc -l < "$WORK/holders" 2>/dev/null | tr -d ' ')" +[ "${N:-0}" -ge "$ROUNDS" ] && ok "the dead holder's lock was taken over in every round ($N holds over $ROUNDS rounds)" || bad "only ${N:-0} holds in $ROUNDS rounds" +grep -q '^exec ' "$WORK/holders" 2>/dev/null && grep -q '^scratch ' "$WORK/holders" 2>/dev/null \ + && ok "both scripts won the lock at some point (they really contended)" || echo " note: one script won every round: $(cut -d' ' -f1 "$WORK/holders" 2>/dev/null | sort | uniq -c | tr '\n' ' ')" +if [ -s "$WORK/overlaps" ]; then bad "two holders at once: $(head -3 "$WORK/overlaps" | tr '\n' ' ')"; else ok "never two holders at once"; fi +[ -e "$LOCK.takeover" ] && bad "takeover file left behind" || ok "no takeover file left behind" + +echo "" +echo "PASS=$PASS FAIL=$FAIL" +[ "$FAIL" -eq 0 ] diff --git a/tests/wave2/test-test-scratch.sh b/tests/wave2/test-test-scratch.sh index ecb26de0..830d1c62 100755 --- a/tests/wave2/test-test-scratch.sh +++ b/tests/wave2/test-test-scratch.sh @@ -114,7 +114,7 @@ OUT="$( L="$L" LIVE="$LIVE" bash -c ' has "the second taker is refused (the live lock survives)" "$OUT" "rc=1" [ "$(cut -d' ' -f1 "$L" 2>/dev/null)" = "$LIVE" ] && ok "the lock still names the live holder" || bad "the live holder's lock was replaced: $(cat "$L" 2>/dev/null)" [ -e "$L.takeover" ] && bad "takeover marker left behind" || ok "takeover marker cleared" -echo "-- a takeover marker older than 30s (a takeover that died midway) is cleared" +echo "-- a takeover marker whose pid is dead (a takeover that died midway) is cleared" printf '999999 1 test-scratch.sh\n' > "$L"; printf '999998 1\n' > "$L.takeover" OUT="$( L="$L" bash -c '. "$1"; holder_live() { return 1; }; take_lock "$L" "race lock"; echo "rc=$?"' _ "$H" 2>&1 )" has "taken after clearing the dead marker" "$OUT" "rc=0" From ab21867725f531243fe6e83d5dfef33b9a684b70 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 13 Oct 2026 15:38:31 +0000 Subject: [PATCH 3/5] =?UTF-8?q?mxtest=20guard:=20review=20fixes=20?= =?UTF-8?q?=E2=80=94=20pipefail-safe=20reader,=20all=20candidate=20units,?= =?UTF-8?q?=20MxTest.=20prefilter,=20override?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Independent review of #322 at 333d435: 1. The reader ran under the callers' pipefail: awk exits at the first match, od dies of SIGPIPE, and the 141 discarded the answer — on a large v1 .mpr the guard did nothing. It is now a subshell function with pipefail off. Fixture: 2 MB v1 model after the setting. 2. `grep -rla … | head -1` took the first unit mentioning the name; a doc string elsewhere switched the guard off. Every candidate unit is read; only the BSON element counts. Fixture: doc-string units beside the injected settings unit. 3. Units without the bytes "MxTest." are not scanned (an 8 MB v1 .mpr took 8 s). 4. MXTK_SKIP_MXTEST_GUARD=1 skips the check, printed loudly (both fixtures). 5. test-app-restart.sh alive() is zombie-aware (3/10 flakes; now 5/5 clean). 6. in_our_tree's comment is back above it; set-after-startup.js cites the real fixture. 7. test-scratch's remedy for a live --test-endpoint app: test a copy whose settings unit comes from HEAD (#244's workaround) before stopping the live app. Refs #244 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01VJgWP5vEoAsNsJYqCDMGNw --- CHANGELOG.md | 2 +- bug-logs/mxcli-bugs.md | 2 +- project-bin/app-restart.sh | 65 +++++++++++++------ project-bin/test-scratch.sh | 63 +++++++++++++----- .../mxtest-guard/set-after-startup.js | 2 +- tests/wave2/test-app-restart.sh | 5 +- tests/wave2/test-test-scratch.sh | 22 ++++++- 7 files changed, 117 insertions(+), 44 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 398efbf3..a4bab42d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,7 +17,7 @@ Sections dated before 2026-09-19 predate the cycle and stay as they are. ## Unreleased -- fix(test-scratch, app-restart): **both refuse to start on a model whose after-startup is still an mxcli test injection (`MxTest.RegisterEndpoint` / `MxTest.TestRunner`), and print the repair** ([#244](https://github.com/MendixMau/mxcli-project-toolkit/issues/244), BUG-157). A `mxcli test` or `run --test-endpoint` stopped by a signal skips its restore, and the next injection chains `RegisterEndpoint` to itself: StackOverflowError, app down. It happened twice in one day on one machine, and again on a second project. The setting is read from the settings unit's bytes, so no mxcli is needed. The fixture uses a settings unit captured from mxcli's own test model. `test-scratch.sh` exits 3 before copying. `app-restart.sh` exits 3 before stopping, except for a live `--test-endpoint` app, which holds the injection legitimately; if stopping that app leaves the setting behind, it exits 5 (new): stopped, not started. Projects pick this up on sync; a clean model sees no change. — MendixMau, second case AlvinnYeo +- fix(test-scratch, app-restart): **both refuse to start on a model whose after-startup is still an mxcli test injection (`MxTest.RegisterEndpoint` / `MxTest.TestRunner`), and print the repair** ([#244](https://github.com/MendixMau/mxcli-project-toolkit/issues/244), BUG-157). A `mxcli test` or `run --test-endpoint` stopped by a signal skips its restore, and the next injection chains `RegisterEndpoint` to itself: StackOverflowError, app down. It happened twice in one day on one machine, and again on a second project. The setting is read from the settings unit's bytes, so no mxcli is needed. Every unit holding the BSON element is checked, not just the first one that mentions the name. A unit without the bytes `MxTest.` is skipped, so an 8 MB v1 model costs one grep. The reader runs with pipefail off, so a large v1 `.mpr` cannot switch the guard off. `MXTK_SKIP_MXTEST_GUARD=1` skips the check and says so. The fixture uses a settings unit captured from mxcli's own test model. `test-scratch.sh` exits 3 before copying. `app-restart.sh` exits 3 before stopping, except for a live `--test-endpoint` app, which holds the injection legitimately; if stopping that app leaves the setting behind, it exits 5 (new): stopped, not started. Projects pick this up on sync; a clean model sees no change. — MendixMau, second case AlvinnYeo - fix(app-restart): **the recorded pid is verified to still be an `mxcli run` before anything is stopped** (review of #304). With `--cmd` / `MXTK_APP_START_CMD`, the identity check read the override, not the process. A record left by a SIGKILL whose pid the OS had reused sent TERM/KILL to that unrelated process and its children. The pid's own command line (`/proc`, `ps`; on Windows the image name `mxcli.exe`) must now name mxcli and `run`. Otherwise the app counts as not running and the process is not touched. — MendixMau - fix(test-scratch): **taking over a stale lock is serialised with exactly exec.sh's protocol, so test-scratch and exec.sh never both hold the shared `.exec.lock`** (review of #304). The old takeover moved the lock aside with `mv -f`. It now uses the same O_EXCL `.takeover` file and rules as exec.sh in #324: stale when its pid is dead or after 10s, re-read under it, remove only the exact stale line, put back a wrongly moved takeover file with `ln`. The port lock uses the same protocol. A new cross-script fixture, `test-shared-lock-takeover.sh`, runs 5 exec.sh + 5 test-scratch contenders on a dead holder's lock: two holders at once on the old code, never on the new. — MendixMau - learn(lint-that-actually-runs): **lint a copy of the model only after copying `.claude/lint-rules` (and `lint-config.yaml`) beside it, and compare rule counts before diffing** ([#240](https://github.com/MendixMau/mxcli-project-toolkit/issues/240), BUG-158). A snapshot in `/tmp` gave 51 findings from 8 rules, against 721 from 29 for the project: no notice, exit 0. mxcli's source also shows `lint-config.yaml` is read only from beside the `.mpr`, so an in-repo snapshot loses it as well. — MendixMau diff --git a/bug-logs/mxcli-bugs.md b/bug-logs/mxcli-bugs.md index 6e80b6e0..29d640bc 100644 --- a/bug-logs/mxcli-bugs.md +++ b/bug-logs/mxcli-bugs.md @@ -6885,7 +6885,7 @@ setting from the model bytes before starting. The setting is the BSON string `Af in the project-settings unit (`mprcontents/4e/70/4e70063d-….mxunit`, or inside a v1 `.mpr`). This needs no mxcli. When the value is `MxTest.RegisterEndpoint` or `MxTest.TestRunner`, both scripts refuse and print the repair. `app-restart.sh` lets a live `--test-endpoint` app through, because it -holds the injection legitimately, and checks again after the stop. +holds the injection legitimately, and checks again after the stop. `MXTK_SKIP_MXTEST_GUARD=1` skips the check out loud, so a false refusal never blocks. **Repair** (any one of): - `git checkout -- mprcontents/4e/70/`, then `git status`: the run may also have left an `MxTest` module. diff --git a/project-bin/app-restart.sh b/project-bin/app-restart.sh index c9529a51..442f05e7 100755 --- a/project-bin/app-restart.sh +++ b/project-bin/app-restart.sh @@ -187,16 +187,20 @@ blocked() { echo " Record this run as INSTRUMENT BLOCKED, not as a test failure." >&2 exit 4 } -# in_our_tree — the recorded pid itself, or a descendant of it (the runtime mxcli started). # ── After-startup left on an mxcli test injection (#244, BUG-157) ─────────── # `mxcli test` and `mxcli run --test-endpoint` point the model's after-startup microflow at # MxTest.RegisterEndpoint (MxTest.TestRunner for a plain test run) and chain the original; only a # clean exit puts it back. A run stopped by a signal leaves it, and the next injection then chains # RegisterEndpoint to itself: StackOverflowError in the after-startup, the runtime never starts. # Read straight from the model bytes — no mxcli, no running app: the setting is the BSON string -# element "AfterStartupMicroflow" of the project-settings unit (mprcontents/**/*.mxunit; inside the -# .mpr itself on a v1 model). Layout checked against real settings units (mxcli's own test models). -mxtest_after_startup() { # $1 = file, or - for stdin. Prints the value ('' = none); 1 = no such element +# element 0x02 "AfterStartupMicroflow" 0x00 of the project-settings unit (mprcontents/**/*.mxunit; +# inside the .mpr itself on a v1 model). Layout checked against real settings units (mxcli's own +# test models). A unit or .mpr without the bytes "MxTest." anywhere is not scanned at all (an 8 MB +# v1 .mpr costs a grep, not an 8 s od). MXTK_SKIP_MXTEST_GUARD=1 skips the check, out loud. +mxtest_after_startup() ( # $1 = file, or - for stdin. Prints the value ('' = none); 1 = no such element + # A subshell with pipefail OFF: awk exits at the first match and od then dies of SIGPIPE; under + # the caller's pipefail that 141 would hide the answer and switch the guard off on a large .mpr. + set +o pipefail { if [ "$1" = - ]; then od -An -v -tx1; else od -An -v -tx1 "$1"; fi; } 2>/dev/null | awk ' BEGIN { for (i = 0; i < 256; i++) hv[sprintf("%02x", i)] = i for (i = 32; i < 127; i++) { c = sprintf("%c", i); ch[sprintf("%02x", i)] = c; hx[c] = sprintf("%02x", i) } @@ -210,27 +214,41 @@ mxtest_after_startup() { # $1 = file, or - for stdin. Prints the value ('' = no if (++n == 4) { if (L < 1 || L > 1024) { st = 0; k = 0 } else if (L == 1) { found = 1; exit } else { st = 2; n = 0 } } } else { if (++n < L) v = v ((b in ch) ? ch[b] : "?"); else { found = 1; exit } } } } END { if (!found) exit 1; print v }' -} +) MXTEST_LEFT=""; MXTEST_UNIT=""; MXTEST_HEAD="" mxtest_left() { # $1 = model dir, $2 = .mpr. 0 = after-startup is an mxcli test injection (sets MXTEST_*) - local unit val + local unit val cands MXTEST_LEFT=""; MXTEST_UNIT=""; MXTEST_HEAD="" + if [ "${MXTK_SKIP_MXTEST_GUARD:-0}" = 1 ]; then + echo " ! MXTK_SKIP_MXTEST_GUARD=1 — NOT checking for a leftover mxcli test after-startup (#244). If the app" >&2 + echo " then dies with StackOverflowError in MxTest.RegisterEndpoint, that is what this check was for." >&2 + return 1 + fi if [ -d "$1/mprcontents" ]; then - unit="$(LC_ALL=C grep -rla --include='*.mxunit' 'AfterStartupMicroflow' "$1/mprcontents" 2>/dev/null | head -1)" - else unit="$2"; fi - [ -n "$unit" ] && [ -f "$unit" ] || return 1 - val="$(mxtest_after_startup "$unit")" || return 1 - case "$val" in MxTest.RegisterEndpoint|MxTest.TestRunner) ;; *) return 1 ;; esac - MXTEST_LEFT="$val"; MXTEST_UNIT="$unit" - case "$unit" in *.mxunit) # what HEAD has, so the repair can name it; "?" = not tracked / no git - MXTEST_HEAD="$(git -C "$(dirname "$unit")" show "HEAD:./$(basename "$unit")" 2>/dev/null | mxtest_after_startup -)" || MXTEST_HEAD="?" ;; - *) MXTEST_HEAD="?" ;; esac - return 0 + cands="$(LC_ALL=C grep -rla --include='*.mxunit' 'AfterStartupMicroflow' "$1/mprcontents" 2>/dev/null)" + else cands="$2"; fi + # Every unit holding the text, not the first: a doc string elsewhere can mention the setting by + # name. Only the BSON element counts, and only a unit that contains "MxTest." can hold the value. + while IFS= read -r unit; do + [ -n "$unit" ] && [ -f "$unit" ] || continue + LC_ALL=C grep -qa 'MxTest\.' "$unit" 2>/dev/null || continue + val="$(mxtest_after_startup "$unit")" || continue + case "$val" in MxTest.RegisterEndpoint|MxTest.TestRunner) ;; *) continue ;; esac + MXTEST_LEFT="$val"; MXTEST_UNIT="$unit" + case "$unit" in *.mxunit) # what HEAD has, so the repair can name it; "?" = not tracked / no git + MXTEST_HEAD="$(git -C "$(dirname "$unit")" show "HEAD:./$(basename "$unit")" 2>/dev/null | mxtest_after_startup -)" || MXTEST_HEAD="?" ;; + *) MXTEST_HEAD="?" ;; esac + return 0 + done <&2 - echo " run --test-endpoint that did not exit cleanly (stopped by a signal or killed; BUG-157)." >&2 + echo " run --test-endpoint that did not exit cleanly (stopped by a signal or killed; BUG-157)," >&2 + echo " or held right now by a live run --test-endpoint of this project." >&2 echo " Starting on it recurses (the next injection chains $MXTEST_LEFT to itself: StackOverflowError)," >&2 echo " or fails the build with CE1613 if the MxTest module is already gone. Either way the app never starts." >&2 echo " set in: $rel" >&2 @@ -238,15 +256,24 @@ mxtest_refusal() { # $1 = tool name. Prints the refusal and the repair to stder echo " HEAD: not readable (no git, or the unit is untracked)" >&2 ;; '') echo " HEAD: no after-startup microflow" >&2 ;; *) echo " HEAD: $was" >&2 ;; esac - echo " → If this project's app is running with --test-endpoint right now, this is its live injection:" >&2 - echo " stop it with Ctrl+C in its own terminal (a clean stop restores the setting), then retry." >&2 + if [ "$1" = test-scratch ]; then + echo " → If this project's app runs with --test-endpoint right now (skills/cloud-dev-environment.md), the" >&2 + echo " live model is fine as it is — test a copy whose settings unit comes from HEAD instead (#244's" >&2 + echo " workaround): git worktree add HEAD, copy the units under test into it, run mxcli test there" >&2 + echo " on free ports. Or stop the live app with Ctrl+C in its own terminal (a clean stop restores it)." >&2 + else + echo " → If this project's app is running with --test-endpoint right now, this is its live injection:" >&2 + echo " stop it with Ctrl+C in its own terminal (a clean stop restores the setting), then retry." >&2 + fi echo " → Otherwise restore it, then retry:" >&2 echo " git checkout -- '$rel' (then git status: the run may have left an MxTest module too)" >&2 echo " or through ./bin/exec.sh (an ALTER SETTINGS — say so in chat; it is what mxcli's own cleanup runs):" >&2 echo " mdl 1;" >&2 echo " alter settings runtime (AfterStartupMicroflow: '$was');" >&2 echo " or Studio Pro: App Settings → Runtime → After startup." >&2 + echo " A false refusal? MXTK_SKIP_MXTEST_GUARD=1 skips this check (and says so)." >&2 } +# in_our_tree — the recorded pid itself, or a descendant of it (the runtime mxcli started). in_our_tree() { local p="$1" i=0 [ "$ALIVE" = 1 ] || return 1 diff --git a/project-bin/test-scratch.sh b/project-bin/test-scratch.sh index 413a2292..f9ae8f2f 100755 --- a/project-bin/test-scratch.sh +++ b/project-bin/test-scratch.sh @@ -66,9 +66,14 @@ else echo "✗ test-scratch: no mxcli (project root or PATH)." >&2; exit 3; fi # clean exit puts it back. A run stopped by a signal leaves it, and the next injection then chains # RegisterEndpoint to itself: StackOverflowError in the after-startup, the runtime never starts. # Read straight from the model bytes — no mxcli, no running app: the setting is the BSON string -# element "AfterStartupMicroflow" of the project-settings unit (mprcontents/**/*.mxunit; inside the -# .mpr itself on a v1 model). Layout checked against real settings units (mxcli's own test models). -mxtest_after_startup() { # $1 = file, or - for stdin. Prints the value ('' = none); 1 = no such element +# element 0x02 "AfterStartupMicroflow" 0x00 of the project-settings unit (mprcontents/**/*.mxunit; +# inside the .mpr itself on a v1 model). Layout checked against real settings units (mxcli's own +# test models). A unit or .mpr without the bytes "MxTest." anywhere is not scanned at all (an 8 MB +# v1 .mpr costs a grep, not an 8 s od). MXTK_SKIP_MXTEST_GUARD=1 skips the check, out loud. +mxtest_after_startup() ( # $1 = file, or - for stdin. Prints the value ('' = none); 1 = no such element + # A subshell with pipefail OFF: awk exits at the first match and od then dies of SIGPIPE; under + # the caller's pipefail that 141 would hide the answer and switch the guard off on a large .mpr. + set +o pipefail { if [ "$1" = - ]; then od -An -v -tx1; else od -An -v -tx1 "$1"; fi; } 2>/dev/null | awk ' BEGIN { for (i = 0; i < 256; i++) hv[sprintf("%02x", i)] = i for (i = 32; i < 127; i++) { c = sprintf("%c", i); ch[sprintf("%02x", i)] = c; hx[c] = sprintf("%02x", i) } @@ -82,27 +87,41 @@ mxtest_after_startup() { # $1 = file, or - for stdin. Prints the value ('' = no if (++n == 4) { if (L < 1 || L > 1024) { st = 0; k = 0 } else if (L == 1) { found = 1; exit } else { st = 2; n = 0 } } } else { if (++n < L) v = v ((b in ch) ? ch[b] : "?"); else { found = 1; exit } } } } END { if (!found) exit 1; print v }' -} +) MXTEST_LEFT=""; MXTEST_UNIT=""; MXTEST_HEAD="" mxtest_left() { # $1 = model dir, $2 = .mpr. 0 = after-startup is an mxcli test injection (sets MXTEST_*) - local unit val + local unit val cands MXTEST_LEFT=""; MXTEST_UNIT=""; MXTEST_HEAD="" + if [ "${MXTK_SKIP_MXTEST_GUARD:-0}" = 1 ]; then + echo " ! MXTK_SKIP_MXTEST_GUARD=1 — NOT checking for a leftover mxcli test after-startup (#244). If the app" >&2 + echo " then dies with StackOverflowError in MxTest.RegisterEndpoint, that is what this check was for." >&2 + return 1 + fi if [ -d "$1/mprcontents" ]; then - unit="$(LC_ALL=C grep -rla --include='*.mxunit' 'AfterStartupMicroflow' "$1/mprcontents" 2>/dev/null | head -1)" - else unit="$2"; fi - [ -n "$unit" ] && [ -f "$unit" ] || return 1 - val="$(mxtest_after_startup "$unit")" || return 1 - case "$val" in MxTest.RegisterEndpoint|MxTest.TestRunner) ;; *) return 1 ;; esac - MXTEST_LEFT="$val"; MXTEST_UNIT="$unit" - case "$unit" in *.mxunit) # what HEAD has, so the repair can name it; "?" = not tracked / no git - MXTEST_HEAD="$(git -C "$(dirname "$unit")" show "HEAD:./$(basename "$unit")" 2>/dev/null | mxtest_after_startup -)" || MXTEST_HEAD="?" ;; - *) MXTEST_HEAD="?" ;; esac - return 0 + cands="$(LC_ALL=C grep -rla --include='*.mxunit' 'AfterStartupMicroflow' "$1/mprcontents" 2>/dev/null)" + else cands="$2"; fi + # Every unit holding the text, not the first: a doc string elsewhere can mention the setting by + # name. Only the BSON element counts, and only a unit that contains "MxTest." can hold the value. + while IFS= read -r unit; do + [ -n "$unit" ] && [ -f "$unit" ] || continue + LC_ALL=C grep -qa 'MxTest\.' "$unit" 2>/dev/null || continue + val="$(mxtest_after_startup "$unit")" || continue + case "$val" in MxTest.RegisterEndpoint|MxTest.TestRunner) ;; *) continue ;; esac + MXTEST_LEFT="$val"; MXTEST_UNIT="$unit" + case "$unit" in *.mxunit) # what HEAD has, so the repair can name it; "?" = not tracked / no git + MXTEST_HEAD="$(git -C "$(dirname "$unit")" show "HEAD:./$(basename "$unit")" 2>/dev/null | mxtest_after_startup -)" || MXTEST_HEAD="?" ;; + *) MXTEST_HEAD="?" ;; esac + return 0 + done <&2 - echo " run --test-endpoint that did not exit cleanly (stopped by a signal or killed; BUG-157)." >&2 + echo " run --test-endpoint that did not exit cleanly (stopped by a signal or killed; BUG-157)," >&2 + echo " or held right now by a live run --test-endpoint of this project." >&2 echo " Starting on it recurses (the next injection chains $MXTEST_LEFT to itself: StackOverflowError)," >&2 echo " or fails the build with CE1613 if the MxTest module is already gone. Either way the app never starts." >&2 echo " set in: $rel" >&2 @@ -110,14 +129,22 @@ mxtest_refusal() { # $1 = tool name. Prints the refusal and the repair to stder echo " HEAD: not readable (no git, or the unit is untracked)" >&2 ;; '') echo " HEAD: no after-startup microflow" >&2 ;; *) echo " HEAD: $was" >&2 ;; esac - echo " → If this project's app is running with --test-endpoint right now, this is its live injection:" >&2 - echo " stop it with Ctrl+C in its own terminal (a clean stop restores the setting), then retry." >&2 + if [ "$1" = test-scratch ]; then + echo " → If this project's app runs with --test-endpoint right now (skills/cloud-dev-environment.md), the" >&2 + echo " live model is fine as it is — test a copy whose settings unit comes from HEAD instead (#244's" >&2 + echo " workaround): git worktree add HEAD, copy the units under test into it, run mxcli test there" >&2 + echo " on free ports. Or stop the live app with Ctrl+C in its own terminal (a clean stop restores it)." >&2 + else + echo " → If this project's app is running with --test-endpoint right now, this is its live injection:" >&2 + echo " stop it with Ctrl+C in its own terminal (a clean stop restores the setting), then retry." >&2 + fi echo " → Otherwise restore it, then retry:" >&2 echo " git checkout -- '$rel' (then git status: the run may have left an MxTest module too)" >&2 echo " or through ./bin/exec.sh (an ALTER SETTINGS — say so in chat; it is what mxcli's own cleanup runs):" >&2 echo " mdl 1;" >&2 echo " alter settings runtime (AfterStartupMicroflow: '$was');" >&2 echo " or Studio Pro: App Settings → Runtime → After startup." >&2 + echo " A false refusal? MXTK_SKIP_MXTEST_GUARD=1 skips this check (and says so)." >&2 } # A model whose after-startup is an mxcli test injection must not be copied and tested: the copy diff --git a/tests/wave2/fixtures/mxtest-guard/set-after-startup.js b/tests/wave2/fixtures/mxtest-guard/set-after-startup.js index 83182623..807a18a1 100644 --- a/tests/wave2/fixtures/mxtest-guard/set-after-startup.js +++ b/tests/wave2/fixtures/mxtest-guard/set-after-startup.js @@ -1,7 +1,7 @@ #!/usr/bin/env node // set-after-startup.js — fixture helper: re-encode a captured Mendix settings unit (BSON) with // AfterStartupMicroflow = , every enclosing length rewritten. Re-encoding with the original -// value ('') reproduces the capture byte for byte (checked by test-mxtest-guard.sh). +// value ('') reproduces the capture byte for byte (checked by tests/wave2/test-test-scratch.sh). const fs = require('fs'); const [inp, out, value] = process.argv.slice(2); const buf = fs.readFileSync(inp); diff --git a/tests/wave2/test-app-restart.sh b/tests/wave2/test-app-restart.sh index c99ee3dd..847216fc 100755 --- a/tests/wave2/test-app-restart.sh +++ b/tests/wave2/test-app-restart.sh @@ -102,7 +102,8 @@ start_app() { # $1 = project dir, $2 = extra run flags (optional); waits for th local i; for i in $(seq 1 50); do [ -f "$1/.mxcli/run-local.json" ] && return 0; sleep 0.2; done; return 1 } hs_pid() { sed -n 's/^ "pid": *\([0-9]*\).*/\1/p' "$1/.mxcli/run-local.json" 2>/dev/null; } -alive() { [ -n "${1:-}" ] && kill -0 "$1" 2>/dev/null; } +# Zombie-aware: a stopped child nobody has reaped yet still answers kill -0 (3/10 flakes on "was not stopped"). +alive() { [ -n "${1:-}" ] && kill -0 "$1" 2>/dev/null || return 1; case "$(ps -o stat= -p "$1" 2>/dev/null)" in Z*) return 1 ;; esac; } restart() { ( cd "$1" && shift && FAKE_GOLDEN="$GOLDEN" LOGDIR="$WORK" ./bin/app-restart.sh "$@" ) 2>&1; } PORT=$(( 20000 + RANDOM % 20000 )) @@ -193,6 +194,8 @@ has "names the leftover" "$OUT" "after-startup microflow is MxTest.RegisterEndpo has "prints the MDL repair" "$OUT" "alter settings runtime (AfterStartupMicroflow:" has "says nothing was stopped" "$OUT" "Nothing was stopped" alive "$CUR" && ok "the app was left running" || bad "the app was stopped" +OUT="$(MXTK_SKIP_MXTEST_GUARD=1 restart "$P" --dry-run)"; RC=$? +[ "$RC" -eq 0 ] && has "MXTK_SKIP_MXTEST_GUARD=1 lets it through, out loud" "$OUT" "NOT checking" || bad "override exit $RC" cp "$GU" "$UNIT" echo "-- #244: a live --test-endpoint app holds the injection legitimately; a stop that does not restore it → exit 5" diff --git a/tests/wave2/test-test-scratch.sh b/tests/wave2/test-test-scratch.sh index 830d1c62..1850d108 100755 --- a/tests/wave2/test-test-scratch.sh +++ b/tests/wave2/test-test-scratch.sh @@ -150,11 +150,27 @@ if command -v node >/dev/null 2>&1; then OUT="$(run l -- tests/a.test.mdl)"; RC=$? [ "$RC" -eq 3 ] && ok "MxTest.TestRunner → refused (exit 3)" || bad "TestRunner exit $RC" cp "$GU" "$UNIT" - echo "-- #244 on a v1 model (settings inside the .mpr itself)" + echo "-- #244: another unit that merely MENTIONS the setting (a doc string) does not switch the guard off" + D1="$P/mprcontents/00/00/00000000-doc.mxunit"; D2="$P/mprcontents/ff/ff/ffffffff-doc.mxunit"; mkdir -p "$(dirname "$D1")" "$(dirname "$D2")" + printf 'Documentation\0Set AfterStartupMicroflow to MxTest.Setup for tests\0' | tee "$D1" > "$D2" + node "$SETAS" "$GU" "$UNIT" "MxTest.RegisterEndpoint" + OUT="$(run n -- tests/a.test.mdl)"; RC=$? + [ "$RC" -eq 3 ] && ok "doc-string units beside the injected settings unit → still refused" || bad "doc-string unit hid the injection (exit $RC)" + echo "-- #244: MXTK_SKIP_MXTEST_GUARD=1 lets a run through, out loud" + OUT="$(MXTK_SKIP_MXTEST_GUARD=1 run o -- tests/a.test.mdl)"; RC=$? + [ "$RC" -eq 0 ] && has "skipped, and says so" "$OUT" "MXTK_SKIP_MXTEST_GUARD=1 — NOT checking" || bad "override exit $RC" + rm -f "$D1" "$D2" + echo "-- #244 on a v1 model (settings inside the .mpr itself), 2 MB of model after the setting" + # Large on purpose: the reader stops at the first match, od then dies of SIGPIPE, and under the + # script's pipefail that once discarded the answer — the guard silently did nothing on a real .mpr. V="$WORK/v1"; mkdir -p "$V/bin"; cp "$P/bin/"*.sh "$V/bin/"; cp "$P/mxcli" "$V/" - { printf 'SQLite format 3\0'; node "$SETAS" "$GU" /dev/stdout "MxTest.RegisterEndpoint"; printf '\0\0trailing'; } > "$V/Fixture.mpr" + node "$SETAS" "$GU" "$WORK/inj.mxunit" "MxTest.RegisterEndpoint" + { printf 'SQLite format 3\0'; cat "$WORK/inj.mxunit"; head -c 2000000 /dev/zero; } > "$V/Fixture.mpr" OUT="$( cd "$V" && REC="$WORK/rec.m" ./bin/test-scratch.sh -- x.test.mdl 2>&1 )"; RC=$? - [ "$RC" -eq 3 ] && has "v1 .mpr → refused, git repair degrades to a placeholder" "$OUT" "HEAD: not readable" || bad "v1 exit $RC: $(printf '%s' "$OUT" | tail -3)" + [ "$RC" -eq 3 ] && has "large v1 .mpr → refused, git repair degrades to a placeholder" "$OUT" "HEAD: not readable" || bad "v1 exit $RC: $(printf '%s' "$OUT" | tail -3)" + { printf 'SQLite format 3\0'; cat "$GU"; head -c 2000000 /dev/zero; } > "$V/Fixture.mpr" + OUT="$( cd "$V" && REC="$WORK/rec.p" ./bin/test-scratch.sh -- x.test.mdl 2>&1 )"; RC=$? + [ "$RC" -eq 0 ] && ok "large clean v1 .mpr → runs" || bad "clean v1 exit $RC: $(printf '%s' "$OUT" | tail -3)" else echo " [skip] node missing — the injected settings units are built from the capture with node" fi From 4785a8c5dd8c722d723225c4d1e1a6e645d34562 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 13 Oct 2026 15:40:13 +0000 Subject: [PATCH 4/5] test-scratch take_over: -mmin +1 and own-file-only removal (in step with #324's review) `find -mmin +0` treats a fresh empty takeover file as stale at once (GNU matches 0-2 s old files, BSD rounds up), so a takeover in progress could be broken into: two holders under load. Now -mmin +1 (empty for over a minute). The takeover file is removed only when its content is what was judged stale (a moved-aside live one is put back with ln, and kept if that fails), and a finished takeover removes the file only while it still holds its own line. Fixture: a fresh empty takeover file blocks the takeover and is left alone; one over a minute old (touch -t 2000) is stale. Fails on the previous body. Keep in step with exec.sh's exec_lock_takeover (PR #324). Refs #244 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01VJgWP5vEoAsNsJYqCDMGNw --- CHANGELOG.md | 2 +- project-bin/test-scratch.sh | 21 +++++++++++++-------- tests/wave2/test-test-scratch.sh | 12 ++++++++++++ 3 files changed, 26 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a4bab42d..6532333e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,7 +19,7 @@ Sections dated before 2026-09-19 predate the cycle and stay as they are. - fix(test-scratch, app-restart): **both refuse to start on a model whose after-startup is still an mxcli test injection (`MxTest.RegisterEndpoint` / `MxTest.TestRunner`), and print the repair** ([#244](https://github.com/MendixMau/mxcli-project-toolkit/issues/244), BUG-157). A `mxcli test` or `run --test-endpoint` stopped by a signal skips its restore, and the next injection chains `RegisterEndpoint` to itself: StackOverflowError, app down. It happened twice in one day on one machine, and again on a second project. The setting is read from the settings unit's bytes, so no mxcli is needed. Every unit holding the BSON element is checked, not just the first one that mentions the name. A unit without the bytes `MxTest.` is skipped, so an 8 MB v1 model costs one grep. The reader runs with pipefail off, so a large v1 `.mpr` cannot switch the guard off. `MXTK_SKIP_MXTEST_GUARD=1` skips the check and says so. The fixture uses a settings unit captured from mxcli's own test model. `test-scratch.sh` exits 3 before copying. `app-restart.sh` exits 3 before stopping, except for a live `--test-endpoint` app, which holds the injection legitimately; if stopping that app leaves the setting behind, it exits 5 (new): stopped, not started. Projects pick this up on sync; a clean model sees no change. — MendixMau, second case AlvinnYeo - fix(app-restart): **the recorded pid is verified to still be an `mxcli run` before anything is stopped** (review of #304). With `--cmd` / `MXTK_APP_START_CMD`, the identity check read the override, not the process. A record left by a SIGKILL whose pid the OS had reused sent TERM/KILL to that unrelated process and its children. The pid's own command line (`/proc`, `ps`; on Windows the image name `mxcli.exe`) must now name mxcli and `run`. Otherwise the app counts as not running and the process is not touched. — MendixMau -- fix(test-scratch): **taking over a stale lock is serialised with exactly exec.sh's protocol, so test-scratch and exec.sh never both hold the shared `.exec.lock`** (review of #304). The old takeover moved the lock aside with `mv -f`. It now uses the same O_EXCL `.takeover` file and rules as exec.sh in #324: stale when its pid is dead or after 10s, re-read under it, remove only the exact stale line, put back a wrongly moved takeover file with `ln`. The port lock uses the same protocol. A new cross-script fixture, `test-shared-lock-takeover.sh`, runs 5 exec.sh + 5 test-scratch contenders on a dead holder's lock: two holders at once on the old code, never on the new. — MendixMau +- fix(test-scratch): **taking over a stale lock is serialised with exactly exec.sh's protocol, so test-scratch and exec.sh never both hold the shared `.exec.lock`** (review of #304). The old takeover moved the lock aside with `mv -f`. It now uses the same O_EXCL `.takeover` file and rules as exec.sh in #324: stale when its pid is dead or after 10s (an empty one after over a minute, `find -mmin +1`: `+0` made a fresh one stale at once), re-read under it, remove only the exact stale line, put back a wrongly moved takeover file with `ln`. It removes only its own takeover file. The port lock uses the same protocol. A new cross-script fixture, `test-shared-lock-takeover.sh`, runs 5 exec.sh + 5 test-scratch contenders on a dead holder's lock: two holders at once on the old code, never on the new. — MendixMau - learn(lint-that-actually-runs): **lint a copy of the model only after copying `.claude/lint-rules` (and `lint-config.yaml`) beside it, and compare rule counts before diffing** ([#240](https://github.com/MendixMau/mxcli-project-toolkit/issues/240), BUG-158). A snapshot in `/tmp` gave 51 findings from 8 rules, against 721 from 29 for the project: no notice, exit 0. mxcli's source also shows `lint-config.yaml` is read only from beside the `.mpr`, so an in-repo snapshot loses it as well. — MendixMau - learn(testing-shape, test-result-audit): **three test-strength rules, each with a denominator: refusals probed on the server, every undeclared 4xx a failure, one clean-database final run** ([#172](https://github.com/MendixMau/mxcli-project-toolkit/issues/172)). A field comparison of two builds against one frozen spec found the toolkit build checked permission refusals only in the client (a hidden menu item), logged unexpected 400/422 responses as information, and reached 20/20 by stitching three runs. `testing-shape.md` §7 gains three report lines (`N of N restricted pairs probed server-side`, `undeclared 4xx/5xx: 0 of M requests — each fails its step`, `one clean-DB run : N of N scenarios incl. timers, not stitched`); `test-result-audit.md` gains check 2e, which carries the reasons and the field evidence and asks for those numbers. Both baseline skills stay inside the 80,000-word baseline budget (79,997): the reasons live in the on-demand audit skill, and two redundant sentences in `testing-shape.md` §4's verdict paragraphs were folded (the empty-set corollary is stated in the Trace row, `harness-architecture.md` and `report-schema.md`). The backing inbox note is trimmed to its unpromoted "Keep" items. — MendixMau - learn(learned-detection-gaps, module-review): **two UI defects every model gate passes: a lazy-loaded combo box filter hides its preset, and a theme that pins row heights by position lets the next row cover an inserted widget** ([#299](https://github.com/MendixMau/mxcli-project-toolkit/issues/299)). Both passed check, mxbuild, lint and the scripted checks on a 127-module production app and were found only in the running app. Two register rows with the first oracle and the pre-build check (pluggable properties that change *when* data loads; grep the theme for fixed `height` on positional row classes before an insert), and two LOOK lines in `module-review.md` §4c with denominators (`N of N filters show their preset on first load`, `N of N widgets inserted into an existing container fully visible`). — jopterhorst diff --git a/project-bin/test-scratch.sh b/project-bin/test-scratch.sh index f9ae8f2f..d20192cf 100755 --- a/project-bin/test-scratch.sh +++ b/project-bin/test-scratch.sh @@ -172,31 +172,36 @@ holder_live() { # that is not its own. WHY (independent review of #304): the old takeover here moved the lock # aside with `mv -f` — check-then-act — so two runs that judged the same dead holder could both # end up holding it. A takeover file left by a holder killed mid-takeover is stale once its pid is -# dead or it is older than 10 s (an EMPTY one once it is a minute old); it is moved aside, and put +# dead or it is older than 10 s (an EMPTY one once it is over a minute old: find -mmin +1); it is moved aside, and put # back with `ln` (fails if a new one exists — never clobbers) if what was moved is not the one # judged stale. The machine-wide port lock uses the same protocol. take_over() { # $1 = lock file, $2 = the stale lock line. 0 iff it was removed (or already gone) - local _tk="$1.takeover" _t _tpid _tsince _tstale _now - if ! ( set -o noclobber; printf '%s %s\n' "$$" "$(date +%s)" > "$_tk" ) 2>/dev/null; then + local _tk="$1.takeover" _t _tpid _tsince _tstale _now _mine + _mine="$$ $(date +%s)" + if ! ( set -o noclobber; printf '%s\n' "$_mine" > "$_tk" ) 2>/dev/null; then _t="$(head -1 "$_tk" 2>/dev/null || true)"; _tpid="${_t%% *}"; _tsince="${_t#* }" case "$_tsince" in ''|*[!0-9]*) _tsince=0 ;; esac _tstale=0 if [ -n "$_t" ]; then { ! kill -0 "$_tpid" 2>/dev/null || [ $(( $(date +%s) - _tsince )) -gt 10 ]; } && _tstale=1 - elif [ -n "$(find "$_tk" -mmin +0 2>/dev/null)" ]; then - _tstale=1 # empty for a minute: its creator died between the create and the write + elif [ -n "$(find "$_tk" -mmin +1 2>/dev/null)" ]; then + # -mmin +1, not +0: GNU find counts a 0-2 s old file as "+0" (and BSD rounds up), so +0 made a + # FRESH empty takeover file — its creator between the create and the write — stale at once. + _tstale=1 # empty for over a minute: its creator died between the create and the write fi if [ "$_tstale" = 1 ]; then if mv "$_tk" "$_tk.stale.$$" 2>/dev/null; then - [ "$(head -1 "$_tk.stale.$$" 2>/dev/null || true)" = "$_t" ] || ln "$_tk.stale.$$" "$_tk" 2>/dev/null || true - rm -f "$_tk.stale.$$" + # Remove what was moved ONLY if it is what was judged stale; anything else is someone's + # live takeover file: put it back (ln never clobbers), and if that fails leave it be. + if [ "$(head -1 "$_tk.stale.$$" 2>/dev/null || true)" = "$_t" ]; then rm -f "$_tk.stale.$$" + else ln "$_tk.stale.$$" "$_tk" 2>/dev/null && rm -f "$_tk.stale.$$"; fi fi fi return 1 fi _now="$(head -1 "$1" 2>/dev/null || true)" [ "$_now" = "$2" ] && rm -f "$1" - rm -f "$_tk" + [ "$(head -1 "$_tk" 2>/dev/null || true)" = "$_mine" ] && rm -f "$_tk" # only our own takeover file return 0 } take_lock() { # $1 = lock file, $2 = what it guards (for messages). 0 = taken, 1 = live holder diff --git a/tests/wave2/test-test-scratch.sh b/tests/wave2/test-test-scratch.sh index 1850d108..4609943f 100755 --- a/tests/wave2/test-test-scratch.sh +++ b/tests/wave2/test-test-scratch.sh @@ -119,6 +119,18 @@ printf '999999 1 test-scratch.sh\n' > "$L"; printf '999998 1\n' > "$L.takeover" OUT="$( L="$L" bash -c '. "$1"; holder_live() { return 1; }; take_lock "$L" "race lock"; echo "rc=$?"' _ "$H" 2>&1 )" has "taken after clearing the dead marker" "$OUT" "rc=0" [ "$(cut -d' ' -f1 "$L" 2>/dev/null)" != 999999 ] && ok "the stale line is gone" || bad "stale line still there" +echo "-- #324 review: a FRESH empty takeover file (its creator between create and write) is not stale" +# find -mmin +0 counted a 0-2 s old file as old (GNU; BSD rounds up), so a takeover in progress was +# treated as dead and two contenders could both take over. +1 = empty for over a minute. +printf '999999 1 test-scratch.sh\n' > "$L"; : > "$L.takeover" +OUT="$( L="$L" bash -c '. "$1"; holder_live() { return 1; }; take_lock "$L" "race lock"; echo "rc=$?"' _ "$H" 2>&1 )" +has "not taken while a fresh empty takeover file exists" "$OUT" "rc=1" +[ -e "$L.takeover" ] && [ ! -s "$L.takeover" ] && ok "the fresh takeover file was left alone" || bad "the fresh takeover file was removed or replaced" +[ "$(cut -d' ' -f1 "$L" 2>/dev/null)" = 999999 ] && ok "the lock was not touched" || bad "the lock was taken over under a live takeover" +touch -t 200001010000 "$L.takeover" +OUT="$( L="$L" bash -c '. "$1"; holder_live() { return 1; }; take_lock "$L" "race lock"; echo "rc=$?"' _ "$H" 2>&1 )" +has "an empty takeover file over a minute old is stale → taken" "$OUT" "rc=0" +[ -e "$L.takeover" ] && bad "takeover file left behind" || ok "and no takeover file is left behind" kill -KILL "$LIVE" 2>/dev/null; wait "$LIVE" 2>/dev/null echo "-- #244: an after-startup left on an mxcli test injection is refused before anything is copied" From 1cb70ad33f0c59f8e926f6680d940ea0013c8eec Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 13 Oct 2026 15:48:46 +0000 Subject: [PATCH 5/5] fixture: re-copy exec.sh lock block from #324 @ 79de404 The aligned exec_lock_takeover (-mmin +1, own-file-only removal) now matches test-scratch take_over() apart from signature and parameter names (diffed). Refs #244 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01VJgWP5vEoAsNsJYqCDMGNw --- .../fixtures/mxtest-guard/exec-lock-block.sh | 26 ++++++++++++------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/tests/wave2/fixtures/mxtest-guard/exec-lock-block.sh b/tests/wave2/fixtures/mxtest-guard/exec-lock-block.sh index a6a31c34..b14b13a8 100644 --- a/tests/wave2/fixtures/mxtest-guard/exec-lock-block.sh +++ b/tests/wave2/fixtures/mxtest-guard/exec-lock-block.sh @@ -1,5 +1,5 @@ # exec-lock-block.sh — VERBATIM copy of project-bin/exec.sh's lock block (exec_lock_live .. the -# LOCK_TAKEN check) from PR #324, branch fix/exec-followups @ ed4efbb. Used by +# LOCK_TAKEN check) from PR #324, branch fix/exec-followups @ 79de404. Used by # test-shared-lock-takeover.sh until #324 is merged; that fixture prefers the real exec.sh's block # once it carries exec_lock_takeover. Do not edit by hand — re-copy. exec_lock_live() { # $1 = pid from the lock; 0 iff that process is alive and plausibly exec.sh @@ -26,30 +26,36 @@ exec_lock_release() { # the trap: remove the lock only while it is still ours # moved A's fresh lock aside, C's noclobber write landed in the gap, B saw a lock and deleted # A's: 12 contenders gave up to 3 holders at once (tests/wave2/test-exec-lock-watch.sh). # A takeover file left by a holder killed inside those few microseconds is stale once its pid is -# dead or it is older than 10 s (an EMPTY one once it is a minute old); it is moved aside, and put back with `ln` (fails if a new one -# exists — never clobbers) if what was moved is not the one judged stale. +# dead or it is older than 10 s (an EMPTY one once it is over a minute old — `find -mmin +1`; +# `+0` matched a file created a second ago, so a live contender's fresh takeover file was judged +# stale and a third contender got in: review of #324). It is moved aside, and put back with `ln` +# (never clobbers; left on disk if that fails) if what was moved is not the one judged stale. +# The holder removes the takeover file only while it still holds the line it wrote. exec_lock_takeover() { # $1 = the stale lock line; 0 iff it was removed (or already gone) - _tk="$LOCK.takeover" - if ! ( set -o noclobber; printf '%s %s\n' "$$" "$(date +%s)" > "$_tk" ) 2>/dev/null; then + _tk="$LOCK.takeover"; _mine="$$ $(date +%s)" + if ! ( set -o noclobber; printf '%s\n' "$_mine" > "$_tk" ) 2>/dev/null; then _t="$(head -1 "$_tk" 2>/dev/null || true)"; _tpid="${_t%% *}"; _tsince="${_t#* }" case "$_tsince" in ''|*[!0-9]*) _tsince=0 ;; esac _tstale=0 if [ -n "$_t" ]; then { ! kill -0 "$_tpid" 2>/dev/null || [ $(( $(date +%s) - _tsince )) -gt 10 ]; } && _tstale=1 - elif [ -n "$(find "$_tk" -mmin +0 2>/dev/null)" ]; then - _tstale=1 # empty for a minute: its creator died between the create and the write + elif [ -n "$(find "$_tk" -mmin +1 2>/dev/null)" ]; then + _tstale=1 # empty for over a minute: its creator died between the create and the write fi if [ "$_tstale" = 1 ]; then if mv "$_tk" "$_tk.stale.$$" 2>/dev/null; then - [ "$(head -1 "$_tk.stale.$$" 2>/dev/null || true)" = "$_t" ] || ln "$_tk.stale.$$" "$_tk" 2>/dev/null || true - rm -f "$_tk.stale.$$" + # Remove what was moved ONLY if it is what was judged stale; anything else is someone's + # live takeover file: put it back (ln never clobbers), and if that fails leave it be. + # Same body as test-scratch.sh's take_over() — keep the two in step (PR #322). + if [ "$(head -1 "$_tk.stale.$$" 2>/dev/null || true)" = "$_t" ]; then rm -f "$_tk.stale.$$" + else ln "$_tk.stale.$$" "$_tk" 2>/dev/null && rm -f "$_tk.stale.$$"; fi fi fi return 1 fi _now="$(head -1 "$LOCK" 2>/dev/null || true)" [ "$_now" = "$1" ] && rm -f "$LOCK" - rm -f "$_tk" + [ "$(head -1 "$_tk" 2>/dev/null || true)" = "$_mine" ] && rm -f "$_tk" # only our own takeover file return 0 } LOCK_TAKEN=0