fix: land the v2.0.0 release-blocker batch (cache, formats, sync CLI, api/voice, hooks/glossary, docs) - #108
Merged
Merged
Conversation
- README Development prerequisites said Node >= 20.19.0; engines.node is >=24.0.0 - SYNC.md GitLab CI recipe pinned node:20, which cannot install or run @deepl/cli - README claimed 429 is not retried; the client retries with Retry-After/jittered backoff - API.md advertised versions 1.2.0/1.1.0 and stale dates in header and footer - API.md cited nonexistent 'deepl auth test' and 'deepl config unset' as exit-code sources - write supported-language lists omitted ja, ko, zh, zh-Hans in API.md and README - sync init alias-deprecation prose rewritten for the 2.0 cut (aliases removed) - CHANGELOG exit-codes cross-link pointed at API.md instead of docs/API.md 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
CacheService always started enabled, so `cache stats` reported the process-local default rather than the persisted `cache.enabled` value and could never print "disabled" — not even after `config set cache.enabled false`, which the translation path does honour. CacheServiceOptions gains `enabled`, and the CLI's option factory (now `resolveCacheOptions`) reads `cache.enabled` alongside `cache.ttl` and `cache.maxSize`. Deriving it there rather than inside CacheService keeps `--config <path>` overrides effective, since that flag replaces the ConfigService the factory closes over.
… backups Resolve the hooks directory with `git rev-parse --git-path hooks` so `deepl hooks install` honours core.hooksPath (husky) and works when `.git` is a gitdir pointer file (linked worktrees, submodules) instead of writing to a `<gitdir>/hooks` path git never reads. A repeat install no longer overwrites an existing `.backup`: the second backup lands in a free numbered slot, and install now returns the hook and backup paths so the CLI can print where the previous hook went. Also resolve findGitRoot's start path to an absolute path — a relative path made the parent-directory walk loop forever.
`cache disable` printed "✓ Cache disabled" and exited 0, but only flipped an in-memory field in a process that exited immediately. Nothing reached config, so the next invocation still had the cache enabled and translations kept writing entries. `cache enable --max-size` appeared to work only because that path already persisted `cache.maxSize`. Both handlers now write `cache.enabled` to config, which is what TranslationService reads. The in-memory enable()/disable() calls remain for the rest of the current process. The e2e regression test asserts the toggle from a separate CLI invocation: an in-process test would have passed while the bug was live.
A translation containing "]]>" ended the CDATA section it was written into, so its remainder became XML markup in the generated resource file. The adjacent-section split that stood in for escaping could not survive the round-trip either, since the deletion pass then rewrote the result. Refuse such values with a ValidationError instead, matching the XLIFF parser's stance on CDATA in translatable content.
Terms whose source or translation is empty, whitespace-only, or contains a tab, carriage return or newline are now skipped with a warning naming the key. Uploading them either split the term across TSV columns or fabricated a different entry pair that DeepL then applied to live translations. Dictionary comparison now normalizes both sides the way the glossary TSV round trip does, so a clean glossary is created once and never re-uploaded. Previously local terms were compared against trimmed API entries, which could never be equal, so every sync issued an update. Glossary failures are also isolated per locale and reported with the locale, glossary name and term count instead of silently ending glossary sync for every remaining locale.
The YAML parser resolves aliases while walking the document AST, a path the yaml package's maxAliasCount guard does not cover. Nested alias fan-out expanded exponentially and a self-referential anchor recursed until the stack overflowed. Both extract and reconstruct now carry an alias budget that caps the number of nodes reached through alias expansion and the alias nesting depth, throwing the parser's usual "YAML parse error" for documents that exceed either limit.
…lect per file addEntry/updateEntry now reject a source or target containing a tab, carriage return or newline, naming the offending character. Passing argv straight through let a single term shift every following column of the uploaded dictionary. tsvToEntries picks TSV or CSV once for the whole file instead of per line, so a quoted CSV field containing a tab is no longer tab-split into garbage, and rows whose parsed fields carry a separator character are skipped.
Commander keeps matching the parent command's options after a subcommand name, so `deepl sync status --locale de` bound `de` to the parent `sync` command and left the subcommand's own option store undefined. Both flags are declared on the parent and on the subcommands, so the filter logic in status/validate/export never received a value and --sync-config silently fell back to the auto-detected config. Add resolveSyncConfig() alongside the existing resolveLocale(), plus a shared parseLocaleFilter(), and route status, validate, export, audit, resolve, push, and pull through them.
…res by code A client-side abort says nothing about whether the server already accepted the request, yet every failure other than a 4xx was replayed for every HTTP method. A slow batch translate or a large document upload that exceeded the 30s timeout was re-sent three more times, each attempt already accepted and billed server-side; worst case, admin key creation duplicated keys whose secret is returned once. Automatic retries are now limited to idempotent methods (GET/HEAD/PUT/ DELETE/OPTIONS/TRACE). A POST is replayed only on transport errors that prove it never reached the server (ECONNREFUSED/ENOTFOUND/EAI_AGAIN); 429 handling is unchanged for all methods because a rate-limited request was rejected without being processed. Classification now branches on the axios error code and the absence of a response instead of matching message substrings: a client-side timeout reads "timeout of 30000ms exceeded", matched no substring, and surfaced as ValidationError (exit 6) where exit-codes.ts documents a network error (exit 5). ECONNABORTED and ERR_CANCELED were absent entirely. 401 now maps to AuthError (exit 2) instead of falling through to exit 6. Also adds a wall-clock budget shared across attempts (default: twice the per-request timeout, backoff sleeps excluded) so a never-responding server bounds total wait instead of running the full attempt count, scopes the Trace ID quoted in an error to the request that failed rather than the client's last-seen header, and stops doubling the "Network error:" prefix. Tests updated deliberately, all of which assumed retry-everything: - deepl-client.test.ts retry-logic and maxRetries cases now exercise an idempotent GET; a new case asserts translate is not replayed. - deepl-client.integration.test.ts 500-retry case likewise, plus a new no-replay assertion. - sync.integration.test.ts 503 recovery now asserts the POST is not replayed and the locale is recorded as failed.
docs/API.md has documented that a locale outside target_locales passed to `sync --locale` exits with a ConfigError, but the run reported success while translating nothing — a typo'd locale made CI report green. Validate the filter in applyCliOverrides, which every config load already funnels through, so the root command and every subcommand that resolves a --locale filter enforce it from one place. The error names the offending locales and the configured ones.
…nc audit `deepl sync audit` fell back to the lock file's per-locale hash when a target file could not be read. That hash is a hash of the SOURCE text, so it is identical across a term group by construction: divergent translations were reported as consistent, and identical ones as an inconsistency displaying a hex hash where a translation should be. Unreadable targets are now excluded from the comparison and listed in a new `missingTargets` field, which the text output prints as well.
`sync init` registered --sync-config but always resolved the config against process.cwd(), so the flag could neither redirect where the file was written nor which existing file triggered the already-exists guard. Resolve the flag to the config file path (any basename, matching what loadSyncConfig accepts) and use its directory as the root that detection and the generated globs are relative to. configExists() and writeSyncConfig() now take that path instead of a root directory.
… timeouts The result endpoint is effectively single-use, yet a client-side timeout replayed the download three more times; a replay against an exhausted result can permanently lose an already-billed translation. Downloads now run with retries disabled. Uploads and downloads also move whole files, so both now get a 300s transfer timeout instead of the 30s interactive request timeout (an explicitly configured longer timeout still wins). Status polling keeps the default timeout.
…ody size Build request URLs through the URL API so a trailing slash on server: no longer yields "//", a base path survives, and a query string or fragment is rejected up front instead of silently truncating the API path. Timeouts now throw TmsTimeoutError, a NetworkError subclass, so a hung TMS exits 5 instead of falling through the untyped-message classifier to 1. Error messages route the server URL through sanitizeUrl, which redacts embedded credentials; the helper moved to src/utils/sanitize-url.ts and is still re-exported from http-client. pullKeys reads the body under a 32MiB cap — the existing key and value limits only applied after the whole response had already been parsed.
writeSyncConfig used a plain writeFile, so an interrupted `deepl sync init` could leave a truncated config in place of a working one. Use the same atomic write-then-rename helper as the rest of the sync writers.
…ve-up WebSocket reconnect was unreachable for real transport failures. The client bridged the socket's 'error' event into callbacks.onError, which marked the stream ended and rejected, so the close event that always follows an error returned early and never reconnected — despite --reconnect defaulting on with three attempts. Every socket also carried two 'error' listeners, so the run promise was rejected twice. The socket error is now recorded and handleClose decides whether to reconnect, reporting the recorded transport error when no attempt is left. VoiceClient no longer routes transport errors into the server-message callbacks, leaving exactly one 'error' listener per socket and keeping protocol errors distinguishable from dropped connections. A terminal failure now also wakes the chunk pump and closes the input generator. Previously a failed reconnect left a library consumer's audio generator suspended at a yield forever, holding its fd. Test updates: the two existing "WebSocket emits error" cases now emit the close event that ws always follows an error with, and the voice-client case asserts the socket owner sees transport errors instead of onError.
Both parsers matched elements with `<tag ...>([\s\S]*?)</tag>`, so every opening tag with no matching close scanned to end of input — quadratic in file size, and reachable with a file well under the sync size cap. A 4 MiB input took minutes; Android reconstruct additionally rescanned the whole file once per dotted key to classify string-array members. Replace the lazy patterns with a single forward scan: sticky opening-tag patterns that cannot match past the following tag, and one cursor for the closing-tag search. A missing closing tag now ends the scan instead of being retried per opening tag. The scan also skips CDATA sections when looking for a closing tag, so markup quoted inside one no longer truncates the element, and a self-closing element is left alone rather than swallowing its successor.
… init BREAKING CHANGE: `deepl sync init --source-lang` and `--target-langs` are removed and now fail with "unknown option" (exit 1). Use --source-locale and --target-locales instead. The aliases were introduced in 1.x with a stderr deprecation warning naming the replacement. The documented policy is that a 1.x alias is removed at the 2.0 cut, and this build is that cut, so keeping them would contradict the shipped policy. Drops the two hidden option registrations, applyDeprecationAliases() and its warning constants, and the sourceLang/targetLangs fields on InitOptions. The commander-tree snapshot is updated intentionally: the two options are gone from the `init` subcommand.
Both the request timeout and the retry count were hardcoded with no flag or config key, so a user whose 50-item batch or 30MB upload needed more than 30s had no way to say so. Both are now global options, validated as integers (>= 1 ms and >= 0 retries) and wired into every client the commands construct. Adds an E2E suite that drives the CLI against a server which accepts requests and never answers: it asserts exit code 5, that the translate POST reaches the server exactly once, and that --timeout bounds the run.
# Conflicts: # src/sync/sync-init.ts # tests/unit/sync/sync-init.test.ts
Adds entries for the cache toggle persistence, YAML alias guard, git hooks and glossary batch, sync option routing and locale validation, the sync init alias removal, and the linear-time Android XML / XLIFF parsers. Also merges the duplicated Fixed/Security headers in Unreleased into one section per category, in canonical Keep a Changelog order. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Clients that wrap their own catch in handleError (write-client, document-client) hand back errors classifyError already produced. Re-running classification on those now returns them unchanged instead of re-deriving a class from the wrapped message, which turned WriteClient's 400 ValidationError into a NetworkError and dropped its style/tone docs hint. Also tightens the total-deadline assertions to prove retries happen before the budget cuts them off, rather than only that the count stays low.
…or note Moves the network-prefix helper below computeBackoffWithJitter so that function keeps its own doc comment, and states VoiceClient's transport-error contract in terms of current behavior.
…last nock v14 emits async socket errors from replyWithError that leak into later tests, which the repo already works around by keeping those blocks last; interleaved they segfaulted the jest worker on a full-suite run. Also aborts pending requests before cleaning interceptors, matching the global teardown.
A 429 proves the server received and rejected the request without processing it, so the idempotency restriction must not apply: the retry stays enabled for POST and the Retry-After delay is respected verbatim.
# Conflicts: # src/cli/index.ts
…client fix batch The README retry section now matches the shipped transport behavior: idempotent-only automatic retry, POST replay restricted to provably-unsent requests, 429 retried for all methods with Retry-After, full-jitter backoff instead of the fixed ladder the section invented, and the new --timeout / --max-retries global flags. CHANGELOG gains the Unreleased entries for the retry, classification, deadline, document-download, voice-reconnect, and watch-stats fixes. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
…sport policy TROUBLESHOOTING no longer claims universal 5xx auto-retry or the fixed backoff ladder; API.md documents the shared wall-clock retry budget next to the global flags. The changelog drops the watch filesWatched entry (that counter was deliberately left for a follow-up issue, not fixed in this batch) and gains the trace-ID cross-quoting and idempotent-classification fixes that shipped with it. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
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.
Summary
Lands the seven remaining P1 release blockers for v2.0.0 (epic sync-8mqh): six audited defect batches fixed on parallel worktree branches plus the pre-release documentation corrections. Each branch passed the full suite independently; the integrated tree passes 262 suites / 5879 tests.
Changes Made
cache enable/disablepersistcache.enabled;cache statsreports the persisted state. Cross-process e2e regression test.xml-scan.ts) replaces the quadratic lazy regexes — adversarial 4 MiB files drop from ~2 min to ~10 ms; well-formed output byte-identical. Android values containing]]>are refused instead of breaking out of CDATA and injecting XML.--locale/--sync-configactually reach the subcommand handlers (commander parent/child binding); out-of-config locales exit 7 (ConfigError) as documented;sync init --sync-confighonored; BREAKING: the deprecatedsync init --source-lang/--target-langsaliases are removed at the 2.0 cut, per the documented policy.--timeout/--max-retriesflags.git rev-parse --git-path hooks(husky/core.hooksPath, worktrees, submodules); backups no longer clobbered; auto-glossary validates terms and is idempotent across runs; glossary entries reject separator chars, dialect sniffed per file;sync auditstops using source hashes as translation identities (adds additivemissingTargetsto the JSON output); TMS URL joining, credential redaction, timeout exit code, 32 MiB bounded response read;sync initwrites atomically.Test Coverage
~130 tests added or deliberately rewritten across unit, integration, and e2e tiers, including cross-process persistence, real-commander e2e option routing (spawnSync on the built CLI), nock server-side request-count assertions, real git worktree/submodule/husky fixtures, and 4 MiB adversarial parser inputs with elapsed-time bounds. Full suite: 262 suites / 5879 tests, exit 0; lint and type-check clean.
Backward Compatibility
❌ Breaking (intended at the 2.0 cut):
deepl sync init --source-lang/--target-langsremoved (use--source-locale/--target-locales); POSTs are no longer retried on 5xx/timeout (behavior, not API);]]>inside Android CDATA values and out-of-config--localevalues now error instead of corrupting/no-op.✅ Maintained: all other CLI surfaces, JSON contracts (one additive field:
sync audit --format jsongainsmissingTargets), on-disk formats, exit-code contract (now matched more accurately).Size: Large ✓
32 commits across 6 independently-tested fix branches plus docs; wide surface but each fix is narrowly scoped to its audited defect.
🤖 Generated with Claude Code