Skip to content

fix: resolve pre-publish blockers, including a broken write command - #111

Merged
sjsyrek merged 13 commits into
mainfrom
fix/pre-publish-council
Jul 30, 2026
Merged

fix: resolve pre-publish blockers, including a broken write command#111
sjsyrek merged 13 commits into
mainfrom
fix/pre-publish-council

Conversation

@sjsyrek

@sjsyrek sjsyrek commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Summary

Pre-publish hardening for the first public release of @deepl/cli. The headline item is that deepl write did not work at all from a published install — the rest are correctness, security and packaging fixes found while verifying the release surface end to end.

The blocker

diff is a static runtime import in src/cli/commands/write.ts but was declared only under devDependencies, which consumers never install. Reproduced against the packed tarball installed into a clean prefix:

$ deepl write "hello world"
Error: Cannot find package 'diff' imported from .../dist/cli/commands/write.js

The entire DeepL Write feature — one of the two headline capabilities — failed before producing output on every real install. A --help sweep cannot surface this class, because write.js is loaded lazily through service-factory's dynamic import, so the module graph is never resolved until the command actually runs. scripts/check-dependencies.mjs now fails the build on it.

Changes Made

Dependencies and packaging

  • diff and @inquirer/prompts declared as runtime dependencies. @inquirer/prompts is imported by init, write --interactive and sync init while only the unused inquirer was declared, so it resolved purely through npm's hoisting: under a strict layout (pnpm, --install-strategy=nested) those commands fail with ERR_MODULE_NOT_FOUND, and under npm the prompt that reads the user's API key binds to whatever major another dependent happens to hoist. Dependency ranges are immutable per published version, so neither is repairable after 2.0.0 ships.
  • Source maps no longer emitted. They were already excluded from the tarball, so emitting them only left dangling sourceMappingURL comments in 334 shipped files, giving library consumers unresolvable stack frames.
  • SECURITY.md now ships, so offline installs carry the reporting channel. .npmignore removed as dead weight — the files array governs. VERSION removed: nothing read it, and a second hand-edited source of truth could only drift.

Correctness

  • deepl init at a non-TTY exits 6 with the documented message instead of starting to prompt and dying with exit 1 plus a Node unsettled top-level await warning. Hit by docker run without -it, CI, and piped invocations. The guard existed in write --interactive but not here, and the only test covered --no-input init — the variant that already worked.
  • sync --auto-commit retried after a refusal no longer reports success without committing. A refused run writes its translations to disk first, so the retry finds nothing to translate; the preflight was gated on that and was skipped, leaving exit 0 with no commit, the translation uncommitted and the tree still dirty.
  • write --lang/--to accept any casing and normalize to the API's form. They previously required an exact match against a mixed-case list, rejecting the lowercase codes deepl languages prints and translate --to accepts — so the CLI's own output was unusable with the command documented as consistent with translate.
  • The shipped bug-report URL pointed at github.com/user/deepl-cli, a live third-party account that could create that repository and receive reports from a DeepL-branded binary.
  • Hidden internal commands no longer appear in shell completions or did-you-mean output, and the suggester is alias-aware — deepl tr resolves to translate rather than tm.
  • The missing-key remediation survives --quiet. It was emitted as a warning, which quiet mode suppresses entirely; docs/API.md claimed otherwise and is corrected.
  • NO_PROXY/no_proxy honoured, with standard semantics. A corporate HTTPS_PROXY was previously applied even to a localhost endpoint.
  • auth set-key --no-verify added. Validation ran before persisting, so on a network without proxy configuration both documented setup paths failed and discarded the key; an unreachable API now also names DEEPL_API_KEY as the zero-network alternative.

