mirror of
https://github.com/vercel-labs/portless.git
synced 2026-09-14 16:37:59 +08:00
Railly Hugo
93acac4e1e
* fix(cli): inject framework flags through package-manager script indirection Zero-arg mode and explicit delegation run dev servers via <pm> run <script>, so the framework command lives inside package.json where injectFrameworkFlags (argv-only) cannot see it. Vite then ignores PORT, listens on 5173, and the assigned route returns 502 (#285). Resolve the script from package.json, compute the flags the underlying framework needs, and forward them through the package manager: after -- for npm; bun, pnpm, and yarn forward trailing arguments directly. An existing --port/--host in the script or in user-supplied trailing args suppresses injection. Covers portless (zero-arg), portless run, portless run <pm> run <script>, and portless <name> <pm> run <script> for npm, pnpm, yarn, and bun. Fixes #285 Co-Authored-By: EfeDurmaz16 <efebarandurmaz05@gmail.com> * fix(cli): guard script flag forwarding against builds, compound scripts, and --flag=value Adversarial review of the script-indirection injection found three holes: - vite build received server flags and aborted with "Unknown option --port"; framework build subcommands are now excluded from forwarding. - compound scripts (vite dev && node other.js) get forwarded flags attached to their last command; forwarding is now disabled when the script contains shell control operators. - --port=5000 was not recognized as an existing port flag, so injection duplicated it; flag detection now matches the --flag=value form too, which also fixes the same duplication on direct invocations. * docs: document compound-script scope limit for --port injection * fix(cli): harden package-script flag injection against runner wrappers and glued operators Two edge cases bypassed the guards added for compound and build scripts: - `bunx vite build` received server flags: the build check inspected a fixed `rawScript[1]`, which a runner wrapper (bunx/npx/yarn dlx) shifts. Locate the framework past the runner (reusing the injectFrameworkFlags resolution) and inspect the first positional after it. - `vite dev&&node x` still forwarded: splitCommand only breaks on whitespace, so an operator glued to a token stays inside it and the exact-token operator check missed it. Match operator characters as a substring instead. * fix(cli): close residual guard bypasses for compound and build scripts A pre-push review gate found two forms in the same class as the guards this branch added: - Newline-separated compound scripts (`vite dev\nnode x`) still forwarded: splitCommand collapses newlines, so a per-token operator check never saw the separator. Test the raw script string via resolveScriptRaw instead, and treat newline/CR as separators alongside the operators. - A space-separated flag value before the build subcommand (`vite --mode production build`) bypassed the build guard: the flag-skip loop stopped at the value. Scan every bare positional after the framework instead of a single slot. Also mirror the compound-script scope caveat into the two apps/docs pages that carry the injection sentence (README and SKILL.md already had it). * fix(cli): quote-aware compound detection + independent flag injection + hermetic shim - Compound-script detection is now quote- and redirection-aware: a quoted metacharacter ('/foo&bar') or a redirection (>vite.log, 2>&1) no longer misreads a single command as compound and skips flag injection (502). - An existing --port no longer suppresses injecting a missing --host (and vice versa); injectFrameworkFlags already checks each independently. - The integration test shim is written to node_modules/.bin and run with an isolated PATH so a real bun installed beside Node cannot shadow it. Found in review by ctate (#366). * fix(cli): resolve the framework through package scripts in one place runApp read path.basename(commandArgs[0]) to decide whether to export HOST. That is `bun` for `bun run dev`, so the Expo LAN carve-out never fired and Metro got HOST=127.0.0.1, degrading its HMR websocket. The injectors and the env binder now share resolveFrameworkBasename. isCompoundShellScript becomes isUnsafeToAppendArgs and rejects a word-initial `#`. The shell discards every flag appended after a comment, so injection reported success and delivered nothing. A `#` inside a word still injects. All five doc surfaces stated one skip class. The code has four. * fix(cli): separate framework resolution from append safety resolvePackageScriptTokens returned null both for "cannot identify" and for "must not touch", so the Expo carve-out went blind for any script portless declines to append to. `expo start --port 4567 # note` needs no injection and still received HOST=127.0.0.1 in LAN mode. An escaped space before `#` was read as a comment. `--open /foo\ #bar` is one argument, so word start is now tracked as state. Docs gain the fifth skip class: runner flags before the script name. * fix(cli): treat a comment after a line continuation as a comment The shell joins `vite dev \<newline># note` into `vite dev # note`, so the comment opens and the appended flags are discarded. The detector cleared word start on the escaped newline. That is right for `--open /foo\ #bar` and wrong for a continuation, which the shell removes entirely. * test(cli): check package-script forwarding against the shell Seventy-four script shapes, grouped by shell construct, run through what portless would run. A shim reads back the child's argv to assert one property: if portless appends flags, the child receives them. A violation is invisible to any test over the constructed array, because the array is correct and the shell throws it away. A second test pins the set of shapes portless declines, so an accidental skip is as visible as a lost flag. Together they go red against every guard revision on this branch. Runs under bun in 1.5 seconds. PORTLESS_DIFFERENTIAL_RUNNERS widens the sweep. * test(cli): pin framework identity apart from append safety The corpus reads the child's argv, so it covers forwarded flags and not the exported environment. Six expo shapes, four of them declined, must all still resolve to `expo`. Goes red against the revision where resolution and append safety shared one nullable return. * test(cli): run the differential sweep where CI runs The sweep defaulted to bun and neither CI job installs it, so the file skipped on both runners. It now picks the first package manager present, and the two pure-function tests no longer sit behind the runner gate. Windows is excluded and recorded, since the shims are `#!/bin/sh`. Shapes that invoke a runner by name are excluded when that runner is absent, so an environment limit is not reported as a defect. * fix(cli): treat `&>` as a separator, not a redirection `&>` is a bash extension. Package managers run scripts through `sh`, which is dash on Debian and Ubuntu, where `vite dev &> out.log --port 4567` backgrounds the framework and loses every appended flag. Only POSIX descriptor duplication stays exempt: `2>&1`, `>&2`, `2>&-`. Under /bin/dash the corpus goes from one violation to zero. The Expo environment test drove real bun, which CI does not install. It now uses the package-manager shim helper, covering a plain expo script and one that supplies its own port and ends in a comment. * test(cli): give the package-manager shim a working PATH on Windows The helper forces PATH=/usr/bin:/bin so a real bun cannot shadow the shim. On Windows that leaves the child without System32, the .cmd shim never runs, and all six tests using the helper fail with ENOENT on the capture file. node_modules/.bin is collected before anything on PATH, so hermeticity does not depend on it. Windows keeps the ambient PATH and POSIX is unchanged. The helper now reports platform, node_modules/.bin contents, and the CLI exit code and output when the shim does not run. * fix(cli): inject server flags only into server subcommands Framework CLIs reject `--port`/`--host` on anything that does not serve, so forwarding them through a package script broke working commands: `vite optimize` exits with `CACError: Unknown option --port`. Excluding `build` by name did not cover `optimize`, `vp test`, `astro check` or the rest. Each framework entry now lists its server subcommands and whether a bare invocation starts a server, and flags are appended only when the subcommand resolves to one of them. Resolving it needs flag arity: `--mode dev build` is a build, not a dev server, and mode names collide with subcommand names. Entries list the flags that consume the next token for the CLIs that accept flags before the subcommand; a CLI that lists none is not classified once a flag precedes the subcommand. An unrecognized subcommand is declined, which costs a missed injection instead of a broken command. Flags now go before a bare `--` rather than after it. Everything past `--` is positional data, so a dev server invoked as `vite dev -- --extra` kept port 5173 while the proxy routed to the assigned port. A package script with its own `--` is skipped instead, since appending cannot reach past it. The rule sits in `injectFrameworkFlags`, so the argv path stops forwarding into `vite build` as well. * docs(cli): note that only server commands get injected flags The scope limit listed compound, commented, env-prefixed, delegating and runner-flagged scripts but not the non-server subcommands, nor the invocations portless declines to classify, so a reader with `"build": "vite build"` had no way to tell whether the missing port was a bug. * fix(cli): preserve nested command argument ownership * test(cli): make LAN bypass checks portable --------- Co-authored-by: EfeDurmaz16 <efebarandurmaz05@gmail.com>