Repository navigation
feat(npm): make @deepgram/cli the real Deepgram CLI - #146
Draft
dg-coreylweathers wants to merge 21 commits into
Draft
dg-coreylweathers wants to merge 21 commits into
dg-coreylweathers wants to merge 21 commits into
Conversation
A zero-dependency Node launcher that runs deepctl from PyPI at the same version as the npm package: a cached venv first, then uv tool run, then a venv built from Python 3.10+, with a clear error when none is available. Launcher output stays on stderr, exit codes and signals pass through, and the child sees DEEPCTL_LAUNCHER=npm.
deepctl started by @deepgram/cli (DEEPCTL_LAUNCHER=npm) is now InstallMethod.NPM, checked before every other method because the launcher runs it through uv or its own venv. dg update prints the npm command instead of running pip/uv, the update banner and dg update --check-only name npm, and the banner is no longer suppressed as one-shot under npm. Plugin update hints say dg rather than deepctl, which npm does not install.
The launcher's environment is replaced on every npm version bump, so plugins installed into it would disappear. Treat npm like Homebrew and system installs.
The npm launcher's uv strategy looked one-shot and suppressed the prompt, and the prompt named deepctl, which npm does not install.
release-please now bumps npm/package.json with the root version. A new publish-npm job runs after verify-published on root tags: it checks the package version equals the tag, skips a version npm already has, publishes with trusted publishing and provenance, then waits for the registry and runs the published package through npx and uv.
Unit tests plus a smoke test that runs the launcher through real uv against the newest deepctl on PyPI. The job is part of the all-checks rollup.
…lms-full.txt llms-full.txt said deepgram/tap/deepctl; the formula is deepgram/tap/deepgram, as the README already says.
npm/LICENSE is a copy of the repository LICENSE, so the tarball and npmjs.com carry the notice.
- Only use a cache root that is a real directory owned by this user and not writable by others; the shared /tmp fallback must also be 0700. Another user could otherwise pre-create /tmp/deepgram-cli-<uid> and plant a cached deepctl that runs as the victim. - Drop the DEEPCTL_NPM_LAUNCHER_DEPTH guard: it aborted legitimate nested dg runs, and the realpath self-exclusion on PATH already prevents the launcher from finding itself. - Prune other versions' venvs only after 30 days unused (each run from a cached venv refreshes its marker), never one with a live lock. - A SIGINT or SIGQUIT sent only to the launcher is forwarded to the child if it is still running after a second; a terminal Ctrl-C is unchanged. - A same-host lock is stale only when its pid is dead; release removes the lock only if it is still ours; lock I/O errors are a clean error. - Windows: also try py -3, and try WindowsApps Pythons last instead of skipping them, letting the version probe reject the Store stub. - Report npx and npm exec runs as DEEPCTL_LAUNCHER=npx.
deepctl_core.launcher captures the variable at startup (src/deepctl/main.py) and removes it from os.environ, so a starter app, shell or nested dg that deepctl starts does not inherit a claim about how it was installed. The update and skills startup checks and install detection all read that one value. Under npx, update hints say npx @deepgram/cli@latest.
The launcher job now fails if npm/LICENSE drifts from LICENSE, and runs the launcher with DEEPCTL_NPM_RUNNER=venv on all three OSes, asserting stdout is exactly the version line.
The publish steps move into a composite action shared by publish-npm and a new publish-npm-manual job, run by dispatching release.yml with npm_tag. The manual job checks the tag exists, that npm/package.json at the tag matches it, and that deepctl==V resolves from PyPI, and skips release-please. A version lower than the current latest dist-tag publishes under v<major>.<minor>-maint. npm is pinned to 11.21.0.
- Under a terminal, a SIGINT/SIGQUIT the launcher sees came from the TTY and reached the child too, so it is no longer forwarded a second time (that cut short a child still cleaning up). Without a terminal the delayed forward stays. - A same-host lock naming our own pid, or older than an hour, is stale: a container that crashed mid-setup restarts as pid 1 with the same hostname and cache volume and would otherwise wait forever. - Stale locks are moved aside under a unique name and deleted only if the owner is still the one judged stale, so two waiters cannot delete each other's fresh lock. - A cached venv whose bin/python no longer resolves (base interpreter uninstalled) is treated as missing and rebuilt instead of failing every run with ENOENT. - Every ancestor of a cache root, as written and as resolved, must belong to root or the user and be sticky if others can write to it, so the cache cannot be swapped between check and exec. - Unexpected filesystem errors are reported in one line. - npx is detected from the package's own path (_npx), so a global dg run from inside an npx process is still reported as npm.
…-tag lookup Drop the manual npm_tag re-publish path and restore release.yml's dispatch behaviour; a failed publish is recovered with Re-run failed jobs, which the job comment documents. With one caller left, the composite action is inlined back into publish-npm. Reading the current latest dist-tag now fails the job on any registry error except E404, instead of publishing as latest.
….cache - Reading process.stdin.isTTY made Node wrap fd 0 in a stream, and for a pipe libuv set O_NONBLOCK on it. deepctl inherits that open file, so 'dg mcp' never answered an initialize that arrived a moment late and piped input such as 'ffmpeg | dg listen' could fail. The launcher now uses tty.isatty() and writes its own messages with fs.writeSync(2), and never touches process.stdin, stdout or stderr. - Under umask 002 (Ubuntu, Fedora) ~/.cache is 0775 with the user's own private group. An ancestor owned by the user whose group is the user's primary group may be group-writable; other-writable or foreign-owned ancestors are still rejected. Missing parents are created 0700. - With a terminal attached, a second SIGINT within 3 seconds is passed on to deepctl. - Note the accepted residual race in stale-lock recovery.
A stand-in deepctl checks O_NONBLOCK on fds 0-2 and reads input that arrives a second late, on Linux and macOS.
…tderr best effort - A group-writable ancestor is trusted only when it is ours and its gid equals our uid (the user-private-group convention). On a shared primary group such as SUSE's users or macOS's staff, another member could swap the cache during the first build. - Drop forwarding a second terminal Ctrl-C: the terminal already delivers every press to deepctl, so it arrived twice and broke deepctl's own second-Ctrl-C handling. - Launcher messages complete partial writes, retry EAGAIN briefly and ignore every other error, so a full or closed stderr no longer aborts the first-run setup.
GitHub's ubuntu runners have a real /usr/bin/python3.12, which the launcher probes before python3, so the test built a real venv and ran the real deepctl instead of the checker.
A crash otherwise showed up only as a bare exit status (139 on a macOS CI run), with nothing saying which process died.
This branch has not been deployed
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.
Why
@deepgram/clion npm is the obvious name for "the Deepgram CLI", and it's broken:npm i @deepgram/clifails withnotarget. The only versions are two old betas (0.3.0-beta.0,0.4.0-beta.0) fromlukeocodes/deepgram-cli.npm i @deepgram/cli@0.4.0-beta.0installs, then crashes on start:MODULE_NOT_FOUND …/src/hooks/init/log.Coding agents try npm first. In our build-journey evals, agents asked to "use the Deepgram CLI" went to
@deepgram/cli, hit both failures, and gave up on the CLI.This PR makes
@deepgram/clithe real CLI.What it does
New
npm/package.@deepgram/clihas no dependencies and no install scripts. It providesdganddeepgram, and runs the Pythondeepctlat the same version as the npm package:~/.cache/deepgram-cli/venvs/<V>) if one is already built. This is fast and works offline.uv tool run --from deepctl==<V>. No local Python is needed.venvgets a private venv, built in place under a lock. All bootstrap output goes to stderr, sodg -o jsonanddg mcpstdout stay clean on the first run.python3-venv, or useinstall.sh/brew.DEEPCTL_NPM_RUNNER=uvx|venv|pathforces a strategy.pathuses adeepctlalready on PATH, and only when asked.Python side.
InstallMethod.NPMis detected throughDEEPCTL_LAUNCHER, whichdeepctl_core.launcherreads once and removes from the environment so it doesn't leak to child processes. With it:dg updateand the update banner tell npm and npx users the npm command.dg.Release.
npm/package.jsonalong with the root package (extra-files).publish-npmjob runs afterverify-published, so npm never points at a version PyPI can't install.npm-publish, npm pinned to 11.21.0) and sets provenance.latest, it publishes underv<maj>.<min>-maintinstead of movinglatest.CI. A new
launcherjob on Linux, macOS and Windows, added toall-checks:cmp LICENSE npm/LICENSE.Docs. npm/npx install lines in
README.mdandweb/public/llms-full.txt. The latter also fixes thedeepgram/tap/deepctl→deepgram/tap/deepgramtypo.The first npm version will be 0.4.1, from the next release PR.
0.4.1is newer than0.4.0-beta.0, solatestmoves and plainnpm i @deepgram/cliworks.Hardening
This went through five rounds of independent review, each followed by a fix round. Every finding was reproduced in Docker before it was fixed, and each fix was checked against its repro. Highlights:
Cache security.
gid == uid)./tmpfallback is0700and owner-checked.These checks block another user from planting or swapping a cached venv. Tested by pre-creating the cache, with a symlink, and with a 150-iteration swap loop.
Locking.
mkdirlock with owner, pid and host.Signals and stdio.
dg mcpover stdio,ffmpeg | dg listen, and similar pipes.Self-repair and pruning.
Testing
Docker (linux/arm64; the uvx, venv, signal and stdin cases also on amd64): 43/43 cases pass. They cover:
python3-venv(PEP 668);npx ./tgz;initializesent 1s late.Unit and lint:
check_dependency_floors.pyand actionlint are clean.Not yet run: real macOS and Windows runs, which the new
launcherjob does on this PR, and thepublish-npmjob, which needs the owner setup below.Before the first release, owner steps (not automated)
@deepgram/cli(deepgramai / greg.holmes / deepgram-kiley) adds a trusted publisher: orgdeepgram, repocli, workflowrelease.yml, environmentnpm-publish.npm-publish, ideally limited tomainand with a required reviewer.npm deprecate @deepgram/cli@0.3.0-beta.0 "Replaced by the official Deepgram CLI. Install @deepgram/cli@latest", and the same for@0.4.0-beta.0.npm view.Until the first publish,
npx @deepgram/clistill runs the old 2023 beta, so the README line goes live slightly ahead of the package.Release-PR checklist (floors can't be raised on this branch)
No published
deepctl-corecontainsdeepctl_core.launcheryet, and the floors check rejects a floor above the workspace version. Before merging the release PR that ships this:packages/deepctl-cmd-update/pyproject.toml: changedeepctl-core>=0.1.10todeepctl-core>=<new core version>.packages/deepctl-cmd-skills/pyproject.toml: changedeepctl-core>=0.2.19todeepctl-core>=<new core version>.packages/deepctl-cmd-plugin/pyproject.toml: adddeepctl-cmd-update>=<new cmd-update version>. It importsInstallMethod, includingNPM, but has never declared the dependency.uv lockandscripts/check_dependency_floors.py.A normal
pip install -U deepctlis already safe, because the root floors pin everything.Known limits and follow-ups (not in this PR)
Accepted limits:
pip install deepctlinto the same prefix overwrites npm'sdg, and brew and npm both wantdg. The npm README says to use one install method.staff, SUSEusers) fall back to a private temp dir. Any user-private-group member could interfere, which is a limit of that convention.nohup dg …doesn't survive SIGHUP the waynohup deepctl …does, because of how Node handles it. Usesetsidor deepctl directly for detached runs.Existing bugs found along the way:
dg updatetells Homebrew users to runbrew upgrade deepctl, but the formula isdeepgram.PermissionErroron~/.deepctl/plugins/venv).actions/checkoutpins are commented# v4but resolve to v7.0.1.Phase 2: PyInstaller per-platform binaries through
optionalDependencies, so npm installs need no Python or uv at all.The history includes a manual re-publish path that was added and later removed, so squash on merge.