Security

  • The test suite no longer inherits real credentials. Suites that spawn the bare deepl command cannot be intercepted by nock, so they reached the live DeepL API with whatever key was exported and read and wrote the developer's cache database — cached responses matching this suite's fixtures were recovered from a real cache, confirming it had happened. globalSetup now clears DEEPL_API_KEY, TMS_API_KEY and TMS_TOKEN and points DEEPL_CONFIG_DIR at a temp directory. Suite runtime dropped from ~48s to ~28s once the calls stopped.
  • ConfigService.save() writes through an unpredictably named temp file created with an exclusive flag. A symlink planted at the fixed config.json.tmp redirected the plaintext API key to a path of the planter's choosing, and the rename left config.json as that symlink for every later write.
  • The prototype-pollution guard in the JSON parser is now pinned by tests. Replacing Object.defineProperty with plain assignment previously left the whole suite green: on a fresh object obj['__proto__'] = value retargets that object's prototype rather than Object.prototype, so the translation was silently dropped while the negative assertion still passed.

Release workflow

  • npm publishing removed from GitHub. It cannot be triggered from this repository; releases are published from GitLab, which is the source of truth and mirrors to GitHub — the arrangement the other DeepL client libraries use. The publish job is gone along with the environment reference, id-token permission and token wiring, and the homebrew job with it (it declared needs: publish, so leaving it would have made the workflow invalid).
  • The tag↔package.json guard was preserved by moving it into the surviving release job rather than deleting it with its former host. A tag can be pushed at any commit, and without the check a mislabelled tag would mint a Release whose title disagrees with the version it contains.
  • gh release create is now idempotent, so re-running the workflow does not fail on work that already succeeded.

Test Coverage

269 suites / 6096 tests, all green. Lint, type-check and check-deps clean; production npm audit reports 0.

Three new gates, each verified to actually fail rather than merely pass:

  • scripts/check-dependencies.mjs — flags a runtime import missing from dependencies. Compares against dependencies alone, which is what catches a runtime import misfiled under devDependencies. Correctly does not flag php-parser, which is reached indirectly via requireModule.
  • tests/unit/docs/documented-surface.test.ts — checks every documented deepl … invocation in the README and docs against the CLI's real surface, read from the hidden _describe command. Verified it fails on both an unknown command and an unknown flag.
  • tests/require-build.ts — now rejects a stale dist/ by mtime, not just a missing one. It caught a genuinely stale build during this work, reducing thousands of misleading failures to one clear message.

Also added: 50 expect.assertions guards on tests that assert only inside a catch and so passed with zero assertions whenever the command unexpectedly succeeded. Efficacy proven by making a guarded command stop failing and confirming the test turns red. Those guards are what surfaced the sync --auto-commit bug above, along with an assertion that could never have matched (/detached HEAD/i against a message reading "HEAD is detached").

Verified from a tarball installed into a clean prefix with no credentials: deepl --version reports the tree's version; write and translate exit 2 on a missing key; init < /dev/null exits 6; an invalid language exits 6; a lowercase language code is accepted; --no-verify stores a key offline with config.json at mode 0600; a module-resolution sweep across 18 command action paths is clean; and npm pack yields 340 files / 213 kB with README, LICENSE and SECURITY.md present and no maps or test files.

Backward Compatibility

No breaking changes. Additive only — auth set-key --no-verify, NO_PROXY support. write --lang now accepts more casings than before rather than fewer.

Removals have no runtime effectVERSION and .npmignore were unread and inert respectively; CLAUDE.md's release procedure is updated to use npm version.

⚠️ One intentional behaviour change: sync --auto-commit now reports the refusal on every attempt rather than silently succeeding on a retry. Publishing also no longer happens on a tag push, by design.

Size: Large ✓

62 files, ~1000 insertions. The risk concentrates in two one-line manifest changes and a handful of guards, each verified against a real tarball install rather than only in-tree.

🤖 Generated with Claude Code

sjsyrek and others added 12 commits July 29, 2026 23:41
Both are loaded at runtime from src/ but were absent from dependencies, so
they resolved only by accident of the dev tree or npm's flat hoisting.

diff is a static import in cli/commands/write.ts and was declared only in
devDependencies, which consumers never install: `deepl write` failed with
"Cannot find package 'diff'" on every install of the packed tarball, before
any output. The --help sweep did not surface it because write.js loads
lazily via service-factory's dynamic import.

@inquirer/prompts is imported by init, write --interactive and sync init
while only the unused `inquirer` was declared. Under a strict layout it
raises ERR_MODULE_NOT_FOUND; under npm it binds to whatever major a
consumer happens to hoist, in the prompt that reads the user's API key.
Dependency ranges are immutable per published version, so neither is
repairable after 2.0.0 ships.

