_slot_ports_free consumed _slot_offset_ports via process substitution
(done < <(...)), so a die on an out-of-range/non-numeric slot exited only
the subshell — the loop read zero ports, any_held stayed 0, and the
function returned 0 ("all free"), silently defeating the port-conflict
guard for a bad slot. Capture into a variable with || die so the failure
propagates to the caller. Real-surface bats prove a bad slot now fails
loudly while valid-slot free/held behavior is unchanged.
A --keep'd isolated stack whose owning process had exited (but whose
containers kept running) was classified 'live' forever and never reaped,
leaking Docker stacks indefinitely. Introduce a start-time-verified
_owner_liveness probe and a new 'kept' state, an ISOLATE_KEEP_TTL (4h,
SHOWCASE_ISOLATE_KEEP_TTL-overridable) that flips an over-age kept slot
to 'stale' so the sweep reclaims it, a com.copilotkit.showcase.isolate
self-id label stamped by apply_isolation, a 'slots --reapable' filter,
and a macOS lsof COMMAND-truncation fix in the own-project port filter.
Real-surface bats cover the liveness false-positive and TTL reaping.
The python rewriter in apply_isolation previously interpolated $slug
directly into the inline python source via bash. A slug containing a
single quote would break the python literal. Internal-tool risk only
(slug is developer-typed), but cheap to harden.
Pass slug via SHOWCASE_ISO_SLUG env var and read os.environ.get(...)
inside the python heredoc. Defense-in-depth; no behavior change for
valid slugs.
The persistent stack's docker-compose.local.yml hardcodes LOCAL_SERVICES_JSON
to the langgraph-python sample for fast N=1 local demos. When --isolate
spawns an iso1 stack with a different slug (e.g. ms-agent-python), the
iso1 harness container inherited that hardcoded value, causing
discovery.railway-services.local-injection to enumerate the wrong service
(showcase-langgraph-python instead of showcase-<requested-slug>). The iso1
probe then targeted the wrong container, broke red-green verification, and
left D5 cells unwritten.
Inject a per-slug LOCAL_SERVICES_JSON override into the iso1 compose
generator so iso1 always probes the slug passed via --isolate.
Cross-session review fixes for the --isolate machinery (one concern:
source + test + docs).
1) Reaper reserved-name guard (critical): _reap_isolate_slot trusted
slot records — a record naming 'showcase' (corrupt, or written by an
older CLI version before apply_isolation reserved the name) passes
the charset regex, so the reap ran `docker compose -p showcase down
--remove-orphans --volumes` against the LIVE default stack,
destroying the PocketBase named volume. The reserved name now gets
the same treatment as the path-traversal guard: warn (naming the
record and why it is dangerous) and leave the slot intact for manual
inspection — no compose-down, no state removal.
Call-site enumeration: _reap_isolate_slot's sole caller is
_sweep_isolate_slots, at 3 sites (dead-PID reap, project-recorded/
no-owner reap, age-fallback reap), all passing
"$slot_entry" "$slot_proj" — all three flow through the new guard
identically.
Red-green: the new bats test ("a slot whose project record reads the
RESERVED 'showcase' is left intact...") was run against the UNFIXED
code first and FAILED — the sweep logged "Attempting to reclaim
stale slot 0 (project showcase has no live containers and no
recorded owner)" and reaped the slot. It passes with the guard.
2) .iso-bak restore race: two concurrent runs can both see a stale
backup; the loser's mv is the FINAL command of its `[ -f ] && mv`
AND-list, so its failure trips set -e and kills the CLI pre-claim
with a raw error. Both mv's now carry `2>/dev/null || true` — the
survivor's restore wins, the loser proceeds with restored originals.
3) Keep-test absence regexes greped only the `--project-name <name>
down` spelling; the reaper's own downs use `-p <name> down`, so a
keep-branch regression via the -p form passed undetected. Both keep
absence assertions now match `(--project-name|-p) <name> down`.
Mutation-verified: a temporary -p-form compose-down added to the
keep branch made BOTH broadened tests FAIL; reverted, suite green.
(All other absence assertions use the word-matched generic
`compose ... down` regex, which already covers both spellings.)
4) RUNBOOK.md/DEBUGGING.md contradicted shipped code: the manual
teardown was quoted without --volumes plus notes claiming
`down --remove-orphans` leaves named volumes (the shipped survival
notice and every teardown path include --volumes), and the name rule
was documented as `[a-z0-9_-]+` (actual: starts with [a-z0-9], then
[a-z0-9_-], uppercase normalized with a warn, 'showcase' reserved).
Both updated to the shipped semantics; the now-redundant separate
`down --volumes` snippets removed.
Verification: full `bats showcase/scripts/__tests__/` green (60 tests);
shellcheck on _common.sh shows no new warnings vs baseline
(pre-existing SC2034/SC2115 only, line-shifted).
- ISOLATE_KEEP promoted to a global so --keep survives cmd_test return
into the trap scope
- early-die and default-stack protection: failed --isolate setup no
longer tears down the default stack; half-initialized state is
cleaned up on the way out
- liveness/PID/age reaping signals with a sweep lock: heartbeat
updates, own-pid lock release, tombstones, and a claim-then-verify
duplicate-name guard close slot-registry races (TOCTOU, lock
takeover, reap order)
- teardown robustness: --volumes on every compose down, failed-down
runs preserve state for diagnosis, reap remnants get a compose-down,
path-traversal guard, uniform rm guards under set -e
- name validation: --isolate names must start with a lowercase letter
or digit; reserved name 'showcase' rejected (it aliases the default
stack)
- fail-loud warning before pre-down of an existing stack; help text
updated
Result of an 8-round, 7-agent code-review loop with red-green
verified fixes.
Previously the EXIT-trap restore_isolation always tore down the isolated
stack, ignoring --keep. Now restore_isolation reads a keep flag (set in
cmd-test.sh when --keep is parsed): when kept it skips compose down, the run-dir
removal, and the slot release, and instead prints a survival notice with the
project, slot, the three offset host ports, and the exact manual teardown
command. The kept stack's live containers keep its slot from being reaped.
Persist the compose project name into each claimed slot dir, and at claim time
reap any slot whose recorded project has no live containers (queried via
docker ps --filter label=com.docker.compose.project). This correctly leaves a
--keep'd stack's slot alone since its containers are still up. The existing
PID/age heuristics remain as a fallback for slots predating the project file.
Move the --isolate slot registry and per-run scratch dir off /tmp (wiped on
reboot, world-writable) to $XDG_STATE_HOME/copilotkit/showcase (slots/ and
runs/<name>). The run dir is now keyed by the finalized project name instead
of the PID so a kept run is locatable for manual teardown. Adds a bats suite
covering the new state-base helper and run-dir location.
- scripts/cli/_common.sh apply_isolation rewrites compose-file relative
paths to absolute (build/context/dockerfile/volumes/env_file), enforces
the docker compose [a-z0-9_-] project-name rule (normalize-with-warn or
hard fail), and exports SHOWCASE_COMPOSE_FILE / SHOWCASE_INFRA_PORT_OFFSET
plus offset host URLs (AIMOCK_URL_LOCAL / DASHBOARD_URL_LOCAL /
POCKETBASE_URL_LOCAL) so the TS harness CLI talks to THIS project's
aimock instead of the default :4010
- harness/src/cli/{aimock-rebuild,config,doctor,lifecycle}.ts honor the
new env vars; lifecycle picks up the offset infra port for health
probes so concurrent stacks no longer report each other's services as
healthy
- Commit a generated harness/package-lock.json (new file) so npm ci
resolves deterministically
The COMPOSE_CMD in apply_isolation was missing --project-name, so Docker
Compose would infer the project name from the directory and collide with
the base showcase stack (and other isolated runs). Adding --project-name
ensures containers, networks, and volumes are fully scoped to the
isolation slot.
apply_isolation previously mutated docker-compose.local.yml and
local-ports.json in-place with .iso-bak backups. If the process crashed
the originals stayed corrupted with +200 port offsets, breaking all
subsequent showcase commands.
Now writes modified copies to a temp directory and overrides
COMPOSE_FILE/PORTS_FILE shell variables so downstream code reads from
the overlay. Originals are never touched. restore_isolation just removes
the temp dir.
Also replaces hardcoded +200 port offset with atomic mkdir-based slot
allocation. Two parallel --isolate runs now get different port ranges
(slot 0 = +200, slot 1 = +400, etc.) instead of colliding on the same
ports. Container names include the slot number for collision-free Docker
naming. Stale slots from crashed runs are reclaimed via PID liveness
checks and a 2-hour age fallback.
TS harness files (config.ts, lifecycle.ts, doctor.ts) honor
LOCAL_PORTS_FILE env var so they read offset ports from the temp overlay.
When passed to `showcase test`, creates an isolated Docker Compose
project with offset ports (+200) and renamed containers, allowing
multiple agents/sessions to run showcase tests simultaneously without
container conflicts.
Usage:
showcase test agno --d5 --isolate # auto-names isolate-<PID>
showcase test agno --d5 --isolate d5verify # explicit name
Ran Shem Tov