refactor(eve): share local server supervision with eve acp - #1473
Draft
chadhietala wants to merge 1 commit into
Draft
refactor(eve): share local server supervision with eve acp#1473chadhietala wants to merge 1 commit into
chadhietala wants to merge 1 commit into
Conversation
#1347's `eve acp` has to supervise a local development server the way `eve dev` already does, and forked the code rather than sharing it: the idempotent `closeServer` latch is byte-identical in both commands, and each carries its own `loadStartHost` dynamic import of `createDevelopmentServer`. Both commands stop the server from two directions — a shutdown signal and their own `finally` — so both need that idempotence, and nothing in `cli/dev/` owned it. A new `cli/dev/supervised-server.ts` owns both: `createServerCloseLatch`, which takes a getter so each command keeps its existing lifecycle ordering (`eve dev` installs the latch before its shutdown signal, `eve acp` after), and `loadStartDevelopmentHost`. `runAcp`'s input was typed as the ACP server's own parameters, which admit a headers *thunk*, so it had to narrow `headers` before use — but both call sites pass a plain record from `resolveDevelopmentUrlTarget`, making that branch unreachable. Typing the input as what the command actually passes removes it, and the hand-rolled authorization-header scan becomes a call to the exported `hasDevelopmentAuthorizationHeader` that `verified-remote-client.ts` and `client-options.ts` already use. Signed-off-by: Chad Hietala <chadhietala@gmail.com>
chadhietala
added
the
simplify-devbox
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
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
1 participant
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.
Found
cli/acp/command.tscopiescli/run.ts's idempotentcloseServerlatch byte for byte, and each file has its ownloadStartHostdynamic import ofcreateDevelopmentServer— nothing incli/dev/owned either.runAcptypes its input as the ACP server's own parameters, which admit a headers thunk, so it narrowstypeof headers === "function"— a branch neither call site can reach, since both pass a plain record fromresolveDevelopmentUrlTarget.hasDevelopmentAuthorizationHeaderalready used byverified-remote-client.tsandclient-options.ts.Did
New
cli/dev/supervised-server.tsownscreateServerCloseLatch(getter-based, so each command keeps its own lifecycle ordering) andloadStartDevelopmentHost; typedrunAcp's input as what the command passes and called the shared predicate. −34/+22 across 3 files.Validated
1016 CLI unit tests pass, including
run.test.ts's ACP cases (remote target without a local server,startHostargs,closecalled once) andlocal-server-process.test.ts. No test changed.tsc --noEmit, lint, fmt,guard:invariantsclean.