scripts/check-dependencies.mjs fails the build on either class: it compares
every import specifier in src/ against dependencies alone, so a runtime
import sitting in devDependencies is an error. Package names are also
matched as quoted strings so indirect loads such as
requireModule('php-parser') count as references.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
npm rejects a duplicate version, but @deepl/cli has no published version for
a mistake to collide with: tagging v2.0.0 before the version bump would
publish the stale version, title a Release v2.0.0 whose notes silently fall
back to generated ones, and sign an immutable provenance attestation binding
the wrong version to the tag. Every job would report success.

Also in the publish job, which is the only one holding NPM_TOKEN: actions are
pinned to commit SHAs so a moved tag cannot run unreviewed code with publish
credentials. Release creation is skipped when the release already exists, so
re-running after a publish failure no longer fails on work that succeeded.

The VERSION file is removed rather than guarded against drift: nothing reads
it, package.json is the only source src/version.ts consults, and npm version
keeps package.json and the lockfile in step.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
nock cannot intercept across a process boundary, so suites that spawn the
bare `deepl` command reached the live DeepL API with whatever key the
developer had exported, and read and wrote the real cache database. Six
cached Write responses matching this suite's fixtures were recovered from a
developer cache, so this had already happened repeatedly; suite runtime drops
from ~48s to ~28s once the calls stop.

globalSetup now unsets DEEPL_API_KEY, TMS_API_KEY and TMS_TOKEN and points
DEEPL_CONFIG_DIR at a temporary directory before workers fork, which covers
every spawner rather than the individual suites that opted out of the
isolated runner. Suites that need a key still pass one explicitly.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
… commands

deepl init only checked --no-input, so with stdin at EOF it began prompting
and died with exit 1 plus a Node unsettled-top-level-await warning, where the
documented code is 6. The sibling guard in register-write.ts already pairs
isNoInput() with process.stdin.isTTY. The existing test covered
`--no-input init`, the variant that worked, so the guard looked tested.

The unexpected-API-response error told users to report bugs at
github.com/user/deepl-cli, a live third-party account that could create that
repository and receive reports from a DeepL-branded binary; the string is
compiled into the tarball, so it would freeze into 2.0.0.

Shell completions and the did-you-mean suggester both enumerated commands
unfiltered, publishing the hidden _describe command to end users. Both now
use commander's visibleCommands. The suggester also considers aliases and
prefers a prefix match, so `deepl tr` resolves to translate rather than tm.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
… config writes

write --lang/--to compared codes against a mixed-case list with an exact
match, so it rejected the lowercase codes `deepl languages` prints and
`translate --to` accepts — the CLI's own discovery output was unusable with
the command documented as consistent with translate. Codes are now accepted
in any casing and normalized to the API's form.

The prototype-pollution guard in formats/json.ts was load-bearing but
unpinned: replacing Object.defineProperty with plain assignment left the
entire suite green. The uncovered path is inserting a key the target lacks,
where `obj['__proto__'] = v` retargets the fresh object's prototype rather
than Object.prototype, so the translation is dropped while the negative
pollution assertion still holds. The new positive-data cases fail on that
mutation.

ConfigService.save() wrote through a fixed config.json.tmp. A symlink planted
there redirected the plaintext API key to a path of the planter's choosing
and left config.json as that symlink for every later write. The temp file now
has an unpredictable name and is created with the exclusive flag, so anything
already at the path fails the write instead of being followed.

auth set-key gains --no-verify, and an unreachable API now names both offline
paths: previously validation ran before persisting, so on a network without
proxy configuration every documented way to store a key was closed.

The missing-key error is raised as AuthError instead of a bare warn, so its
remediation survives --quiet, which suppresses warnings entirely; API.md said
otherwise and is corrected.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
The README led with a Homebrew command against a tap that does not exist yet,
while the release workflow gates its formula-bump job off for exactly that
reason — two files in the same commit disagreeing. Homebrew is now listed
after npm and marked as pending. TROUBLESHOOTING pointed a user already
blocked by an unsupported Node version at that same command as the remedy.

