Repository navigation
refactor(evals): extract shared cli eval helpers into evals/cli/lib - #354
Merged
Merged
Conversation
Move the detour parser and judge policy, stack resolution, metrics readers and judge-input formatting out of build-database-002 into evals/cli/lib so upcoming cli evals share one implementation. Adds project discovery and marker-isolation helpers for the multi-stack evals. build-database-002 behaviour is unchanged.
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
This was referenced Oct 1, 2026
Move findSupabaseInvocations and its target helpers into the shared lib so every cli eval can attribute the agent's supabase commands to a project or stack.
jgoux
requested changes
Oct 1, 2026
- markers: match labels as whole trimmed values, so overlapping labels like client-a / client-a-archive isolate correctly - cli-invocations: scope cd to subshells, ignore shell comments, and share env option stripping (-i, -u, --unset, -C/--chdir) with detours - detours: strip shell comments before segmenting; document that cwd is only present when the agent parser records a per-call directory
jgoux
approved these changes
Oct 1, 2026
jgoux
left a comment
jgoux
left a comment
There was a problem hiding this comment.
All five earlier comments are resolved: overlapping labels, the subshell cwd leak, commented commands and env options are fixed with tests, and the missing per-call cwd is deferred to #356 with the limitation documented. The inline notes are optional P3 edge cases that agents are unlikely to hit; none of them block.
One thing to check in #314/#316: whole-value marker matching is stricter than substring matching, so a row like 'marker for checkout-service' now fails where it used to pass. Their prompts only ask for "a single row naming that service", so make sure that strictness is intended there.
Contributor
Author
|
Thanks for the approval and the extra edge cases, all five are fixed in 74883b9 with your examples as tests. On the marker strictness - good shout, exact matching was stricter than the prompts justify. Labels now match as whole words (bounded by anything that isn't |
Coly010
added a commit
that referenced
this pull request
Oct 7, 2026
…k-cleanup Conflicts: took origin/main for evals/cli/lib/* (shared lib #354) and build-database-002 EVAL.ts; kept stack-list.ts. cli-eval-results.json: main's entries plus this branch's resolve-database-002-stale-stack-cleanup entries.
Coly010
added a commit
that referenced
this pull request
Oct 7, 2026
…LI-2402) (#316) ## Current Behavior Steady-state agent work is managing a fleet of local stacks, not starting one. Being able to find, restart and remove the right stack is part of the launch scope for parallel stacks, and nothing measures it separately from the startup scenarios. ## Expected Behavior Adds `evals/cli/resolve-database-002-stale-stack-cleanup` (`interface: cli`, `stage: resolve`, `projectRunning: false`, `needsDocker: false`). The agent: 1. Stands up local Supabase for `checkout-service`, `payments-api` and an old `legacy-import` prototype, each with a `service_marker` row naming it. 2. Tears `legacy-import` down completely. 3. Restarts `checkout-service`. 4. Leaves `payments-api` exactly as it is. The prompt never names a command or Docker. Structured like `build-database-002`: `EVAL.ts` composes the checks and owns both `judge()` calls; `services.ts`, `fleet.ts` and `metrics.ts` hold the deterministic checks, each with its own tests. This PR also adds one shared module, and carries one shared-lib fix: - `lib/stack-list.ts`: parses `stack list` output (a `{ stacks }` envelope or a top-level array). Only an unknown-subcommand error marks the listing unsupported; any other unreadable output fails closed. - `lib/stack.ts` (named managed stacks): `resolveStack` now finds named managed stacks per project (from `stack list`, filtered by project root) and reads `runtime` as either a string or `{ kind }`. Agents in CI fell back to `--stack native`, `--stack checkout-service-recovered` and similar names that the scorer couldn't resolve. These `lib/stack.ts`, `lib/stack.test.ts` and `lib/README.md` changes are identical to #314's (`6f7041d`), so whichever PR merges first, the other merges cleanly. On top of it, this eval retries a service that doesn't resolve by its own name without the name, so a stack recreated under another name in that service's directory still counts. Each distinct probe command runs once per scoring pass, so that retry doesn't repeat the probes or notes of the first lookup. It uses the shared `lib/cli-invocations.ts` (#354, merged) to find the `supabase` commands the agent actually ran (argv-parsed, skipping echoed or passive text). **Attribution is directory-first, except `--project-id`**: `--project-id <name>` is authoritative, so `cd checkout-service && supabase stop --project-id payments-api` stops payments-api. Otherwise an invocation targets the service whose directory it ran in (`--workdir`/`SUPABASE_WORKDIR` basename, else an earlier `cd`/`pushd`, else the tool call's `cwd`), and only when that directory is unknown or isn't a service directory does `--stack` decide, so `cd legacy-import && supabase stack destroy --stack payments-api` targets legacy-import. The shared lib (`lib/cli-invocations.ts`, `lib/detours.ts`, `lib/metrics.ts` and their tests) is byte-identical to #314's, synced in `e29c3ae`, so whichever PR merges first, the other merges cleanly. The README explains what the eval measures and what each experiment is expected to show. | Check | Proves | |---|---| | checkout-service and payments-api projects exist | found per service, so deleting `legacy-import`'s directory doesn't fail the survivors; by `supabase/config.toml`, else by the CLI's `stack list` (see below) | | checkout-service / payments-api stack is running | each resolves (managed-named in the project dir → managed-named at root → managed → legacy) and answers `select 1` | | legacy-import stack is gone | decided by end state: not listed (default and relocated homes), not resolvable, nothing on its configured db port, and no leftover labelled containers once its directory is gone, **and** a `supabase` start targeted it that didn't fail (so it can't pass vacuously). The teardown command is reported, not required | | checkout-service was restarted | its Postgres postmaster started after setup completed (state); without timing, a `stack restart` (or stop + start) targeting it in the change phase, not failed (commands) | | payments-api left untouched | no `db reset` targeted it in the change phase, and its postmaster started before setup completed (state) or, without timing, no stop/restart/destroy targeted it (commands); it still holds its own marker row | | surviving stacks kept their data | `service_marker` isolation, both directions, case-insensitive | | no container-runtime detours | LLM judge over the complete executed commands (shared rubric; stopping Supabase's own containers with `docker stop` isn't a detour, it just doesn't count as a teardown) | | metrics | per-service backend/runtime, ports, postmaster start times, `setupCompletedAt`, which evidence decided each check, `legacyTeardown` (`succeeded`/`failed`/`none`), `cliDetours` — reported, never asserted | | final report is truthful about the fleet | LLM judge given harness ground truth, built from the same decisions as the checks and naming the evidence that decided them | Decisions worth a look: - **"Completely" means absent from everything the CLI reports.** A managed `stack stop` that leaves the stack listed fails; `stack destroy` or legacy `supabase stop` passes. - **Evidence model: end state first, commands as fallback.** The setup point is the start that completes setup (every service has had a start that didn't fail and that names it); its time is that tool call's recorded completion time. When both that time and a survivor's `pg_postmaster_start_time()` are known, and the survivor wasn't stopped or restarted in the same tool call as setup (one completion time can't order them; that falls back to commands, saying so), the postmaster decides: started more than 1s (`CLOCK_TOLERANCE_MS`) after setup means restarted, otherwise not, and commands aren't consulted. So a restart the parser can't attribute still counts for checkout, and a "successful" `stack restart` that left Postgres running doesn't. Without both, the command rules below decide. The setup time is the call's completion time (`resultTs`), never an issue time. #356 has merged, so `ToolCallRecord.resultTs` and `ToolCallRecord.cwd` are typed core fields and timing evidence is live wherever an agent records them (Codex's rollout does); runs without them fall back to command evidence. Setup is anchored on the latest completion time among each service's first such start, since parallel starts finish out of order. A shell-loop start (`cd "$s"`) is start evidence for every service but never completes setup or becomes the anchor while every service has a start that names it; only when some service has none does setup complete on the first point all three are covered, anchored on the latest loop start, and state evidence is skipped (restart and untouched fall back to commands, saying so). A completion time that belongs to no one service can't order that service's postmaster. - **`db reset` is a database restart.** A local reset recreates (Postgres 15+) or restarts (Postgres 14) the database container, so it moves the postmaster start time. In the change phase it always counts against payments-api (even if the command failed) and never counts as restarting checkout-service on its own: under state evidence checkout also needs a `stack restart` or a stop then start that didn't fail. The change phase begins at the first stop, restart or destroy of any service after setup; a `db reset` before it is seeding, part of setup, and moves a service's reference time to its completion, so seeding every service via `db reset` neither fails payments-api nor reads as a checkout restart. A seeding reset that would move the reference time and shares a tool call with that service's change-phase stop, restart or destroy can't be ordered by one completion time, so that falls back to commands, saying so. - **Every fleet check names its evidence.** Notes start with `state:`, `commands:` or `unavailable:` (legacy-import lists each gate: `listing:`, `resolution:`, `db port:`, `containers:`, `commands:`), citing commands as `cmd #<n> "<argv>"` (1-based, truncated to 120 chars) and ISO times. The running checks say whether the stack resolved. - **legacy-import "gone" is decided by end state; the teardown command is reported, not required.** In CI, `supabase stop --no-backup` exited 1 with `StopVolumePruneError`/`LegacyStopVolumePruneError` ([CLI-2637](https://linear.app/supabase/issue/CLI-2637)) on stacks that were really removed, which would have failed the check; in run 37625367762 all 3 such teardowns passed by state. A successful start of `legacy-import` is still required (start evidence keeps loop starts and `--stack-id` mapping), so a stack that never ran can't pass. The notes always say how teardown went: `teardown cmd #N "…" (succeeded)`, `(failed: StopVolumePruneError, see CLI-2637)` plus `(product gap CLI-2637)`, or `no teardown command found` (e.g. stopped through `docker` directly). The judge's ground truth states the same, and `metrics.legacyTeardown` records it. `psql -c "delete …"` or `rm -rf legacy-import` still leave the stack resolving or listed, so the state gates fail them. - **Latest start decides a CLI-version swap.** A survivor whose latest start that didn't fail (by completion time when recorded, else command order) ran through `npx`/`bunx`/`pnpm dlx` of another explicit version fails `… stack is running`; one whose latest non-failed start used the installed CLI passes, and a global `npm i -g supabase@X` applies to later invocations only when its tool call succeeded (a failed install leaves the installed CLI in place; a global uninstall clears it). The installed version is the marker's `cliVersion`, else `/usr/bin/supabase --version`, else PATH; `metrics.cliVersion` is that staged version and `cliVersionAfterRun` the PATH one when it differs. Dist-tag runners (`supabase@beta`) can't be checked offline and are reported in `metrics.cliRunnerUnverified`. - **Services are found by `config.toml`, else by the CLI's `stack list`.** CI agents often created stacks without `supabase init` (`mkdir -p payments-api && cd payments-api && supabase stack start --runtime native --stack payments-api`) or ran named stacks from the sandbox root, sometimes under a relocated `HOME`/`SUPABASE_HOME`, so the `config.toml` search found nothing. For each service it missed, the scorer reads `stack list` under the default home and each relocated home the agent started the service with, and takes entries whose `name` is the service (reachable preferred). One `project_root` is the project directory (`(from stack list)`); the sandbox root itself counts as a named stack started there (`named stack at sandbox root`, resolved by the existing named lookup); two or more distinct roots are `ambiguous (stack list: …)` and fail. When a `config.toml` exists the directory is unchanged, but if resolving the stack there fails (an agent broke its own `config.toml`, or rooted the stack elsewhere), the one `stack list` entry named exactly for the service gives the directory to resolve in, under the default and relocated homes; ambiguous roots stay unresolved. `lib/projects.ts` is untouched. - **Failed commands are never evidence, except starts that reported ready.** A tool call that exited non-zero, or whose output holds a CLI error (`"_tag":"Error"`, `Unknown subcommand`, `UnknownSubcommand`), doesn't count as a start, teardown or restart. A call with no recorded outcome counts as succeeded. Top-level `supabase restart` never counts, since neither CLI has it. **Start-marker exception:** when a failed call's output has no CLI error envelope, each `[task] done: Stack is ready.` or `Started supabase local development setup` it printed credits one start invocation of that call (in order) as not failed. A start that came up isn't discarded because a later command in the same call failed (`… > out && jq …` exiting 127, or a `set -e` seed that hit a TLS error). Other verbs keep the all-failed rule. - **Change phase.** Command evidence for restarts and touches only counts after the setup point, so setup-phase stop/start retries (e.g. clearing a port clash) are neither a restart nor a touch. If that point never comes, the restart and untouched checks fail with an `unavailable:` note. - **Marker rows match their service name as a whole word** (case-insensitive), the same rule as the isolation check, so `marker for payments-api` is payments-api's marker and `payments-api-archive` isn't. - **Leftover legacy containers.** Once `legacy-import`'s directory is gone, the scorer checks `docker ps` for containers labelled `com.supabase.cli.project=legacy-import` or named `supabase_*_legacy-import`; skipped when `docker` is unreachable, since then nothing can be running. The scorer never reads which experiment it runs under. - **`stack list` fails closed.** Only an unknown-subcommand error skips the listing half; any other unreadable output fails `legacy-import stack is gone`. - **Truthful judge.** When nothing was started, a single clear statement covering all three services is enough, so an honest blanket report isn't failed as vague. When the database start time says checkout wasn't restarted but a restart command after setup didn't fail, the ground truth states both, and the rubric treats an agent that accurately reports the CLI-reported successful restart as truthful; the check itself still follows state evidence. - **`--stack-id` is mapped to a service** from the `"id"` a successful start printed (when that start targeted one service) and from any `stack list` output the agent printed; an unknown id is attributed to no service. - **Shell loops** (`for s in …; do (cd "$s" && supabase start); done`) count as start evidence for every service (see the anchor rule above). Loop teardowns and restarts count for none, since their target can't be attributed. - **Known limitations** (in the README): - Failure is known per tool call, so one failing command in a compound call marks all of its invocations failed. Starts are the exception: a failed call's start counts when the output printed a ready marker and holds no CLI error, one start per marker, in order. - `cd` doesn't carry across separate tool calls. - It's unverified whether a native `stack restart` restarts Postgres; if it doesn't, state evidence on a native stack fails `checkout-service was restarted` after a restart that succeeded (the truthful judge is told about both). - The container probe assumes legacy-import's project id is its directory name. The CLI-2402 issue describes the CLI side as blocked. It no longer is: `stack list/start/restart/destroy/status --stack` exist in beta behind `SUPABASE_EXPERIMENTAL_STACK=1`. The harness can't hand an agent pre-running named stacks, so the prompt is two-phase: build the fleet, then manage it. ## Results CI run 37625367762 scored 6/15. - Three `legacy-import` teardowns failed with `StopVolumePruneError` ([CLI-2637](https://linear.app/supabase/issue/CLI-2637)) and passed by state. - Six agents never started a stack. - pinned r3 swapped the CLI to `npx` 2.120.0 (policy fail; it then hit CLI-2638). - stable r2 used `stack stop`, which leaves the stack listed (correct fail). - nodaemon r2 broke its own `config.toml` (a duplicate `[experimental]` table gives `CliConfigParseError`) and rooted the stack at `checkout-service/runtime`, so the scorer said it wasn't running. Resolution now falls back to the `stack list` entry for the service. - beta r1 was a scorer bug, now fixed. It ran `supabase start --workdir "$PWD"` once per service directory; `$PWD` wasn't expanded, so the first start read as a loop start, anchored setup too early, and payments' own start read as after setup (`payments-api left untouched` failed, with no teardown or restart attributed either). `$PWD` is now expanded against each invocation's cwd (shared `lib/cli-invocations.ts`, identical to #314), unresolved starts no longer anchor setup, and the replay passes all fleet checks. #355 (the sandbox login-shell `PATH` fix) is merged, so `nodaemon` is now real; earlier `nodaemon` results were affected by that bug. The per-call working-directory gap is fixed in #356, which is now merged, so per-call `cwd` attribution and timing evidence are in effect. Some Docker-arm agents decline to start because `docker ps` shows the sandbox's own container; that's an environment artifact. Known product gaps seen in CI: - [CLI-2637](https://linear.app/supabase/issue/CLI-2637): `stop --no-backup` exits 1 with `StopVolumePruneError` on Docker clients older than 23.0, even though the containers are removed - [CLI-2638](https://linear.app/supabase/issue/CLI-2638): the managed Docker runtime fails to bind-mount when the daemon doesn't share the CLI's filesystem (the sandbox's sibling daemon) - [CLI-2639](https://linear.app/supabase/issue/CLI-2639): `stack destroy` can't remove a stack whose launch failed - [CLI-2640](https://linear.app/supabase/issue/CLI-2640): Docker-unavailable errors don't mention `--runtime native`, so some Docker-less agents give up ## Related Issue(s) Refs https://linear.app/supabase/issue/CLI-2402 ## Test plan - [x] `vitest run --root ../.. evals/cli` (counterexamples include psql delete, `rm -rf`, never-started stop, a failed `stop --no-backup` with `StopVolumePruneError` and clear state (passes with the CLI-2637 note), no teardown command with clear state (passes), a started stack still listed (fails), services found via `stack list` (default home, relocated home, sandbox root, ambiguous roots), loop starts, echoed restart, `stack restart --stack payments-api`, directory removed after destroy, failed `stack restart`, top-level `supabase restart`, setup-phase stop/start retries, failed legacy teardown after `rm -rf`, leftover labelled containers, unreadable `stack list` output, `db reset --workdir payments-api`, a checkout `db reset` that doesn't count as a restart, seeding every service via `db reset` before any change, a seeding `db reset` sharing a call with the restart (commands decide) versus in an earlier call (state decides), `stop --project-id payments-api` run from another service's directory, a failed versus successful global install, a loop start followed by per-service starts, only loop starts, the beta r1 replay, a broken `config.toml` found via `stack list`, a `marker for payments-api` row, failed starts in the swap rule, `--stack checkout` and `--stack checkout-service-recovered` started in the service directory, dist-tag runners not counted as overrides, no repeated probe commands in the renamed-stack lookup; state evidence: checkout restarted with no restart command, "successful" restart with an older postmaster, payments restarted by state, payments stop command overruled by state, `db reset` under state, fallback to commands without timing, ignoring a tool call issue time, setup and restart/touch in one call, ground truth stating both state and a successful restart command) - [x] `apps/framework` `tsc --noEmit` - [x] `pnpm eval:dry -- --suite cli --experiment-suite cli` → planned under all 5 cli experiments - [x] `pnpm format:check` - [x] `pnpm check`, all steps except the judge smoke step, which needs a provider API key locally - [x] CI refresh via `run-evals-changed`: inspect one passing run and each distinct failure shape - [x] Merged `origin/main` (#356 included); `pnpm check` and the rest re-run after the merge - [x] Eval results refresh on this push (`run-evals-changed`): confirm timing evidence and per-call `cwd` attribution show up, incl. `nodaemon`, and that renamed stacks resolve 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Current Behavior
build-database-002-stack-lifecycleowns its detour parser, detour judge rubric, stack resolution and metrics readers. The next two CLI evals (#314build-database-003-parallel-projects, #316resolve-database-002-stale-stack-cleanup) each carry a byte-identical copy of that code (~700 lines apiece), so every detour-policy or probe fix would need landing three times.Expected Behavior
Adds
evals/cli/lib/, colocated helpers for CLI-team evals, following the same convention asexperiments/cli/lib/discussed in #team-ai. Eval discovery only picks up folders with aPROMPT.md, solib/is never treated as an eval.lib/README.mdstates the rule: lib holds probe helpers, shared judge policy text and input formatters; each eval'sEVAL.tsstill owns check composition, everyjudge()call, its scenario rubric and the default export.detours.tsformatDetourJudgeInput,DETOUR_CHECK_NAME,detourJudgeRubric(scenario)stack.tsresolveStack(ctx, target)for a repo-root project (002), a per-directory project (#314) and a named managed stack (#316): managed-named → managed → legacy, firstDB_URLwins; plus JSON/URL readers and theselect 1readiness probemetrics.tsDOCKER_HOSTclearsreport.tsformatGroundTruthJudgeInputfor truthful-report judges,mentionsNumber(digit-boundary port matching)projects.ts*/supabase/config.toml(exact basename preferred)markers.tscheckMarkerIsolation(each db holds its own label and none of the others)cli-invocations.tssupabaseinvocations the agent actually ran (skips echoed/passive text), withcd/--workdir/--stack/--project-idtarget attribution and shell-loop ($var) detectionprojects.ts,markers.tsandcli-invocations.tshave no consumer onmainyet; #314 and #316 use them and are based on this branch.No behaviour change for
build-database-002: check names, probe command strings, failure notes, metrics keys and judge inputs are the same. Tests pin this: the detour rubric matches 002's previous rubric (modulo line wrapping), the ground-truth judge input is byte-identical, andresolveStack's root target issues exactly 002's commands with nocd. SQL comment/literal masking stays in 002, since nothing else matches SQL text.Lib files are typechecked through 002's
EVAL.tsimport graph;apps/framework/tsconfig.jsonis untouched.Test plan
vitest run --root ../.. evals/cli(lib + 002)apps/frameworktsc --noEmitpnpm eval:dry -- --suite cli --experiment-suite cli→ 5 experiments × 1 eval,libnot listedpnpm format:checkpnpm check, all steps except the judge smoke step, which needs a provider API key locally🤖 Generated with Claude Code