mirror of
https://github.com/civitai/civitai.git
synced 2026-09-20 22:08:18 +08:00
Justin Maier
a94f1b70f5
* feat(dev-server): serialise unit-test runs behind the daemon The unit suite takes every core. One run is fine; several agents each starting one at the same moment is what flattens the machine, and capping the worker pool per run does not stop that. This adds the scheduler. Two calls, because a caller needs to know where it stands before it decides how to wait. `test run` returns immediately with either "started" or a position plus the command to wait on; `test wait` blocks until the run finishes and exits with its exit code. The wait polls from the CLI rather than holding a daemon request, since the daemon is single-process and a held request would stall every other agent's polling. The daemon owns the run, not the caller, which is what makes a dead agent harmless: it releases nothing because it was holding nothing. Slots are held while a run is tracked and released on child exit, never on a status field -- status is a report, not an observation, and cannot see a grandchild that outlived a kill. Three deadlines close the rest: a queued run whose caller stopped polling is dropped, a run that overruns its ceiling is killed, and a kill that produces no exit frees the slot anyway after a grace period. Concurrency defaults to 1 and is configurable. 0 is legal and means paused, with the pause reported to callers rather than left to be inferred from a position that never moves. The 404 on an unknown run id is load-bearing rather than incidental: it is how a waiter learns the daemon was restarted and its run is gone, instead of polling for a result nobody will produce. * fix(dev-server): close four holes an adversarial review found in the queue 1. `test wait` exited 0 for a cancelled or timed-out run whose child happened to exit 0 -- the window between a kill being issued and it landing. This is the worst possible failure for a command meant to substitute for the suite in a verification chain, because it reports a green run that never finished. Only a completed run that itself exited 0 is a pass now, and the decision lives in one exported function so it can be tested rather than inferred. 2. A malformed TEST_CONCURRENCY killed the daemon at import. The queue is built at module scope and the constructor throws, so a typo in an optional test setting stopped the daemon binding at all -- taking every agent's dev server, session list and worktree tooling with it. It now falls back to 1 and says so, like every other setting in that file. 3. A runner reporting its exit synchronously lost the event, because the listener was attached after the runner returned. The finished run held the only slot until the 30-minute ceiling while everything behind it was abandoned rather than run, and it then settled as `timeout` with an error string that was false. The exit path is now built before the runner is called. 4. The line whose removal produces exactly the permanent wedge this feature exists to prevent had no coverage: 15/15 passed with it deleted. The test sweeps inside the grace the way the daemon's own 5s timer does, so a reset deadline now fails as `expected [] to deeply equal ['<id>']`. Also: children get their own process group on POSIX, without which killing by negative pid names no group and silently leaves vitest running while its slot is handed on -- serialisation quietly becoming concurrency 2 under exactly the load this is for. Daemon shutdown kills synchronously so the child cannot outlive it. Every fix has a mutation control: reverting each one fails on a named value. * fix(dev-server): three more from the second review pass The identity half of the exit guard was untested -- 21/21 passed with it removed. It only fires when a late exit arrives after a force-release, and no test delivered one. It was defended in practice only by the exit-code rule catching the consequence, which is two fixes covering for each other rather than either being pinned. Now a test force-releases a slot and then delivers the exit. `exitCode || 1` passed a signal death straight through as -1, which a shell sees as 255 -- and `detached` + SIGKILL means that is now the normal shape of every cancel and timeout on POSIX. Real failing codes still pass through; anything that is not a positive integer becomes 1. The exit-code table gains a row with a distinctive code, since every row it had was one whose answer is 1 anyway, so it could not tell a passed-through code from a hardcoded one. A child that dies by signal reports no code at all. That is an OOM kill or an outside hand, not a verdict on the tests, so it settles as `error` rather than claiming a test result nothing produced. `execFileSync` on shutdown had no timeout. It runs on the daemon's event loop and is also reached from the SIGINT handler, so a taskkill that blocked would leave the daemon unkillable by signal. A kill we cannot complete is better abandoned -- the sweep frees the slot regardless. * fix(dev-server): keep the verdict when a runner reports then throws The catch around startRun neither checked nor set the settled flag, so a runner that reported an exit and then threw had its real result overwritten by the noise that followed it -- completed/0 became error/null. Degrades safe and the shipped runner cannot produce it, but a verdict that exists should not be discarded. * feat(dev-server): route test:unit:run through the queue when opted in Replaces the hook approach. A PreToolUse hook has to decide from the command text whether a run is happening, and three adversarial passes showed that cannot be done: it ended with 20 known bypasses and 6 false denials, including refusing a commit whose message merely mentioned the suite. Inside the script there is nothing to parse -- whatever shell, wrapper, quoting or directory reaches it gets the queue. It routes rather than refuses, which is the difference that makes it work: no second command to learn, nothing to wrap around, and an agent that never read the guidance still gets queued instead of an error it will work around. Off unless CIVITAI_TEST_QUEUE is set, and off in CI regardless, so this is a no-op for everyone who does not run the daemon -- the same vitest invocation as before. A file-scoped run stays direct: queueing a two-second check behind a nine-minute suite would break the fast loop and push callers toward batching more into each run, which is the opposite of the point. The queue being unreachable falls back to running directly. Nobody should be unable to run tests because a daemon is down.