sourceMap and declarationMap are disabled: maps are excluded from the
published package, so emitting them only left dangling sourceMappingURL
comments in 334 shipped files, giving library consumers unresolvable stack
frames. SECURITY.md now ships so offline installs carry the reporting
channel, and the redundant .npmignore is removed — the files array governs.

The changelog entry describing the build clean step named an internal module
path, which release.yml would have republished in the v2.0.0 release notes.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
…verify

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
The docs were accurate at this cut, but only because a human checked every
invocation by hand; nothing failed when a command or flag drifted. The new
suite reads the real surface from the hidden _describe command and checks
every `deepl …` line in README, API, SYNC and TROUBLESHOOTING against it.
Verified it fails on both an unknown command and an unknown flag.

require-build now also compares dist's mtime against the newest file in src.
It previously checked existence only, while its own comment named the worse
hazard: the suites execute dist/cli/index.js, so a stale build reports
results that do not describe the current source.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
A corporate HTTPS_PROXY was applied to every request, including one aimed at
localhost, because no bypass list was consulted. NO_PROXY and no_proxy are now
matched against the target host with the standard semantics: * for everything,
a leading dot or *. for subdomains, and an optional port that must agree.

Docs: the exit-code classifier list is reordered to the sequence the code
actually tests, which is what decides the code when a message matches more
than one pattern; the global-options table gains --timeout and --max-retries,
which were documented only in a later section; the quick start now leads with
--from-stdin rather than the argument form the CLI warns about; and the bash
completion instructions no longer imply an unprivileged write to
/etc/bash_completion.d, and name the bash-completion package the generated
script needs to do anything.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
These tests assert only inside a catch block, so they passed with zero
assertions whenever the command under test unexpectedly succeeded — the
failure they exist to detect was the one they could not see. Each now
declares the number of assertions its catch block runs.

Verified the guard bites: making one guarded command stop failing turns the
test red where it previously stayed green.

Sites that already assert outside the try, or that call fail() on the
success path, are left alone — they cannot pass vacuously, so a guard there
adds nothing but noise.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
A refused auto-commit still writes its translations to disk, so the retry
finds nothing to translate. The preflight was gated on this run having
written files, so the retry skipped the checks entirely and exited 0 — no
commit made, the translation still uncommitted, the tree still dirty, and no
error shown. Reproduced directly: the second run returned success with
fileResults empty and HEAD unmoved.

The repo-state checks now run whenever --auto-commit is requested; only
staging and committing are skipped when there is nothing to commit.

Two existing tests covered this path and could not see it: their assertions
sat in a catch block the retry never entered. Declaring the assertion counts
made both fail, which also exposed an assertion that could never have matched
— it looked for "detached HEAD" in a message reading "HEAD is detached".
Counts corrected, the regex fixed, and a test added for the retry itself.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@sjsyrek

sjsyrek commented Jul 30, 2026

Copy link
Copy Markdown
Contributor Author

Update: a second real bug, found by the assertion guards

b8502be adds a fix that was not in the original scope, because the expect.assertions guards surfaced it.

Retrying deepl sync --auto-commit after a refusal reported success without committing. Reproduced directly:

CALL1 threw: ValidationError  "Refusing to auto-commit: ... README.md (M), src.ts"
de.json on disk after call 1: true
CALL2 threw: NOTHING    fileResults.length: 0    success: true
HEAD after both calls: initial        still dirty: 4 entries

A refused run writes its translations to disk before the preflight, so the retry finds nothing to translate. The preflight was gated on result.fileResults.length > 0, so it was skipped entirely and run() resolved — exit 0, no commit, translation left uncommitted, tree still dirty, no error. Same false-green class as a --locale defect fixed earlier in this release.

Fix: the repo-state checks now run whenever --auto-commit is requested; staging and committing are still skipped when there is nothing to commit. After the fix CALL2 throws.

Two tests covered this path and could not see it — their assertions sat inside a catch the retry never entered. Declaring the counts made them fail, which in turn exposed a third defect: the detached-HEAD test asserted /detached HEAD/i against a message reading "HEAD is detached". Reversed word order, so it could never have matched, dead since it was written. Both fixed, plus a new test for the retry itself.

One existing unit test (should not call git when fileResults is empty) encoded the old contract and now asserts the intended one: preflight runs, staging and commit do not.

Deliberately not changed here: the watch path (sync-command.ts:387) keeps the same gate. It is materially less severe — runOnce() catches the throw and logs it via Logger.error, the watcher continues, and no exit code implies success — and naively removing it would log a refusal on nearly every trigger, since a dirty tree is the normal state while editing. Filed as a follow-up issue with three candidate fixes, including the deeper one (make expectedStaged aware of configured target paths) that would also remove the manual-intervention dead end in the one-shot path.

269 suites / 6096 tests green; lint, type-check, check-deps clean; CI green on this commit.

npm publishing cannot be triggered from GitHub for this repository. Releases
are published from GitLab, which is the source of truth and mirrors to
GitHub — the same arrangement the other DeepL client libraries use, where
deepl-node publishes from a .gitlab-ci.yml job and has its GitHub publish
step commented out.

Removes the publish job, along with the npm-publish environment reference,
id-token permission, NPM_TOKEN wiring and --provenance. Removes the homebrew
formula-bump job too: it declared `needs: publish`, so leaving it would make
the workflow invalid, and its bump logic is recorded on the tap issue for
whoever builds the GitLab pipeline.

The tag-versus-package.json guard moves into the release job rather than
being deleted with its former host. A tag can be pushed at any commit, and
without the check a mislabelled tag would mint a Release whose title
disagrees with the version it contains.

What remains on a tag push is the GitHub Release and nothing else. The three
changelog entries describing GitHub-side publishing, provenance and the
required-reviewer gate are reworded, since they would otherwise ship as false
claims in the 2.0.0 notes.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@sjsyrek

sjsyrek commented Jul 30, 2026

Copy link
Copy Markdown
Contributor Author

Update: npm publishing removed from GitHub (d180349)

Publishing to npm cannot be triggered from GitHub for this repository. Releases will be published from GitLab, which becomes the source of truth and mirrors to GitHub — the arrangement the rest of the DeepL client-library fleet already uses. deepl-node publishes from a .gitlab-ci.yml job and has its GitHub publish step commented out with the note "Test and npm publish stage are disabled for now."

Removed from .github/workflows/release.yml:

  • the publish job in full — npm-publish environment, id-token: write, NPM_TOKEN wiring and --provenance
  • the homebrew job — not optional cleanup, since it declared needs: publish and would have made the workflow invalid. Its formula-bump logic is preserved as a comment on the tap issue.

Preserved rather than deleted: the tag↔package.json guard, moved into the surviving release job. It lived inside publish, so deleting that job would have silently taken the guard with it — and it still catches a mislabelled tag minting a Release whose title disagrees with its contents.

What a tag push does now: creates the GitHub Release, and nothing else. Verified no NPM_TOKEN exists at either repo or environment scope, and ci.yml/security.yml both filter on branches: [main], so neither fires on a tag.

Provenance is dropped, and this is worth stating plainly: npm --provenance needs a CI with OIDC and cannot be produced from a laptop. That puts @deepl/cli at parity with the fleet rather than behind it — npm view deepl-node dist.attestations returns nothing for 1.27.0. It can return once GitLab publishes, since npm supports GitLab CI for trusted publishing.

Three [Unreleased] changelog entries describing GitHub-side publishing, provenance and the required-reviewer gate are reworded; they would otherwise have shipped as false claims in the 2.0.0 notes.

Suite green (the one suite failure in my local run was the known Node 24 V8 SIGSEGV worker flake — 0 tests failed, and it passes in isolation).

@sjsyrek sjsyrek changed the title fix: resolve all pre-publish council findings, including a broken write command fix: resolve pre-publish blockers, including a broken write command Jul 30, 2026
@sjsyrek
sjsyrek merged commit 081790e into main Jul 30, 2026
5 checks passed
@sjsyrek
sjsyrek deleted the fix/pre-publish-council branch July 30, 2026 14:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant