diff --git a/CHANGELOG.md b/CHANGELOG.md index 903b416..272ada6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added - **ci**: Pushing a `v*` tag now runs the full release pipeline. The npm publish job is enabled (it was hard-disabled with `if: false` since its introduction, which is why the repo has 17 tags and zero GitHub Releases); it authenticates with a granular `NPM_TOKEN` for the first publish and keeps `--provenance` attestation. A new, deliberately independent `release` job creates a GitHub Release from the tag, with notes extracted from the version's CHANGELOG section (falling back to generated notes if the section is missing) — a publish failure cannot suppress the Release, and vice versa. A `homebrew` formula-bump job (version + sha256 PR against `DeepL/homebrew-tap`) ships gated with `if: false` until the tap repo and its cross-repo token exist. +- **cli**: Global `--timeout ` and `--max-retries ` options override the HTTP transport defaults (30000 ms, 3 retries) for a single invocation. Neither was previously configurable from the CLI. ### Changed @@ -20,17 +21,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **tests**: Jest's haste map no longer indexes `.claude/` (via `modulePathIgnorePatterns`). A leftover agent worktree under `.claude/worktrees/` duplicated the manual mocks in `tests/__mocks__/` and made every jest run emit a `jest-haste-map: duplicate manual mock found` warning; test files in worktrees were already excluded, but module indexing was not. Verified warning-free with a worktree present. - **BREAKING — package**: The package is now published as the scoped **`@deepl/cli`** (previously the unpublished working name `deepl-cli`). Scoped packages default to restricted visibility, so `publishConfig.access: "public"` is set explicitly — without it the publish fails. The `bin` name is unchanged: the command is still `deepl`, and scoping changes only the install string (`npm install -g @deepl/cli`). Repository, bugs, and homepage metadata now point at `github.com/DeepL/deepl-cli` directly instead of relying on the redirect from the legacy `DeepLcom` org name. - **BREAKING — cache/runtime**: The translation cache now uses Node's built-in `node:sqlite` module instead of the `better-sqlite3` native addon, and the CLI consequently requires **Node.js >= 24** (`engines.node` is now `>=24.0.0`). Node 24 is the first release where `node:sqlite` is non-experimental; Node 18/20 lack the module entirely and Node 22 would both warn on every invocation and downgrade the bundled SQLite (3.50.4 vs 3.53.1). Existing cache databases are read in place with no migration — the on-disk format is unchanged (SQLite 3.53.2-written files verified readable, WAL mode and `user_version` stamp included). **Migration**: upgrade to Node 24 (current LTS), e.g. `nvm install 24`; no other action is needed, and the cache keeps its contents. - - **ci**: `.nvmrc` now pins Node 24, matching the CI matrix. It still read `20`, so `nvm use` handed local developers a runtime that cannot load `node:sqlite` and does not satisfy `commander`'s `engines.node >=22.12.0`. - **build**: `npm run build` now runs a `clean` step first, removing `dist/` and `tsconfig.tsbuildinfo` before compiling. Without it, `tsc` leaves output for sources that no longer exist, so a file rename could ship stale artifacts — verified reproducible: planting `dist/sync/strings-client.js` and rebuilding left both files in `npm pack` output. Removing the build-info file is required too; deleting only `dist/` makes the incremental compiler report the output as up to date and emit nothing. - **tests**: Raised the timeout for the `watchAndSync` test block to 30s and replaced its fixed-round setup flush with a wait on an observable readiness signal. These tests intermittently exceeded the 10s default on CI runners — always as timeouts, never assertions — failing unrelated dependency PRs. Suite duration was measured to be flat across 250/25/5 flush rounds, so the historical 20 → 50 → 250 escalation could not have addressed it; the cost is runner CPU starvation (1.8s locally vs 10.8s observed on CI). The flush now also fails with the pending state rather than a bare timeout if setup genuinely stalls. - **deps**: `commander` 14.0.3 → 15.0.0. commander 15 is ESM-only (`"type": "module"`) and requires Node >=22.12.0, so it was unmergeable until the Node 24 baseline landed. Jest's `transformIgnorePatterns` allowlist gains `commander` so ts-jest transforms it under CommonJS test execution; without that, 28 suites fail to load with `SyntaxError: Cannot use import statement outside a module`. - **ci**: Test matrix, release workflow, and security workflow now target Node 24 (previously Node 20 and 22). Node 20 reached end-of-life in April 2026, and Node 24 is the current LTS. This aligns CI with the runtime the project is moving to; the `engines.node` range is unchanged in this entry and is bumped separately. -### Security +### Removed -- **translate/sync**: Placeholder restoration no longer hangs the CLI with unbounded memory growth. `restorePlaceholders` looped `while (restored.includes(placeholder))`, replacing one occurrence per pass — so when the preserved original itself contained the placeholder token, every pass re-inserted it and the guard never went false. Measured before the fix: the input `{__VAR_0__}` grew from 9 to 400,009 bytes across 200,000 iterations without converging, and the real function had to be killed. It needed no attacker and no network: `preserveVariables`' pattern matches `{__VAR_0__}` (underscores and digits are in its character class), variable preservation runs unconditionally, and restoration also runs on **cached** results — so a locale value of that shape hung the process with no API call. Each placeholder is now restored in a single pass, using the function form of the replacement so `$&`/`$1` inside a preserved value stay literal. -- **sync**: Bucket `include` globs can no longer escape the project root, and `--dry-run` no longer modifies the working tree. `include` entries were validated only as non-empty strings, while `target_path_pattern` a few lines later already rejected `..`. The unvalidated glob's literal prefix was resolved and handed to the stale-`.bak` sweep, which recursed with **no containment check** — deleting every old `*.bak` it found and *re-creating* any file whose `.bak` existed while the live file was missing or empty. Verified: `include: "../../../../../../**/*.json"` produced a sweep root of `/var`, an out-of-root `.bak` was deleted and its sibling resurrected with the backup's contents. Two things made it worse: the sweep was gated only on watch runs, so it ran under `--dry-run` — the flag a cautious user reaches for to avoid side effects — and its errors were swallowed entirely, so it was silent. `include` entries are now rejected at config load for traversal segments and absolute paths (the check the `sync init` wizard already applied, which simply did not exist on the load path), the sweep independently refuses any root outside the project and logs the attempt, the sweep is skipped under `--dry-run`, and its failures are reported instead of discarded. +- **deps**: `better-sqlite3` and `@types/better-sqlite3`. The production dependency tree no longer contains any native addon, removing the entire class of ABI-mismatch failures (`ERR_DLOPEN_FAILED` / `NODE_MODULE_VERSION` errors after a Node major upgrade, e.g. via `brew upgrade node`), a 1.9 MB platform-specific binary, and the C++ compilation-toolchain requirement for installs from source. The cacheless-degradation safety net remains: a runtime whose `node:sqlite` is missing (Node < 22.5.0) warns once and runs uncached rather than crashing, and never touches the cache database. +- **BREAKING — sync**: `deepl sync init --source-lang` and `--target-langs`, the deprecated aliases introduced in 1.x, are removed and now fail with `unknown option` (exit 1). Use `--source-locale` and `--target-locales`. This is the documented removal of 1.x aliases at the 2.0 cut. `deepl translate --target-lang` is unaffected — it is the API's wire name, not a deprecated alias. ### Fixed @@ -59,23 +59,40 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **glossary**: A glossary term named after an `Object.prototype` member is no longer silently dropped or misreported. `tsvToEntries` accumulated into a plain object and tested for duplicates with `entries[source] !== undefined`, so `toString` (and friends) triggered a spurious "Duplicate source" warning, and a `__proto__` term was swallowed by the prototype setter instead of being stored. Genuine duplicate detection is unchanged. - **sync**: A lockfile entry whose i18n key is named `__proto__` no longer vanishes on every write — the key-sorting JSON replacer accumulated into a plain object, where that assignment invokes the prototype setter. Auto-glossary term extraction, which is keyed by untrusted source strings, was fixed the same way. - **formats**: The JSON parser no longer pollutes `Object.prototype` via a `__proto__` key in a resource file, and now round-trips prototype-named keys as ordinary data. `hasKey` used `part in record`, which reports inherited members, and `setKeyWithParts` assigned with `current[part] = …`, where `current['__proto__'] = {}` invokes the prototype setter instead of creating a property. Membership is now an own-property check, and assignment goes through `Object.defineProperty`, so a key legitimately called `toString` translates like any other. - - **hooks**: Generated git hooks no longer emit a broken install instruction. The `pre-push` hook template told users to globally install the unpublished `deepl-cli` name — which fails with `ENOVERSIONS` — and now points at `@deepl/cli`. Reported by **@maa-xx** in #70, who correctly diagnosed that the documentation instructed readers to install a package that does not resolve; their docs fix was superseded by actually publishing the package, but this source-level occurrence was the one their PR missed and is now guarded by a regression test asserting generated hook output never references an unpublished package name. - -### Removed - -- **deps**: `better-sqlite3` and `@types/better-sqlite3`. The production dependency tree no longer contains any native addon, removing the entire class of ABI-mismatch failures (`ERR_DLOPEN_FAILED` / `NODE_MODULE_VERSION` errors after a Node major upgrade, e.g. via `brew upgrade node`), a 1.9 MB platform-specific binary, and the C++ compilation-toolchain requirement for installs from source. The cacheless-degradation safety net remains: a runtime whose `node:sqlite` is missing (Node < 22.5.0) warns once and runs uncached rather than crashing, and never touches the cache database. - -### Fixed - - **cache**: A cache backend that fails to load (e.g. `better-sqlite3` ABI mismatch after a Node major upgrade, `ERR_DLOPEN_FAILED`) is no longer misclassified as database corruption. Previously the constructor's catch-all renamed the user's healthy `cache.db` to `cache.db.corrupt-` and recreated an empty database — verified to quarantine a 2,646-entry cache that passed `integrity_check`. Native-module load failures now leave the database and its `-wal`/`-shm` sidecars untouched; genuine corruption still triggers the rename-aside recovery. `deepl translate` and `deepl write` degrade to running without a cache (single warning per process, exit code 0) instead of crashing, since the cache backend is loaded lazily behind a warn-once latch; `deepl cache …` subcommands, which cannot run cacheless, fail with an actionable error suggesting a reinstall or matching Node version. - **sync**: `deepl sync --frozen` now reports an accurate key count when drift is caused by a newly-added target locale. Previously the message read `Sync drift detected: 0 new, 0 stale keys.` because the frozen branch in `processBucket` short-circuited before promoting current-status keys missing a target-locale translation into `newKeys` — even though `--dry-run` against the same state correctly reported the backfill count. The drift exit code (10) is unchanged. The drift message now also surfaces `deletedKeys` and only mentions nonzero categories, mirroring the success-path summary format. +- **cache**: `deepl cache enable` / `deepl cache disable` now persist `cache.enabled` to the config file. Both reported success but only flipped an in-memory flag in a process that exited immediately, so the state reverted instantly and subsequent translations kept using the cache after it had been disabled. `deepl cache stats` likewise read a process-local flag that always initialized to enabled, so the status line could never show `disabled` — not even after `deepl config set cache.enabled false`; it now reports the persisted state. +- **sync**: Subcommands now honor `--locale`. Commander bound the flag to the parent `sync` command even when it trailed the subcommand name, so `sync status --locale de` listed every configured locale and `sync export --locale de` emitted XLIFF for all of them. Affects `status`, `validate`, and `export`. +- **sync**: Subcommands now honor `--sync-config`. The flag was silently ignored and the auto-detected `.deepl-sync.yaml` used instead, so `sync validate --sync-config /missing.yaml` exited 0. Affects `status`, `validate`, `export`, `audit`, `resolve`, `push`, and `pull`. +- **sync**: A `--locale` value that is not in `target_locales` now exits with a `ConfigError` (exit 7) naming the offending and configured locales, as documented. Previously `sync --locale ` exited 0 reporting success while translating nothing, which made a typo'd locale in CI report green. +- **sync**: `deepl sync init --sync-config ` now writes the config at that path and checks the already-exists guard against it, instead of always using the current directory. `.deepl-sync.yaml` is also written atomically, so an interrupted run cannot leave a truncated config behind. +- **formats**: Android XML and XLIFF parsing is now linear in file size. Both parsers matched elements with a lazy `([\s\S]*?)` pattern, so every opening tag without a matching close rescanned the rest of the file; a 4 MiB resource file took minutes to parse. Android reconstruct also rescanned the whole file once per dotted key to identify `` members (2.7 s → 63 ms on a 3.6 MiB, 52,000-entry file). +- **formats**: Android XML self-closing elements (``) are no longer deleted together with the element that follows them. +- **formats**: Android XML values whose CDATA body contains `` are no longer truncated on extract, and plural `` attributes are preserved when a translation is written back. +- **formats**: XLIFF files with a CDATA section in a `` between `` and `` are no longer rejected; only CDATA inside `` / `` is unsupported. +- **hooks**: `deepl hooks install` now resolves the hooks directory git actually reads (`git rev-parse --git-path hooks`), so it honours `core.hooksPath` (husky) and works inside linked worktrees and submodules where `.git` is a pointer file. Previously it reported success while writing a hook git never ran, and crashed with a raw `ENOTDIR` on worktrees and submodules. +- **hooks**: `deepl hooks install` no longer overwrites an existing hook backup. A repeat install writes to the next free `.backup` slot, and the install output now prints the hook path and the backup path. `findGitRoot` also no longer loops forever when given a relative start path. +- **sync**: Auto-glossary sync (`translation.glossary: auto`) skips terms whose source or translation is empty or contains a tab, carriage return, or newline. Such terms were uploaded as corrupted or outright wrong glossary entries, which DeepL then applied to live translations. An unchanged dictionary is no longer re-uploaded on every run (entries were compared against a lossy TSV round trip that could never compare equal), and a glossary failure for one locale no longer ends glossary sync for the remaining locales — the warning now names the locale, glossary, and cause. +- **glossary**: `deepl glossary add-entry` / `update-entry` reject terms containing a tab, carriage return, or newline instead of shifting every following column of the uploaded dictionary, and glossary import picks the TSV or CSV dialect once per file, so a quoted CSV field containing a tab is no longer split into garbage columns. +- **sync**: `deepl sync audit` no longer uses the lock file's source hash as a stand-in for a translation. Divergent translations were reported as consistent and identical ones as an inconsistency displaying a hex hash. Targets that cannot be read are now listed separately as missing — a new additive `missingTargets` field in the `sync audit --format json` output. +- **sync**: TMS request URLs are built with the URL API: a trailing slash on `server:` no longer produces a doubled separator, a base path is preserved, and a URL with a query string or fragment is rejected instead of silently truncating the API path. TMS timeouts now exit with the network-error code (5) instead of 1, TMS error messages redact credentials embedded in the server URL, and `deepl sync pull` enforces a 32 MiB cap on the response body while reading it, instead of after the whole payload had been parsed. +- **api**: Non-idempotent POST requests are no longer re-submitted after a client-side timeout. Every failure short of a 4xx reached the retry loop for all HTTP methods, so a batch or document upload that outlived the 30 s timeout was silently re-sent up to three more times — each already accepted and billed server-side (confirmed with server-side request counts: 4 per timed-out translate and upload), with the worst case being duplicate admin API keys whose secret is returned only once. Automatic retry is now restricted to idempotent methods (GET, HEAD, PUT, DELETE); a POST is replayed only on an error that proves the request never reached the server (`ECONNREFUSED`, `ENOTFOUND`, `EAI_AGAIN`). A 429 is still retried for every method, honoring `Retry-After`. +- **api**: A client-side timeout now exits 5 (network error) instead of 6 (invalid input), matching the documented exit-code contract. Error classification substring-matched the message and missed axios's `timeout of 30000ms exceeded` (`ECONNABORTED` and `ERR_CANCELED` were absent entirely), falling through to `ValidationError` — so CI that retries on 5 and hard-fails on 6 did exactly the wrong thing on a flaky network. Classification now branches on `error.code` and the absence of a response. An HTTP 401 is likewise mapped to `AuthError` (exit 2) instead of falling through to exit 6. +- **api**: Retries run under an overall time budget rather than only a per-attempt timeout — twice the request timeout by default — so a never-responding server no longer holds a single command for two minutes (measured 125 s before). Honest `Retry-After` waits are not charged against the budget. +- **api**: The Trace ID quoted in an error message now belongs to the request that failed rather than the client's last-seen response, so concurrent requests no longer cross-quote each other's Trace IDs. +- **api**: Errors already classified by the API client are no longer re-classified when a client wraps its own error handling, which could turn a validation error into a network error and drop its recovery hint. Error messages also no longer print a doubled `Network error: Network error:` prefix. +- **api**: The document translation result endpoint is never retried — the download is effectively single-use, so a retry after a timeout on a large file could permanently lose an already-billed translation — and document transfers get their own larger timeout. +- **voice**: `deepl voice` now actually reconnects after a transport failure. The socket `error` handler marked the stream ended before the `close` event that always follows arrived, so the reconnect path (up to 3 attempts, `--reconnect` on by default) was unreachable for a real network drop — only a clean remote close ever reconnected. A reconnect that exhausts its attempts now closes the audio input instead of leaving the stream generator awaiting forever. ### Security +- **translate/sync**: Placeholder restoration no longer hangs the CLI with unbounded memory growth. `restorePlaceholders` looped `while (restored.includes(placeholder))`, replacing one occurrence per pass — so when the preserved original itself contained the placeholder token, every pass re-inserted it and the guard never went false. Measured before the fix: the input `{__VAR_0__}` grew from 9 to 400,009 bytes across 200,000 iterations without converging, and the real function had to be killed. It needed no attacker and no network: `preserveVariables`' pattern matches `{__VAR_0__}` (underscores and digits are in its character class), variable preservation runs unconditionally, and restoration also runs on **cached** results — so a locale value of that shape hung the process with no API call. Each placeholder is now restored in a single pass, using the function form of the replacement so `$&`/`$1` inside a preserved value stay literal. +- **sync**: Bucket `include` globs can no longer escape the project root, and `--dry-run` no longer modifies the working tree. `include` entries were validated only as non-empty strings, while `target_path_pattern` a few lines later already rejected `..`. The unvalidated glob's literal prefix was resolved and handed to the stale-`.bak` sweep, which recursed with **no containment check** — deleting every old `*.bak` it found and *re-creating* any file whose `.bak` existed while the live file was missing or empty. Verified: `include: "../../../../../../**/*.json"` produced a sweep root of `/var`, an out-of-root `.bak` was deleted and its sibling resurrected with the backup's contents. Two things made it worse: the sweep was gated only on watch runs, so it ran under `--dry-run` — the flag a cautious user reaches for to avoid side effects — and its errors were swallowed entirely, so it was silent. `include` entries are now rejected at config load for traversal segments and absolute paths (the check the `sync init` wizard already applied, which simply did not exist on the load path), the sweep independently refuses any root outside the project and logs the attempt, the sweep is skipped under `--dry-run`, and its failures are reported instead of discarded. - **deps**: Resolved four production-dependency advisories flagged by `npm audit --omit=dev` via lockfile-only transitive bumps (no `package.json` ranges changed): `brace-expansion` 5.0.5 → 5.0.6 (GHSA-jxxr-4gwj-5jf2, ReDoS), `form-data` → 4.0.6 (GHSA-hmw2-7cc7-3qxx, CRLF injection), and `ws` 8.20.0 → 8.21.0 (GHSA-58qx-3vcg-4xpx and GHSA-96hv-2xvq-fx4p, uninitialized-memory disclosure and memory-exhaustion DoS). Production `npm audit` is back to zero vulnerabilities, unblocking the CI audit gate. - **deps**: Resolved two further `brace-expansion` denial-of-service advisories disclosed after the bump above: `brace-expansion` 5.0.6 → 5.0.8 (GHSA-3jxr-9vmj-r5cp, exponential-time expansion of consecutive non-expanding `{}` groups; GHSA-mh99-v99m-4gvg, unbounded expansion length causing an out-of-memory crash). Reached in production through `minimatch`, which accepts `^5.0.5`, so this is again a lockfile-only bump with no `package.json` range changes. Dev-tree instances of the same advisories are intentionally left in place: npm's proposed remediation downgrades `jest` 30 → 25 and `ts-jest` 29 → 27, `devDependencies` are not installed by consumers, and the CI audit gate is production-only. - +- **formats**: YAML alias expansion in the i18n format parser is now bounded by a node-expansion budget and an alias nesting-depth limit. Documents whose anchors and aliases expand exponentially, or whose anchors reference themselves, previously hung `deepl sync` indefinitely; they now fail fast with a `YAML parse error`. +- **formats**: Android XML translations containing `]]>` are refused instead of being written into a CDATA section, where they closed the section and had their remainder parsed as XML — injecting elements into generated resource files. This was reachable without a malicious API response, since a failed translation is written through verbatim and the source file is used as the template for a missing target locale. ## [1.2.0] - 2026-04-25 ### Added @@ -159,7 +176,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `deepl tm list` subcommand — lists all translation memories on the account, mirroring `deepl glossary list`. Text output filters control chars and zero-width codepoints from TM names so a malicious API-returned name cannot corrupt the terminal; `--format json` emits the raw `TranslationMemory[]` as returned by `GET /v3/translation_memories`. Help text on `deepl translate --translation-memory` now cross-references the new command - `src/utils/uuid.ts` — shared strict UUID regex (`UUID_RE`) + `validateUuid` / `validateTranslationMemoryId` helpers. `validateTranslationMemoryId` is dormant today (TM IDs only appear in `/v2/translate` POST bodies, which are JSON-escaped) but guards the path-injection surface the moment any future per-TM endpoint interpolates a user-supplied UUID into a URL segment - **sync**: `deepl sync resolve` now prints a per-entry decision report (`kept ours` / `kept theirs` / `length-heuristic` / `unresolved`) plus a summary, and accepts `--dry-run` to preview decisions without writing the lockfile. -- **sync docs**: `docs/SYNC.md` Exit Codes table and `docs/API.md` sync Behavior bullet now cross-link to the canonical [Exit Codes](API.md#exit-codes) appendix. +- **sync docs**: `docs/SYNC.md` Exit Codes table and `docs/API.md` sync Behavior bullet now cross-link to the canonical [Exit Codes](docs/API.md#exit-codes) appendix. - **sync**: New `sync.max_scan_files` config key (default 50,000). - **errors**: `SyncConflictError` class in `src/utils/errors.ts` mirroring `SyncDriftError` — `ExitCode.SyncConflict` (11) is now throwable as a typed error so library consumers can `instanceof`-match the conflict case. - **SECURITY.md**: `1.1.x` row added to the Supported Versions table. diff --git a/README.md b/README.md index 5928420..69bfc5b 100644 --- a/README.md +++ b/README.md @@ -650,9 +650,13 @@ deepl write "Fresh improvement please." --lang en-US --no-cache - Spanish (`es`) - French (`fr`) - Italian (`it`) +- Japanese (`ja`) +- Korean (`ko`) - Portuguese (`pt`) - generic, defaults to Brazilian Portuguese - Portuguese - Brazilian (`pt-BR`) - Portuguese - European (`pt-PT`) +- Chinese (`zh`) - generic, defaults to Simplified Chinese +- Chinese - Simplified (`zh-Hans`) **Writing Styles:** @@ -1043,22 +1047,24 @@ DeepL CLI includes built-in retry logic and timeout handling for robust API comm **Automatic Retry Logic:** -- Automatically retries failed requests on transient errors (5xx, network failures) -- Default: 3 retries with exponential backoff -- Does not retry on client errors (4xx - bad request, auth failures, etc.) -- Exponential backoff delays: 1s, 2s, 4s, 8s, 10s (capped at 10s) +- Idempotent requests (GET, HEAD, PUT, DELETE) are retried on 5xx responses and transient network failures +- Non-idempotent requests (POST — translations, uploads, glossary and key creation) are replayed only when the error proves the request never reached the server (connection refused, DNS failure), so a slow response can never be submitted and billed twice +- Rate limiting (429) is retried for all methods, honoring the `Retry-After` header when the server sends one +- Other client errors (4xx — bad request, auth failures, etc.) are not retried +- Default: 3 retries with full-jitter exponential backoff (capped at 10s) **Timeout Configuration:** -- Default timeout: 30 seconds per request +- Default timeout: 30 seconds per request, with an overall time budget across retries - Applies to all API requests (translate, usage, languages, etc.) +- Override per invocation with the global `--timeout ` and `--max-retries ` flags **Features:** - ✅ Automatic retry on transient failures - ✅ Exponential backoff to avoid overwhelming the API -- ✅ Smart error detection (retries 5xx, not 4xx) -- ✅ Configurable via library-consumer options; not exposed as a CLI flag +- ✅ Retry policy aware of request idempotency +- ✅ Configurable via `--timeout` / `--max-retries` (or library-consumer options) - ✅ Works across all DeepL API endpoints **Retry Behavior Examples:** @@ -1072,11 +1078,12 @@ deepl translate "Hello" --to es deepl translate "Hello" --to es # Fails immediately without retries on auth errors -# Rate limiting (429) - does not retry -# You may want to wait before retrying manually +# Rate limiting (429) - retried automatically +# Honors the Retry-After header when the server sends one, +# otherwise falls back to jittered exponential backoff ``` -**Note:** Retry and timeout settings use sensible defaults optimized for the DeepL API. These are internal features that work automatically - no configuration required. +**Note:** Retry and timeout settings use sensible defaults optimized for the DeepL API. No configuration is required; `--timeout` and `--max-retries` are available when a specific invocation needs different bounds (e.g. very large documents on a slow uplink). ### Glossaries @@ -1331,7 +1338,7 @@ Cache location: `~/.cache/deepl-cli/cache.db` (or `~/.deepl-cli/cache.db` for le ### Prerequisites -- Node.js >= 20.19.0 +- Node.js >= 24.0.0 - npm >= 9.0.0 - DeepL API key diff --git a/docs/API.md b/docs/API.md index f7a6aea..3caa1d5 100644 --- a/docs/API.md +++ b/docs/API.md @@ -1,7 +1,7 @@ # DeepL CLI - API Reference -**Version**: 1.2.0 -**Last Updated**: April 25, 2026 +**Version**: 2.0.0 +**Last Updated**: July 29, 2026 Complete reference for all DeepL CLI commands, options, and configuration. @@ -52,8 +52,12 @@ Options that work with all commands: --verbose, -v Show extra information (source language, timing, cache status) --config, -c FILE Use alternate configuration file --no-input Disable all interactive prompts (abort instead of prompting) +--timeout MS HTTP request timeout in milliseconds (default: 30000) +--max-retries N Maximum automatic retries for retryable requests (default: 3) ``` +Automatic retries apply to idempotent requests, and to rate-limited (429) responses for every method; a request that submits work is otherwise never replayed. Retries share a wall-clock budget of twice `--timeout`, so raising `--max-retries` alone does not extend how long the CLI waits on an unresponsive endpoint. + **Examples:** ```bash @@ -78,6 +82,9 @@ deepl --quiet translate docs/ --to es --output docs-es/ # Use custom config file deepl --config ~/.deepl-work.json translate "Hello" --to es +# Give a large document more time, and disable automatic retries +deepl --timeout 120000 --max-retries 0 translate report.pdf --to de + # Use custom config directory (via environment variable) export DEEPL_CONFIG_DIR=/path/to/config deepl translate "Hello" --to es @@ -730,9 +737,13 @@ When `--style` or `--tone` is set for a target language that does not support it - `es` - Spanish - `fr` - French - `it` - Italian +- `ja` - Japanese +- `ko` - Korean - `pt` - Portuguese (generic, defaults to Brazilian Portuguese) - `pt-BR` - Brazilian Portuguese - `pt-PT` - European Portuguese +- `zh` - Chinese (generic, defaults to Simplified Chinese) +- `zh-Hans` - Simplified Chinese #### Examples @@ -1134,7 +1145,7 @@ Interactive setup wizard that creates `.deepl-sync.yaml` by scanning the project - `--path GLOB` - Source file path or glob pattern - `--sync-config PATH` - Path to `.deepl-sync.yaml` -`--source-lang` and `--target-langs` are accepted as deprecated aliases for one minor release and emit a stderr warning; they will be removed in the next major release. `deepl translate --target-lang` is unchanged — it operates on strings and stays aligned with the DeepL API's wire name. +`--source-lang` and `--target-langs` were accepted as deprecated aliases during `1.x` and were removed in `2.0.0`; use `--source-locale` / `--target-locales`. `deepl translate --target-lang` is unchanged — it operates on strings and stays aligned with the DeepL API's wire name. **Examples:** @@ -3071,7 +3082,7 @@ Unclassified failure: emitted when an error escapes every typed handler and matc Authentication failed or no API key is available. Emitted by: -- `deepl auth set-key`, `deepl auth test` when the key cannot be validated +- `deepl auth set-key` when the key cannot be validated - Every command that touches the API (`translate`, `write`, `voice`, `glossary`, `usage`, `sync`, `tm list`, `admin`, etc.) when `DEEPL_API_KEY` is unset and no key is in the config file - HTTP 401/403 responses from the DeepL API @@ -3113,7 +3124,7 @@ Remediation: re-read the command's `--help` and the relevant section of this API The configuration file or a configuration value is invalid. Emitted by: - `deepl config set` with a key that is not in the schema, or a value that fails validation (invalid language code, invalid formality, invalid output format, non-positive cache size, non-HTTPS `baseUrl`, path-traversal attempts) -- `deepl config get/unset` with a malformed key +- `deepl config get` with a malformed key - Any command that loads the config file when the file fails to parse, is missing a required field, or specifies an unsupported version - `sync` when `.deepl-sync.yaml` is missing required fields, has invalid locales, or declares an unsupported version - `sync push` / `sync pull` when the remote TMS returns 401/403 (surfaced as `ConfigError` with a hint to check `TMS_API_KEY` / `TMS_TOKEN` and the relevant YAML fields) @@ -3230,5 +3241,5 @@ deepl write --check README.md --- -**Last Updated**: April 20, 2026 -**DeepL CLI Version**: 1.1.0 +**Last Updated**: July 29, 2026 +**DeepL CLI Version**: 2.0.0 diff --git a/docs/SYNC.md b/docs/SYNC.md index 066c9ee..1e1f94e 100644 --- a/docs/SYNC.md +++ b/docs/SYNC.md @@ -476,7 +476,7 @@ deepl sync init [OPTIONS] | `--path ` | Source file path or glob pattern | | `--sync-config ` | Path to `.deepl-sync.yaml` (default: auto-detect) | -`--source-lang` and `--target-langs` remain accepted as deprecated aliases for one minor release and emit a stderr deprecation warning; they will be removed in the next major release. The `--locale` filter on `sync push` / `pull` / `status` / `export` is unchanged. `deepl translate --target-lang` is unchanged — it operates on strings and stays aligned with the DeepL API's wire name. +`--source-lang` and `--target-langs` were accepted as deprecated aliases during `1.x` and were removed in `2.0.0`; use `--source-locale` and `--target-locales`. The `--locale` filter on `sync push` / `pull` / `status` / `export` is unchanged. `deepl translate --target-lang` is unchanged — it operates on strings and stays aligned with the DeepL API's wire name. **Examples:** @@ -793,7 +793,7 @@ Each target locale's approved dictionary is fetched exactly once per `sync pull` ### Worked example -`deepl sync init` currently accepts `--source-lang` and `--target-langs` as deprecated aliases for `--source-locale` and `--target-locales` (shipped via commit `d65cbb3`). Both aliases emit a stderr deprecation warning pointing at the replacement flag. Under the policy above, these aliases will be removed at the `2.0` cut — this is confirmed, and the removal will not slip into any `1.x` release. See the [`deepl sync init`](#deepl-sync-init) section for the current accepted synopsis. +`deepl sync init` accepted `--source-lang` and `--target-langs` as deprecated aliases for `--source-locale` and `--target-locales` throughout `1.x`; both emitted a stderr deprecation warning pointing at the replacement flag. Per the policy above, the aliases were removed at the `2.0` cut — since `2.0.0` they fail as unknown options. See the [`deepl sync init`](#deepl-sync-init) section for the current accepted synopsis. ## CI/CD Integration @@ -861,7 +861,7 @@ The `.deepl-sync.lock` file is only staged when this sync run actually wrote an ```yaml i18n-check: stage: test - image: node:20 + image: node:24 script: - npm install -g @deepl/cli - deepl sync --frozen diff --git a/docs/TROUBLESHOOTING.md b/docs/TROUBLESHOOTING.md index 8cc995f..ccd4696 100644 --- a/docs/TROUBLESHOOTING.md +++ b/docs/TROUBLESHOOTING.md @@ -139,7 +139,7 @@ Common issues and solutions when using the DeepL CLI. 2. For batch/directory translation, the CLI uses concurrency control internally. Avoid running multiple CLI instances simultaneously on the same API key. -3. The CLI automatically retries with exponential backoff (1s, 2s, 4s, up to 10s, max 3 retries). If errors persist, wait and try again. +3. Retries honor the server's `Retry-After` header when present, falling back to full-jitter exponential backoff (max 3 retries by default; see the global `--max-retries` flag). If errors persist, wait and try again. --- @@ -167,7 +167,7 @@ Common issues and solutions when using the DeepL CLI. export HTTP_PROXY=http://proxy.example.com:8080 ``` -4. The CLI retries on transient network errors automatically with exponential backoff. +4. The CLI retries idempotent requests on transient network errors automatically. Requests that submit work (translations, document uploads, glossary creation) are replayed only when the error proves the request never reached the server, so they are never double-billed — for those, rerun the command (exit code 5 is safe to retry at the script level). ### "Request failed with status code 503" @@ -183,7 +183,7 @@ Common issues and solutions when using the DeepL CLI. deepl translate "Hello" --to es --no-cache ``` -3. The CLI automatically retries on 503 errors with exponential backoff. If the error persists, the API may be experiencing an extended outage. +3. Read-only requests are retried automatically on 503. Requests that submit work (translate, document upload) surface the error immediately so they are never double-billed; retry them at the script level (exit code 5). If the error persists, the API may be experiencing an extended outage. --- diff --git a/src/api/document-client.ts b/src/api/document-client.ts index f50e240..6849094 100644 --- a/src/api/document-client.ts +++ b/src/api/document-client.ts @@ -16,11 +16,19 @@ interface DeepLDocumentStatusResponse { error_message?: string; } +/** Uploads and downloads move whole files, so they need far more headroom + * than the interactive request timeout. */ +const TRANSFER_TIMEOUT_MS = 300_000; + export class DocumentClient extends HttpClient { constructor(apiKey: string, options: DeepLClientOptions = {}) { super(apiKey, options); } + private get transferTimeout(): number { + return Math.max(this.requestTimeout, TRANSFER_TIMEOUT_MS); + } + async uploadDocument( file: Buffer, options: DocumentTranslationOptions @@ -69,7 +77,8 @@ export class DocumentClient extends HttpClient { ...formData.getHeaders(), }, }; - } + }, + { timeout: this.transferTimeout } ); return { @@ -101,6 +110,11 @@ export class DocumentClient extends HttpClient { } } + /** + * The result endpoint is effectively single-use: a replayed download can + * consume the result of an already-billed translation and lose it + * permanently, so this request is never retried. + */ async downloadDocument(handle: DocumentHandle): Promise { try { const response = await this.makeRawRequest( @@ -116,7 +130,8 @@ export class DocumentClient extends HttpClient { }, responseType: 'arraybuffer', }; - } + }, + { maxRetries: 0, timeout: this.transferTimeout } ); return Buffer.from(response); diff --git a/src/api/http-client.ts b/src/api/http-client.ts index b323ed5..61d5b68 100644 --- a/src/api/http-client.ts +++ b/src/api/http-client.ts @@ -8,11 +8,13 @@ import { QuotaError, NetworkError, ConfigError, + DeepLCLIError, ValidationError, } from '../utils/errors.js'; import { Logger } from '../utils/logger.js'; import { errorMessage } from '../utils/error-message.js'; import { sanitizeForTerminal } from '../utils/control-chars.js'; +import { sanitizeUrl } from '../utils/sanitize-url.js'; import { VERSION } from '../version.js'; export const USER_AGENT = `deepl-cli/${VERSION} node/${process.versions.node}`; @@ -31,22 +33,14 @@ export interface DeepLClientOptions { usePro?: boolean; timeout?: number; maxRetries?: number; + /** Wall-clock budget for all attempts of one request, excluding backoff + * sleeps. Defaults to `timeout * 2`. */ + totalTimeout?: number; baseUrl?: string; proxy?: ProxyConfig; } -export function sanitizeUrl(url: string): string { - try { - const parsed = new URL(url); - if (parsed.username || parsed.password) { - parsed.username = '***'; - parsed.password = '***'; - } - return parsed.toString(); - } catch { - return '[invalid URL]'; - } -} +export { sanitizeUrl }; export const FREE_API_URL = 'https://api-free.deepl.com'; export const PRO_API_URL = 'https://api.deepl.com'; @@ -58,6 +52,44 @@ const KEEP_ALIVE_MSECS = 1000; const RETRY_INITIAL_DELAY_MS = 1000; const RETRY_MAX_DELAY_MS = 10000; const RETRY_AFTER_MAX_SECONDS = 60; +const TOTAL_TIMEOUT_FACTOR = 2; + +/** + * Methods a failed attempt may be replayed on. A POST is excluded because a + * client-side abort (timeout, mid-flight reset) says nothing about whether + * the server already accepted — and billed — the request; replaying it + * duplicates the work (a second translation, a second uploaded document, a + * second API key whose secret is only returned once). + */ +const IDEMPOTENT_METHODS = new Set([ + 'GET', + 'HEAD', + 'PUT', + 'DELETE', + 'OPTIONS', + 'TRACE', +]); + +/** Transport errors that prove the request never reached the server, so even + * a non-idempotent request is safe to replay. */ +const UNSENT_REQUEST_CODES = new Set([ + 'ECONNREFUSED', + 'ENOTFOUND', + 'EAI_AGAIN', +]); + +/** Axios codes for a request the client itself gave up on. */ +const CLIENT_ABORT_CODES = new Set([ + 'ECONNABORTED', + 'ETIMEDOUT', + 'ERR_CANCELED', +]); + +/** Per-request overrides for the retry policy and timeouts. */ +export interface RequestPolicy { + maxRetries?: number; + timeout?: number; +} /** * Compute a retry delay for attempt `n` with full jitter: a uniform @@ -73,9 +105,19 @@ export function computeBackoffWithJitter(attempt: number): number { return Math.floor(Math.random() * cap); } +/** Prefixes a transport failure without stuttering when the underlying + * message already carries the label. */ +function prefixNetwork(detail: string, label: string): string { + return /^network (error|timeout)\b/i.test(detail) + ? detail + : `${label}: ${detail}`; +} + export class HttpClient { protected client: AxiosInstance; protected maxRetries: number; + protected requestTimeout: number; + protected totalTimeout: number; protected _lastTraceId?: string; private static parseProxyFromEnv(): ProxyConfig | undefined { @@ -133,6 +175,9 @@ export class HttpClient { options.baseUrl ?? (options.usePro ? PRO_API_URL : FREE_API_URL); this.maxRetries = options.maxRetries ?? DEFAULT_MAX_RETRIES; + this.requestTimeout = options.timeout ?? DEFAULT_TIMEOUT; + this.totalTimeout = + options.totalTimeout ?? this.requestTimeout * TOTAL_TIMEOUT_FACTOR; const axiosConfig: Record = { baseURL, @@ -266,54 +311,66 @@ export class HttpClient { protected async makeRawRequest( method: 'GET' | 'POST' | 'PUT' | 'PATCH' | 'DELETE', path: string, - buildConfig: () => Record + buildConfig: () => Record, + policy?: RequestPolicy ): Promise { - return this.executeWithRetry(method, path, buildConfig); + return this.executeWithRetry(method, path, buildConfig, policy); } protected async executeWithRetry( method: string, path: string, - buildConfig: () => Record + buildConfig: () => Record, + policy?: RequestPolicy ): Promise { + const maxRetries = policy?.maxRetries ?? this.maxRetries; + const requestTimeout = policy?.timeout ?? this.requestTimeout; + // The budget covers time spent inside attempts only; backoff and + // Retry-After sleeps are excluded, so an honest rate-limit wait is not + // charged against the deadline. + let remainingBudget = Math.max(this.totalTimeout, requestTimeout); let lastError: Error | undefined; + let traceId: string | undefined; - for (let attempt = 0; attempt <= this.maxRetries; attempt++) { + for (let attempt = 0; attempt <= maxRetries; attempt++) { + const requestStart = Date.now(); try { const config = buildConfig(); - const requestStart = Date.now(); const response = await this.client.request({ method, url: path, ...config, + timeout: Math.min(requestTimeout, remainingBudget), }); const requestElapsed = Date.now() - requestStart; Logger.verbose( `[verbose] HTTP ${method} ${path} completed in ${requestElapsed}ms (status ${response.status})` ); - const traceId = response.headers?.['x-trace-id'] as string | undefined; - if (traceId) { - this._lastTraceId = traceId; + const responseTraceId = response.headers?.['x-trace-id'] as + | string + | undefined; + if (responseTraceId) { + this._lastTraceId = responseTraceId; } return response.data; } catch (error) { + remainingBudget -= Date.now() - requestStart; lastError = error as Error; if (this.isAxiosError(error)) { - const traceId = error.response?.headers?.['x-trace-id'] as + const responseTraceId = error.response?.headers?.['x-trace-id'] as | string | undefined; - if (traceId) { - this._lastTraceId = traceId; + if (responseTraceId) { + traceId = responseTraceId; + this._lastTraceId = responseTraceId; } - } - if (this.isAxiosError(error)) { const status = error.response?.status; - if (status === 429 && attempt < this.maxRetries) { + if (status === 429 && attempt < maxRetries) { const retryAfterDelay = this.parseRetryAfter( error.response?.headers?.['retry-after'] as string | undefined ); @@ -324,43 +381,78 @@ export class HttpClient { const delay = retryAfterDelay ?? computeBackoffWithJitter(attempt); Logger.verbose( - `[verbose] HTTP ${method} ${path} retry ${attempt + 1}/${this.maxRetries} in ${delay}ms (status 429${retryAfterDelay !== null && retryAfterDelay !== undefined ? ', Retry-After' : ', jitter backoff'})` + `[verbose] HTTP ${method} ${path} retry ${attempt + 1}/${maxRetries} in ${delay}ms (status 429${retryAfterDelay !== null && retryAfterDelay !== undefined ? ', Retry-After' : ', jitter backoff'})` ); await this.sleep(delay); continue; } if (status && status >= 400 && status < 500) { - throw this.handleError(error); + throw this.handleError(error, undefined, traceId); } } - if (attempt < this.maxRetries) { + if ( + attempt < maxRetries && + remainingBudget > 0 && + this.isReplayable(method, error) + ) { const delay = computeBackoffWithJitter(attempt); const status = this.isAxiosError(error) ? error.response?.status : undefined; Logger.verbose( - `[verbose] HTTP ${method} ${path} retry ${attempt + 1}/${this.maxRetries} in ${delay}ms (${status ? `status ${status}` : 'network error'}, jitter backoff)` + `[verbose] HTTP ${method} ${path} retry ${attempt + 1}/${maxRetries} in ${delay}ms (${status ? `status ${status}` : 'network error'}, jitter backoff)` ); await this.sleep(delay); + continue; } + + break; } } throw lastError - ? this.handleError(lastError) + ? this.handleError(lastError, undefined, traceId) : new NetworkError('Request failed after retries'); } - protected handleError(error: unknown, context?: string): Error { - const result = this.classifyError(error); + /** + * Whether a failed attempt may be sent again. 4xx responses never reach + * here (they throw immediately) and 429 is handled above, so the cases + * left are 5xx responses and transport failures. + */ + private isReplayable(method: string, error: unknown): boolean { + if (!this.isAxiosError(error)) { + return false; + } + if (!error.response && error.code && UNSENT_REQUEST_CODES.has(error.code)) { + return true; + } + return IDEMPOTENT_METHODS.has(method.toUpperCase()); + } + + protected handleError( + error: unknown, + context?: string, + traceId?: string + ): Error { + const result = this.classifyError(error, traceId); if (context) { result.message = `${result.message} [${context}]`; } return result; } - private classifyError(error: unknown): Error { - const traceIdSuffix = this._lastTraceId - ? ` (Trace ID: ${this._lastTraceId})` + private classifyError(error: unknown, traceId?: string): Error { + // Classification is idempotent: callers that wrap their own catch in + // handleError (write-client, document-client) hand back errors this + // method already produced, and re-deriving a class from them would + // discard the specific one. + if (error instanceof DeepLCLIError) { + return error; + } + + const requestTraceId = traceId ?? this._lastTraceId; + const traceIdSuffix = requestTraceId + ? ` (Trace ID: ${requestTraceId})` : ''; if (this.isAxiosError(error)) { @@ -377,6 +469,10 @@ export class HttpClient { const message = sanitizeForTerminal(responseData?.message ?? error.message ?? ''); switch (status) { + case 401: + return new AuthError( + `Authentication failed: Invalid or missing API key${traceIdSuffix}` + ); case 403: return new AuthError( `Authentication failed: Invalid API key${traceIdSuffix}` @@ -399,8 +495,14 @@ export class HttpClient { `Server error (${status}): ${message}${traceIdSuffix}` ); } - if (!error.response && this.isNetworkLevelError(error)) { - return new NetworkError(`Network error: ${error.message}`); + // No response at all means the request never completed a round + // trip: a refused connection, a DNS failure, a reset socket, or a + // client-side abort. All of those are network conditions, and the + // axios `code` is the reliable signal — the message is not (a + // timeout reads "timeout of 30000ms exceeded" and matches no + // substring list). + if (!error.response) { + return this.transportError(error); } return new ValidationError(`API error: ${message}${traceIdSuffix}`); } @@ -408,7 +510,7 @@ export class HttpClient { if (error instanceof Error) { if (this.isNetworkLevelError(error)) { - return new NetworkError(`Network error: ${error.message}`); + return new NetworkError(prefixNetwork(error.message, 'Network error')); } return error; } @@ -416,6 +518,15 @@ export class HttpClient { return new NetworkError('Unknown error occurred'); } + private transportError(error: AxiosError): NetworkError { + const detail = sanitizeForTerminal(error.message ?? ''); + const label = + error.code && CLIENT_ABORT_CODES.has(error.code) + ? 'Network timeout' + : 'Network error'; + return new NetworkError(prefixNetwork(detail, label)); + } + private isNetworkLevelError(error: Error): boolean { const msg = error.message.toLowerCase(); return ( diff --git a/src/api/voice-client.ts b/src/api/voice-client.ts index 6530b6f..d19600e 100644 --- a/src/api/voice-client.ts +++ b/src/api/voice-client.ts @@ -99,15 +99,10 @@ export class VoiceClient extends HttpClient { this.dispatchMessage(message, callbacks); }); - ws.on('error', (error: Error) => { - callbacks.onError?.({ - request_type: 'unknown', - error_code: 0, - reason_code: 0, - error_message: error.message, - }); - }); - + // Transport errors stay on the socket: `callbacks.onError` carries + // server error messages only, so a dropped connection is + // distinguishable from a protocol error, and the socket keeps a single + // 'error' listener owned by whoever drives its lifecycle. return ws; } diff --git a/src/cli/cache-loader.ts b/src/cli/cache-loader.ts index 0d58030..3dbbf7c 100644 --- a/src/cli/cache-loader.ts +++ b/src/cli/cache-loader.ts @@ -7,12 +7,29 @@ */ import type { CacheService, CacheServiceOptions } from '../storage/cache.js'; +import type { ConfigService } from '../storage/config.js'; import { Logger } from '../utils/logger.js'; import { errorMessage } from '../utils/error-message.js'; import { isNativeModuleLoadError } from '../utils/native-module-error.js'; type CacheModule = Pick; +/** + * Derive cache-service options from persisted config. Reading `cache.enabled` + * here is what keeps the service's in-memory flag from diverging from config: + * `cache stats` and the translation path then agree on the same value. + */ +export function resolveCacheOptions(config: ConfigService, dbPath: string): CacheServiceOptions { + const ttlSeconds = config.getValue('cache.ttl'); + return { + dbPath, + // Config TTL is in seconds, CacheService expects milliseconds + ttl: ttlSeconds !== undefined ? ttlSeconds * 1000 : undefined, + maxSize: config.getValue('cache.maxSize'), + enabled: config.getValue('cache.enabled'), + }; +} + export function createCacheServiceGetter( getOptions: () => CacheServiceOptions, importCacheModule: () => Promise = () => import('../storage/cache.js'), diff --git a/src/cli/commands/cache.ts b/src/cli/commands/cache.ts index 22d6e67..d8567e8 100644 --- a/src/cli/commands/cache.ts +++ b/src/cli/commands/cache.ts @@ -39,23 +39,24 @@ export class CacheCommand { } /** - * Enable cache with optional max size + * Enable cache with optional max size. + * Persisted to config: the in-memory flag alone would die with this process. */ async enable(maxSize?: number): Promise { - // Set max size if provided if (maxSize !== undefined) { this.config.set('cache.maxSize', maxSize); this.cache.setMaxSize(maxSize); } - // Enable cache + this.config.set('cache.enabled', true); this.cache.enable(); } /** - * Disable cache + * Disable cache. Persisted to config, as with enable(). */ async disable(): Promise { + this.config.set('cache.enabled', false); this.cache.disable(); } diff --git a/src/cli/commands/hooks.ts b/src/cli/commands/hooks.ts index 40f9b8f..bdf9134 100644 --- a/src/cli/commands/hooks.ts +++ b/src/cli/commands/hooks.ts @@ -29,9 +29,17 @@ export class HooksCommand { throw new ValidationError('Not in a git repository. Run this command from within a git repository.'); } - this.gitHooksService.install(hookType); + const result = this.gitHooksService.install(hookType); - return chalk.green(`✓ Installed ${hookType} hook`); + const lines = [chalk.green(`✓ Installed ${hookType} hook`)]; + if (result?.hookPath) { + lines.push(chalk.gray(` Path: ${result.hookPath}`)); + } + if (result?.backupPath) { + lines.push(chalk.gray(` Previous hook backed up to: ${result.backupPath}`)); + } + + return lines.join('\n'); } /** diff --git a/src/cli/commands/sync/register-sync-audit.ts b/src/cli/commands/sync/register-sync-audit.ts index 5f9c39c..5167dbe 100644 --- a/src/cli/commands/sync/register-sync-audit.ts +++ b/src/cli/commands/sync/register-sync-audit.ts @@ -4,7 +4,7 @@ import { ValidationError } from '../../../utils/errors.js'; import type { ServiceDeps } from '../service-factory.js'; import type { TargetTranslationIndex } from '../../../sync/sync-glossary-report.js'; import { extractTranslatable } from '../../../sync/sync-bucket-walker.js'; -import { emitJsonErrorAndExit, resolveFormat } from './sync-options.js'; +import { emitJsonErrorAndExit, resolveFormat, resolveSyncConfig } from './sync-options.js'; interface AuditOptions { format?: string; @@ -65,7 +65,9 @@ async function handleSyncAudit( const pathMod = await import('path'); const fsMod = await import('fs'); - const config = await loadSyncConfig(process.cwd(), { configPath: options.syncConfig }); + const config = await loadSyncConfig(process.cwd(), { + configPath: resolveSyncConfig(options, command), + }); const lockPath = pathMod.join(config.projectRoot, LOCK_FILE_NAME); const lockManager = new SyncLockManager(lockPath); const lockFile = await lockManager.read(); @@ -93,7 +95,7 @@ async function handleSyncAudit( for (const entry of entries) keyMap.set(entry.key, entry.value); fileLocaleMap.set(locale, keyMap); } catch { - // Unreadable / unparseable target file — fall through to hash identity. + // Unreadable / unparseable target file — reported as a missing target. } } if (fileLocaleMap.size > 0) targetTranslations.set(relPath, fileLocaleMap); @@ -117,6 +119,15 @@ async function handleSyncAudit( Logger.info(` Files: ${inc.files.join(', ')}`); } } + + if (report.missingTargets.length > 0) { + Logger.info( + `\n${report.missingTargets.length} target(s) could not be read and were excluded from the comparison:`, + ); + for (const target of report.missingTargets) { + Logger.info(` ${target.filePath} [${target.locale}]`); + } + } } } catch (error) { if (options.format === 'json') { diff --git a/src/cli/commands/sync/register-sync-export.ts b/src/cli/commands/sync/register-sync-export.ts index 8cea483..7579760 100644 --- a/src/cli/commands/sync/register-sync-export.ts +++ b/src/cli/commands/sync/register-sync-export.ts @@ -2,7 +2,13 @@ import { Command, Option } from 'commander'; import { Logger } from '../../../utils/logger.js'; import { ValidationError } from '../../../utils/errors.js'; import type { ServiceDeps } from '../service-factory.js'; -import { emitJsonErrorAndExit, resolveFormat } from './sync-options.js'; +import { + emitJsonErrorAndExit, + parseLocaleFilter, + resolveFormat, + resolveLocale, + resolveSyncConfig, +} from './sync-options.js'; interface ExportOptions { locale?: string; @@ -47,9 +53,12 @@ async function handleSyncExport( const pathMod = await import('path'); const fsMod = await import('fs'); - const config = await loadSyncConfig(process.cwd(), { configPath: options.syncConfig }); + const localeFilter = parseLocaleFilter(resolveLocale(options, command)); + const config = await loadSyncConfig(process.cwd(), { + configPath: resolveSyncConfig(options, command), + localeFilter, + }); const registry = await createDefaultRegistry(); - const localeFilter = options.locale?.split(',').map((l: string) => l.trim()); const result = await exportTranslations(config, registry, { localeFilter }); if (options.output) { diff --git a/src/cli/commands/sync/register-sync-init.ts b/src/cli/commands/sync/register-sync-init.ts index 56ca5cc..6c08c4c 100644 --- a/src/cli/commands/sync/register-sync-init.ts +++ b/src/cli/commands/sync/register-sync-init.ts @@ -8,19 +8,13 @@ import { emitJsonErrorAndExit, emitJsonInitSuccessAndExit, resolveFormat, + resolveSyncConfig, } from './sync-options.js'; import { ExitCode } from '../../../utils/exit-codes.js'; -const SOURCE_LANG_DEPRECATION_WARNING = - '[deprecated] --source-lang is renamed to --source-locale and will be removed in the next major release. Please update your scripts.\n'; -const TARGET_LANGS_DEPRECATION_WARNING = - '[deprecated] --target-langs is renamed to --target-locales and will be removed in the next major release. Please update your scripts.\n'; - interface InitOptions { sourceLocale?: string; targetLocales?: string; - sourceLang?: string; - targetLangs?: string; fileFormat?: string; path?: string; syncConfig?: string; @@ -37,8 +31,6 @@ export function registerSyncInit( .description('Initialize .deepl-sync.yaml configuration') .option('--source-locale ', 'Source locale') .option('--target-locales ', 'Target locales (comma-separated)') - .addOption(new Option('--source-lang ').hideHelp()) - .addOption(new Option('--target-langs ').hideHelp()) .addOption( new Option('--file-format ', 'File format').choices([...SUPPORTED_FORMAT_KEYS]), ) @@ -50,17 +42,6 @@ export function registerSyncInit( .action((options: InitOptions, command: Command) => handleSyncInit(options, command, deps)); } -function applyDeprecationAliases(options: InitOptions): void { - if (options.sourceLang !== undefined && options.sourceLocale === undefined) { - options.sourceLocale = options.sourceLang; - process.stderr.write(SOURCE_LANG_DEPRECATION_WARNING); - } - if (options.targetLangs !== undefined && options.targetLocales === undefined) { - options.targetLocales = options.targetLangs; - process.stderr.write(TARGET_LANGS_DEPRECATION_WARNING); - } -} - interface InitSuccessPayload { configPath: string; sourceLocale: string; @@ -86,14 +67,18 @@ async function handleSyncInit( const { handleError } = deps; options.format = resolveFormat(options, command); try { - applyDeprecationAliases(options); - - const { configExists, detectI18nFiles, generateSyncConfig, writeSyncConfig } = + const { configExists, detectI18nFiles, generateSyncConfig, resolveInitConfigPath, writeSyncConfig } = await import('../../../sync/sync-init.js'); + const pathMod = await import('path'); - const cwd = process.cwd(); + const targetConfigPath = resolveInitConfigPath( + process.cwd(), + resolveSyncConfig(options, command), + ); + const cwd = pathMod.dirname(targetConfigPath); + const displayPath = pathMod.relative(process.cwd(), targetConfigPath) || targetConfigPath; - if (configExists(cwd)) { + if (configExists(targetConfigPath)) { if (options.format === 'json') { // Already-present config is not an error per se, but scripted // bootstrap flows need a non-ok envelope to branch on. @@ -101,16 +86,16 @@ async function handleSyncInit( ok: false, error: { code: 'ConfigError', - message: 'Config file .deepl-sync.yaml already exists.', + message: `Config file ${displayPath} already exists.`, suggestion: - 'Remove or rename the existing .deepl-sync.yaml, or edit it directly.', + 'Remove or rename the existing config file, or edit it directly.', }, exitCode: ExitCode.ConfigError, }; process.stderr.write(JSON.stringify(envelope) + '\n'); process.exit(ExitCode.ConfigError); } - Logger.warn(chalk.yellow('Config file .deepl-sync.yaml already exists.')); + Logger.warn(chalk.yellow(`Config file ${displayPath} already exists.`)); return; } @@ -133,7 +118,7 @@ async function handleSyncInit( format: options.fileFormat, pattern: options.path, }); - const configPath = await writeSyncConfig(cwd, content); + const configPath = await writeSyncConfig(targetConfigPath, content); emitInitSuccess(options.format, { configPath, sourceLocale: validated.sourceLocale, @@ -203,7 +188,7 @@ async function handleSyncInit( pattern: options.path ?? project.pattern, targetPathPattern: options.path ? undefined : project.targetPathPattern, }); - const configPath = await writeSyncConfig(cwd, content); + const configPath = await writeSyncConfig(targetConfigPath, content); emitInitSuccess(options.format, { configPath, sourceLocale, diff --git a/src/cli/commands/sync/register-sync-pull.ts b/src/cli/commands/sync/register-sync-pull.ts index 920a326..49a3844 100644 --- a/src/cli/commands/sync/register-sync-pull.ts +++ b/src/cli/commands/sync/register-sync-pull.ts @@ -2,7 +2,13 @@ import { Command, Option } from 'commander'; import { Logger } from '../../../utils/logger.js'; import { ConfigError } from '../../../utils/errors.js'; import type { ServiceDeps } from '../service-factory.js'; -import { emitJsonErrorAndExit, resolveFormat, resolveLocale } from './sync-options.js'; +import { + emitJsonErrorAndExit, + parseLocaleFilter, + resolveFormat, + resolveLocale, + resolveSyncConfig, +} from './sync-options.js'; interface PullOptions { locale?: string; @@ -40,6 +46,7 @@ full field list and REST contract. .action((options: PullOptions, command: Command) => { options.format = resolveFormat(options, command); options.locale = resolveLocale(options, command); + options.syncConfig = resolveSyncConfig(options, command); return handleSyncPull(options, deps.handleError); }); } @@ -55,7 +62,11 @@ export async function handleSyncPull( const { pullTranslations, formatSkippedSummary } = await import('../../../sync/sync-tms.js'); const { acquireSyncProcessLock } = await import('../../../sync/sync-process-lock.js'); - const config = await loadSyncConfig(process.cwd(), { configPath: options.syncConfig }); + const localeFilter = parseLocaleFilter(options.locale); + const config = await loadSyncConfig(process.cwd(), { + configPath: options.syncConfig, + localeFilter, + }); if (!config.tms?.enabled) { throw new ConfigError( 'TMS integration not configured', @@ -67,7 +78,6 @@ export async function handleSyncPull( try { const client = createTmsClient(config.tms); const registry = await createDefaultRegistry(); - const localeFilter = options.locale?.split(',').map((l: string) => l.trim()); const result = await pullTranslations(config, client, registry, { localeFilter }); if (options.format === 'json') { process.stdout.write(JSON.stringify({ ok: true, pulled: result.pulled, skipped: result.skipped }) + '\n'); diff --git a/src/cli/commands/sync/register-sync-push.ts b/src/cli/commands/sync/register-sync-push.ts index eae5d49..a4dfe27 100644 --- a/src/cli/commands/sync/register-sync-push.ts +++ b/src/cli/commands/sync/register-sync-push.ts @@ -2,7 +2,13 @@ import { Command, Option } from 'commander'; import { Logger } from '../../../utils/logger.js'; import { ConfigError } from '../../../utils/errors.js'; import type { ServiceDeps } from '../service-factory.js'; -import { emitJsonErrorAndExit, resolveFormat, resolveLocale } from './sync-options.js'; +import { + emitJsonErrorAndExit, + parseLocaleFilter, + resolveFormat, + resolveLocale, + resolveSyncConfig, +} from './sync-options.js'; interface PushOptions { locale?: string; @@ -52,7 +58,11 @@ async function handleSyncPush( const { createDefaultRegistry } = await import('../../../formats/index.js'); const { pushTranslations, formatSkippedSummary } = await import('../../../sync/sync-tms.js'); - const config = await loadSyncConfig(process.cwd(), { configPath: options.syncConfig }); + const localeFilter = parseLocaleFilter(resolveLocale(options, command)); + const config = await loadSyncConfig(process.cwd(), { + configPath: resolveSyncConfig(options, command), + localeFilter, + }); if (!config.tms?.enabled) { throw new ConfigError( 'TMS integration not configured', @@ -63,8 +73,6 @@ async function handleSyncPush( const client = createTmsClient(config.tms); const registry = await createDefaultRegistry(); - const locale = resolveLocale(options, command); - const localeFilter = locale?.split(',').map((l: string) => l.trim()); const result = await pushTranslations(config, client, registry, { localeFilter }); if (options.format === 'json') { process.stdout.write(JSON.stringify({ ok: true, pushed: result.pushed, skipped: result.skipped }) + '\n'); diff --git a/src/cli/commands/sync/register-sync-resolve.ts b/src/cli/commands/sync/register-sync-resolve.ts index b4c6b37..e2a3657 100644 --- a/src/cli/commands/sync/register-sync-resolve.ts +++ b/src/cli/commands/sync/register-sync-resolve.ts @@ -2,7 +2,7 @@ import { Command, Option } from 'commander'; import { Logger } from '../../../utils/logger.js'; import { SyncConflictError } from '../../../utils/errors.js'; import type { ServiceDeps } from '../service-factory.js'; -import { emitJsonErrorAndExit, resolveFormat } from './sync-options.js'; +import { emitJsonErrorAndExit, resolveFormat, resolveSyncConfig } from './sync-options.js'; interface ResolveOptions { syncConfig?: string; @@ -43,7 +43,9 @@ async function handleSyncResolve( const { resolveLockFile } = await import('../../../sync/sync-resolve.js'); const pathMod = await import('path'); - const config = await loadSyncConfig(process.cwd(), { configPath: options.syncConfig }); + const config = await loadSyncConfig(process.cwd(), { + configPath: resolveSyncConfig(options, command), + }); const lockPath = pathMod.join(config.projectRoot, LOCK_FILE_NAME); const result = await resolveLockFile(lockPath, { dryRun }); diff --git a/src/cli/commands/sync/register-sync-status.ts b/src/cli/commands/sync/register-sync-status.ts index 6895d0e..6eeadf3 100644 --- a/src/cli/commands/sync/register-sync-status.ts +++ b/src/cli/commands/sync/register-sync-status.ts @@ -1,7 +1,13 @@ import { Command, Option } from 'commander'; import { Logger } from '../../../utils/logger.js'; import type { ServiceDeps } from '../service-factory.js'; -import { emitJsonErrorAndExit, resolveFormat } from './sync-options.js'; +import { + emitJsonErrorAndExit, + parseLocaleFilter, + resolveFormat, + resolveLocale, + resolveSyncConfig, +} from './sync-options.js'; interface StatusOptions { locale?: string; @@ -37,20 +43,17 @@ async function handleSyncStatus( const { createDefaultRegistry } = await import('../../../formats/index.js'); const { computeSyncStatus } = await import('../../../sync/sync-status.js'); - const config = await loadSyncConfig(process.cwd(), { configPath: options.syncConfig }); + const localeFilter = parseLocaleFilter(resolveLocale(options, command)); + const config = await loadSyncConfig(process.cwd(), { + configPath: resolveSyncConfig(options, command), + localeFilter, + }); const registry = await createDefaultRegistry(); const status = await computeSyncStatus(config, registry); - let locales = status.locales; - if (options.locale) { - const filterLocales = options.locale.split(',').map((l) => l.trim()); - locales = locales.filter((l) => filterLocales.includes(l.locale)); - if (locales.length === 0) { - Logger.warn( - `No matching locales for filter. Available: ${config.target_locales.join(', ')}`, - ); - } - } + const locales = localeFilter + ? status.locales.filter((l) => localeFilter.includes(l.locale)) + : status.locales; if (options.format === 'json') { process.stdout.write(JSON.stringify({ ...status, locales }, null, 2) + '\n'); diff --git a/src/cli/commands/sync/register-sync-validate.ts b/src/cli/commands/sync/register-sync-validate.ts index 729c54b..fda92af 100644 --- a/src/cli/commands/sync/register-sync-validate.ts +++ b/src/cli/commands/sync/register-sync-validate.ts @@ -3,7 +3,13 @@ import chalk from 'chalk'; import { Logger } from '../../../utils/logger.js'; import { ExitCode } from '../../../utils/exit-codes.js'; import type { ServiceDeps } from '../service-factory.js'; -import { emitJsonErrorAndExit, resolveFormat } from './sync-options.js'; +import { + emitJsonErrorAndExit, + parseLocaleFilter, + resolveFormat, + resolveLocale, + resolveSyncConfig, +} from './sync-options.js'; interface ValidateOptions { locale?: string; @@ -39,17 +45,16 @@ async function handleSyncValidate( const { createDefaultRegistry } = await import('../../../formats/index.js'); const { validateTranslations } = await import('../../../sync/sync-validate.js'); - const config = await loadSyncConfig(process.cwd(), { configPath: options.syncConfig }); + const localeFilter = parseLocaleFilter(resolveLocale(options, command)); + const config = await loadSyncConfig(process.cwd(), { + configPath: resolveSyncConfig(options, command), + localeFilter, + }); - const allLocales = [...config.target_locales]; - if (options.locale) { - const filterLocales = options.locale.split(',').map((l) => l.trim()); + if (localeFilter) { config.target_locales = config.target_locales.filter((l: string) => - filterLocales.includes(l), + localeFilter.includes(l), ); - if (config.target_locales.length === 0) { - Logger.warn(`No matching locales for filter. Available: ${allLocales.join(', ')}`); - } } const registry = await createDefaultRegistry(); diff --git a/src/cli/commands/sync/sync-options.ts b/src/cli/commands/sync/sync-options.ts index 4590b0f..91965d6 100644 --- a/src/cli/commands/sync/sync-options.ts +++ b/src/cli/commands/sync/sync-options.ts @@ -21,9 +21,13 @@ export function resolveFormat( } /** - * Resolve --locale across parent (`sync`) and subcommand scopes so push/pull - * narrow the fan-out regardless of where the flag sits on the invocation + * Resolve --locale across parent (`sync`) and subcommand scopes so a + * subcommand narrows regardless of where the flag sits on the invocation * line. Subcommand value wins; otherwise fall back to the parent's. + * + * Without positional options, commander keeps matching the parent's flags + * after the subcommand name, so `deepl sync status --locale de` binds `de` to + * the parent `sync` command and leaves the subcommand's own store undefined. */ export function resolveLocale( opts: { locale?: string }, @@ -33,6 +37,28 @@ export function resolveLocale( return command.parent?.opts()['locale'] as string | undefined; } +/** + * Resolve --sync-config across parent (`sync`) and subcommand scopes. Same + * parent/child binding rule as {@link resolveLocale}: a subcommand that reads + * only its own store silently falls back to the auto-detected config. + */ +export function resolveSyncConfig( + opts: { syncConfig?: string }, + command: Command, +): string | undefined { + if (opts.syncConfig !== undefined) return opts.syncConfig; + return command.parent?.opts()['syncConfig'] as string | undefined; +} + +/** + * Split a resolved --locale value into the comma-separated filter list that + * `loadSyncConfig` validates against `target_locales`. + */ +export function parseLocaleFilter(locale: string | undefined): string[] | undefined { + if (!locale) return undefined; + return locale.split(',').map((l) => l.trim()); +} + /** * JSON error envelope shape emitted on stderr when --format json is set and * a command fails. Shared across every `deepl sync` subcommand so script diff --git a/src/cli/index.ts b/src/cli/index.ts index b7670ad..3ec44c0 100644 --- a/src/cli/index.ts +++ b/src/cli/index.ts @@ -11,7 +11,7 @@ import { readFileSync } from 'fs'; import { fileURLToPath } from 'url'; import { dirname, join, resolve, isAbsolute, extname } from 'path'; import { ConfigService } from '../storage/config.js'; -import { createCacheServiceGetter } from './cache-loader.js'; +import { createCacheServiceGetter, resolveCacheOptions } from './cache-loader.js'; import { resolvePaths } from '../utils/paths.js'; import type { DeepLClient } from '../api/deepl-client.js'; import { Logger } from '../utils/logger.js'; @@ -59,16 +59,13 @@ const paths = resolvePaths(); // Create config service - can be overridden by --config flag let configService = new ConfigService(paths.configFile); -const getCacheService = createCacheServiceGetter(() => { - const configTtl = configService.getValue('cache.ttl'); - const configMaxSize = configService.getValue('cache.maxSize'); - return { - dbPath: paths.cacheFile, - // Config TTL is in seconds, CacheService expects milliseconds - ttl: configTtl !== undefined ? configTtl * 1000 : undefined, - maxSize: configMaxSize, - }; -}); +// HTTP transport overrides from --timeout / --max-retries, applied to every +// client the commands construct. +let httpOptions: { timeout?: number; maxRetries?: number } = {}; + +const getCacheService = createCacheServiceGetter(() => + resolveCacheOptions(configService, paths.cacheFile), +); /** * Handle error and exit with appropriate exit code @@ -89,6 +86,21 @@ function handleError(error: unknown): never { process.exit(exitCode); } +/** + * Parse an integer CLI option, exiting with InvalidInput when it is not a + * whole number at or above `min`. + */ +function parseIntOption(raw: string, flag: string, min: number): number { + const value = Number(raw); + if (!Number.isInteger(value) || value < min) { + Logger.error( + chalk.red(`Error: ${flag} must be an integer >= ${min} (got "${raw}")`) + ); + process.exit(ExitCode.InvalidInput); + } + return value; +} + /** * Create DeepL client with API key from config or env */ @@ -125,7 +137,7 @@ async function createDeepLClient( } const { DeepLClient: Client } = await import('../api/deepl-client.js'); - return new Client(key, { baseUrl, usePro }); + return new Client(key, { baseUrl, usePro, ...httpOptions }); } // Create program @@ -151,6 +163,14 @@ program '--no-input', 'Disable all interactive prompts (abort instead of prompting)' ) + .option( + '--timeout ', + 'HTTP request timeout in milliseconds (default: 30000)' + ) + .option( + '--max-retries ', + 'Maximum automatic retries for retryable requests (default: 3)' + ) .hook('preAction', (thisCommand) => { const options = thisCommand.opts(); @@ -210,6 +230,19 @@ program if (options['input'] === false) { setNoInput(true); } + + httpOptions = { + ...(options['timeout'] !== undefined && { + timeout: parseIntOption(options['timeout'] as string, '--timeout', 1), + }), + ...(options['maxRetries'] !== undefined && { + maxRetries: parseIntOption( + options['maxRetries'] as string, + '--max-retries', + 0 + ), + }), + }; }); /** @@ -241,7 +274,7 @@ function getApiKeyAndOptions(): { validateApiUrl(baseUrl); } - return { apiKey: key, options: { baseUrl } }; + return { apiKey: key, options: { baseUrl, ...httpOptions } }; } // Shared dependencies passed to register functions diff --git a/src/formats/android-xml.ts b/src/formats/android-xml.ts index a7bdadd..d59b431 100644 --- a/src/formats/android-xml.ts +++ b/src/formats/android-xml.ts @@ -1,25 +1,45 @@ import type { ExtractedEntry, FormatParser, TranslatedEntry } from './format.js'; +import { ValidationError } from '../utils/errors.js'; +import { + replaceElements, + scanElements, + type ElementPattern, + type ScannedElement, +} from './xml-scan.js'; interface PluralItem { quantity: string; value: string; } -const STRING_RE = - /([\s\S]*?)<\/string>/g; +const ATTRS = String.raw`((?:\s+[a-zA-Z_:][a-zA-Z0-9_:.-]*=(?:"[^"<]*"|'[^'<]*'))*)`; -const TRANSLATABLE_FALSE_RE = /\btranslatable\s*=\s*"false"/; +const STRING_EL: ElementPattern = { + open: new RegExp(String.raw``, 'y'), + close: /<\/string>/y, +}; + +const PLURALS_EL: ElementPattern = { + open: new RegExp(String.raw``, 'y'), + close: /<\/plurals>/y, +}; -const PLURALS_RE = - /]*)>([\s\S]*?)<\/plurals>/g; +const PLURAL_ITEM_EL: ElementPattern = { + open: new RegExp(String.raw``, 'y'), + close: /<\/item>/y, +}; -const PLURAL_ITEM_RE = - /]*>([\s\S]*?)<\/item>/g; +const STRING_ARRAY_EL: ElementPattern = { + open: new RegExp(String.raw``, 'y'), + close: /<\/string-array>/y, +}; -const STRING_ARRAY_RE = - /]*)>([\s\S]*?)<\/string-array>/g; +const ARRAY_ITEM_EL: ElementPattern = { + open: //y, + close: /<\/item>/y, +}; -const ARRAY_ITEM_RE = /([\s\S]*?)<\/item>/g; +const TRANSLATABLE_FALSE_RE = /\btranslatable\s*=\s*"false"/; const XML_ENTITY_RE = /&(?:#x([0-9a-fA-F]+)|#(\d+)|(amp|lt|gt|quot|apos));/g; @@ -76,6 +96,22 @@ function escapeAndroid(value: string): string { .replace(/>/g, '>'); } +/** + * Refuse a value that would close the CDATA section it is written into. + * Nothing inside a CDATA body is entity-escaped, so the text after a "]]>" + * would be parsed as XML — an injected element in a generated resource file. + * Fail fast rather than rewrite the value into something the round-trip + * cannot reproduce, matching the XLIFF parser's stance on CDATA. + */ +function assertNoCdataBreakout(value: string): void { + if (value.includes(']]>')) { + throw new ValidationError( + 'Android CDATA values containing "]]>" are not supported.', + 'Remove the "]]>" sequence from the text, or drop the wrapper in the source file so the value is entity-escaped instead.', + ); + } +} + export class AndroidXmlFormatParser implements FormatParser { readonly name = 'Android XML'; readonly configKey = 'android_xml'; @@ -95,6 +131,7 @@ export class AndroidXmlFormatParser implements FormatParser { const translations = new Map(); const pluralTranslations = new Map>(); const arrayTranslations = new Map>(); + let arrayNames: Set | undefined; for (const entry of entries) { if (entry.metadata?.['plurals']) { @@ -107,10 +144,12 @@ export class AndroidXmlFormatParser implements FormatParser { } else if (entry.key.includes('.')) { const lastDot = entry.key.lastIndexOf('.'); const arrayName = entry.key.substring(0, lastDot); - const indexStr = entry.key.substring(lastDot + 1); - const index = parseInt(indexStr, 10); + const index = parseInt(entry.key.substring(lastDot + 1), 10); + arrayNames ??= new Set( + scanElements(originalContent, STRING_ARRAY_EL).map((el) => el.groups[0]!), + ); - if (!isNaN(index) && this.isStringArrayKey(originalContent, arrayName)) { + if (!isNaN(index) && arrayNames.has(arrayName)) { if (!arrayTranslations.has(arrayName)) { arrayTranslations.set(arrayName, new Map()); } @@ -123,115 +162,76 @@ export class AndroidXmlFormatParser implements FormatParser { } } - let result = originalContent; - - result = result.replace(STRING_RE, (match, name: string, attrs: string, innerText: string) => { + let result = replaceElements(originalContent, STRING_EL, (el) => { + const attrs = el.groups[1] ?? ''; if (TRANSLATABLE_FALSE_RE.test(attrs)) { - return match; + return el.text; } - const translation = translations.get(name); - if (translation !== undefined) { - const escapedTranslation = this.escapeForReconstruct(innerText, translation); - return `${escapedTranslation}`; + const translation = translations.get(el.groups[0]!); + if (translation === undefined) { + return null; } - return match; + return this.rewriteInner(el, this.escapeForReconstruct(el.inner, translation)); }); - result = result.replace(PLURALS_RE, (match, name: string, extraAttrs: string, innerContent: string) => { - const quantityMap = pluralTranslations.get(name); + result = replaceElements(result, PLURALS_EL, (el) => { + const quantityMap = pluralTranslations.get(el.groups[0]!); if (!quantityMap) { - return match; + return null; } - const updatedInner = innerContent.replace( - PLURAL_ITEM_RE, - (itemMatch, quantity: string, value: string) => { - const translation = quantityMap.get(quantity); - if (translation !== undefined) { - return `${this.escapeForReconstruct(value, translation)}`; - } - return itemMatch; - }, - ); - return `${updatedInner}`; + const inner = replaceElements(el.inner, PLURAL_ITEM_EL, (item) => { + const translation = quantityMap.get(item.groups[0]!); + if (translation === undefined) { + return item.text; + } + return this.rewriteInner(item, this.escapeForReconstruct(item.inner, translation)); + }); + return this.rewriteInner(el, inner); }); - result = result.replace(STRING_ARRAY_RE, (match, name: string, extraAttrs: string, innerContent: string) => { - const indexMap = arrayTranslations.get(name); + result = replaceElements(result, STRING_ARRAY_EL, (el) => { + const indexMap = arrayTranslations.get(el.groups[0]!); if (!indexMap) { - return match; + return null; } - let idx = 0; - const updatedInner = innerContent.replace( - ARRAY_ITEM_RE, - (itemMatch, value: string) => { - const translation = indexMap.get(idx); - idx++; - if (translation !== undefined) { - return `${this.escapeForReconstruct(value, translation)}`; - } - return itemMatch; - }, - ); - return `${updatedInner}`; + let index = 0; + const inner = replaceElements(el.inner, ARRAY_ITEM_EL, (item) => { + const translation = indexMap.get(index); + index++; + if (translation === undefined) { + return item.text; + } + return this.rewriteInner(item, this.escapeForReconstruct(item.inner, translation)); + }); + return this.rewriteInner(el, inner); }); - const stringKeys = new Set([...translations.keys()]); - const pluralKeys = new Set([...pluralTranslations.keys()]); - const arrayKeys = new Set([...arrayTranslations.keys()]); - - result = result.replace( - /[ \t]*]*>[\s\S]*?<\/string>\s*\n?/g, - (match, name: string) => stringKeys.has(name) || TRANSLATABLE_FALSE_RE.test(match) ? match : '', - ); - result = result.replace( - /[ \t]*]*>[\s\S]*?<\/plurals>\s*\n?/g, - (match, name: string) => pluralKeys.has(name) ? match : '', - ); - result = result.replace( - /[ \t]*]*>[\s\S]*?<\/string-array>\s*\n?/g, - (match, name: string) => arrayKeys.has(name) ? match : '', - ); - return result; } - private extractStrings(content: string, entries: ExtractedEntry[]): void { - const regex = new RegExp(STRING_RE.source, STRING_RE.flags); - let match: RegExpExecArray | null; - while ((match = regex.exec(content)) !== null) { - const name = match[1]!; - const attrs = match[2] ?? ''; - const rawValue = match[3]!; + private rewriteInner(element: ScannedElement, inner: string): string { + return `${element.openTag}${inner}${element.closeTag}`; + } - if (TRANSLATABLE_FALSE_RE.test(attrs)) { + private extractStrings(content: string, entries: ExtractedEntry[]): void { + for (const el of scanElements(content, STRING_EL)) { + if (TRANSLATABLE_FALSE_RE.test(el.groups[1] ?? '')) { continue; } - - const value = this.decodeValue(rawValue); - entries.push({ key: name, value }); + entries.push({ key: el.groups[0]!, value: this.decodeValue(el.inner) }); } } private extractPlurals(content: string, entries: ExtractedEntry[]): void { - const pluralsRegex = new RegExp(PLURALS_RE.source, PLURALS_RE.flags); - let match: RegExpExecArray | null; - while ((match = pluralsRegex.exec(content)) !== null) { - const name = match[1]!; - const innerContent = match[3]!; - - const plurals: PluralItem[] = []; - const itemRegex = new RegExp(PLURAL_ITEM_RE.source, PLURAL_ITEM_RE.flags); - let itemMatch: RegExpExecArray | null; - while ((itemMatch = itemRegex.exec(innerContent)) !== null) { - plurals.push({ - quantity: itemMatch[1]!, - value: this.decodeValue(itemMatch[2]!), - }); - } + for (const el of scanElements(content, PLURALS_EL)) { + const plurals: PluralItem[] = scanElements(el.inner, PLURAL_ITEM_EL).map((item) => ({ + quantity: item.groups[0]!, + value: this.decodeValue(item.inner), + })); const defaultItem = plurals.find(p => p.quantity === 'other') ?? plurals[0]; entries.push({ - key: name, + key: el.groups[0]!, value: defaultItem?.value ?? '', metadata: { plurals }, }); @@ -239,20 +239,11 @@ export class AndroidXmlFormatParser implements FormatParser { } private extractStringArrays(content: string, entries: ExtractedEntry[]): void { - const arrayRegex = new RegExp(STRING_ARRAY_RE.source, STRING_ARRAY_RE.flags); - let match: RegExpExecArray | null; - while ((match = arrayRegex.exec(content)) !== null) { - const name = match[1]!; - const innerContent = match[3]!; - - const itemRegex = new RegExp(ARRAY_ITEM_RE.source, ARRAY_ITEM_RE.flags); - let itemMatch: RegExpExecArray | null; + for (const el of scanElements(content, STRING_ARRAY_EL)) { + const name = el.groups[0]!; let index = 0; - while ((itemMatch = itemRegex.exec(innerContent)) !== null) { - entries.push({ - key: `${name}.${index}`, - value: this.decodeValue(itemMatch[1]!), - }); + for (const item of scanElements(el.inner, ARRAY_ITEM_EL)) { + entries.push({ key: `${name}.${index}`, value: this.decodeValue(item.inner) }); index++; } } @@ -260,8 +251,7 @@ export class AndroidXmlFormatParser implements FormatParser { private decodeValue(raw: string): string { if (raw.startsWith('" - // into `]]]]>`, so a literal "]]>" spans two sections. + // Adjacent sections concatenate, so `` is "ab". const sectionRe = //g; let joined = ''; let consumedTo = 0; @@ -277,19 +267,10 @@ export class AndroidXmlFormatParser implements FormatParser { } private escapeForReconstruct(originalInner: string, translation: string): string { - if (/^" would close the section early and the - // remainder would be parsed as XML. Splitting into adjacent CDATA - // sections keeps the text literal without escaping it. - const safe = translation.replace(/]]>/g, ']]]]>'); - return ``; + if (originalInner.startsWith('`; } return escapeAndroid(translation); } - - private isStringArrayKey(content: string, name: string): boolean { - const escaped = name.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); - const regex = new RegExp(`]*version=["'](\d+\.\d+)["']/i; -const TRANS_UNIT_RE = - /<(?:\w+:)?trans-unit\s+id=["']([^"']+)["'][^>]*>([\s\S]*?)<\/(?:\w+:)?trans-unit>/gi; +const TRANS_UNIT_EL: ElementPattern = { + open: /<(?:\w+:)?trans-unit\s+id=["']([^"'<]+)["'][^><]*>/iy, + close: /<\/(?:\w+:)?trans-unit>/iy, +}; -const UNIT_RE = - /<(?:\w+:)?unit\s+id=["']([^"']+)["'][^>]*>([\s\S]*?)<\/(?:\w+:)?unit>/gi; +const UNIT_EL: ElementPattern = { + open: /<(?:\w+:)?unit\s+id=["']([^"'<]+)["'][^><]*>/iy, + close: /<\/(?:\w+:)?unit>/iy, +}; + +const SOURCE_EL: ElementPattern = { + open: /<(\w+:)?source>/iy, + close: /<\/(?:\w+:)?source>/iy, +}; -const SOURCE_RE = /<(?:\w+:)?source>([\s\S]*?)<\/(?:\w+:)?source>/i; // Attributes are optional but must be preserved: `state` is a standard XLIFF // attribute that every CAT tool writes, so requiring a bare tag would make -// those elements invisible to this regex. -const TARGET_RE = /<(\w+:)?target((?:\s[^>]*)?)>([\s\S]*?)<\/(?:\w+:)?target>/i; -const NOTE_RE = /<(?:\w+:)?note(?:\s[^>]*)?>([\s\S]*?)<\/(?:\w+:)?note>/i; -const SEGMENT_RE = /<(?:\w+:)?segment(?:\s[^>]*)?>([\s\S]*?)<\/(?:\w+:)?segment>/i; +// those elements invisible to this scan. +const TARGET_EL: ElementPattern = { + open: /<(\w+:)?target((?:\s[^><]*)?)>/iy, + close: /<\/(?:\w+:)?target>/iy, +}; + +const NOTE_EL: ElementPattern = { + open: /<(?:\w+:)?note(?:\s[^><]*)?>/iy, + close: /<\/(?:\w+:)?note>/iy, +}; + +const SEGMENT_EL: ElementPattern = { + open: /<(?:\w+:)?segment(?:\s[^><]*)?>/iy, + close: /<\/(?:\w+:)?segment>/iy, +}; + +const TRANSLATABLE_EL: ElementPattern = { + open: /<(?:\w+:)?(?:source|target)(?:\s[^><]*)?>/iy, + close: /<\/(?:\w+:)?(?:source|target)>/iy, +}; const NAMED_ENTITIES: Record = { amp: '&', @@ -50,9 +81,6 @@ function unescapeXml(value: string): string { }); } -const CDATA_IN_TRANSLATABLE_RE = - /<(?:\w+:)?(?:source|target)[^>]*>[\s\S]*?/i; - /** * Refuse XLIFF input that contains a CDATA section inside a `` or * `` element. The regex-based extract/reconstruct pair cannot @@ -64,11 +92,13 @@ const CDATA_IN_TRANSLATABLE_RE = */ function assertNoCdataInTranslatable(content: string): void { if (!content.includes(' / elements containing CDATA sections are not supported.', - 'Inline the literal text without the wrapper, or preprocess the file to entity-escape CDATA content before syncing.', - ); + for (const element of scanElements(content, TRANSLATABLE_EL)) { + if (element.inner.includes(' / elements containing CDATA sections are not supported.', + 'Inline the literal text without the wrapper, or preprocess the file to entity-escape CDATA content before syncing.', + ); + } } } @@ -84,6 +114,36 @@ function detectVersion(content: string): string { return match?.[1] ?? '1.2'; } +function rewriteInner(element: ScannedElement, inner: string): string { + return `${element.openTag}${inner}${element.closeTag}`; +} + +/** + * Replace the first `` in a block, or insert one after `` + * when the block has none. + */ +function applyTarget(block: string, escaped: string): string { + const target = findElement(block, TARGET_EL); + if (target) { + const ns = target.groups[0] ?? ''; + const attrs = target.groups[1] ?? ''; + return ( + block.slice(0, target.start) + + `<${ns}target${attrs}>${escaped}` + + block.slice(target.end) + ); + } + + const source = findElement(block, SOURCE_EL); + if (!source) return block; + const ns = source.groups[0] ?? ''; + return ( + block.slice(0, source.end) + + `\n <${ns}target>${escaped}` + + block.slice(source.end) + ); +} + export class XliffFormatParser implements FormatParser { readonly name = 'XLIFF'; readonly configKey = 'xliff'; @@ -118,156 +178,67 @@ export class XliffFormatParser implements FormatParser { } extractContext(content: string, key: string): string | undefined { - const version = detectVersion(content); + const unit = detectVersion(content) === '2.0' ? UNIT_EL : TRANS_UNIT_EL; - if (version === '2.0') { - return this.extractContextV2(content, key); + for (const element of scanElements(content, unit)) { + if (element.groups[0] !== key) continue; + const note = findElement(element.inner, NOTE_EL); + return note ? unescapeXml(note.inner) : undefined; } - return this.extractContextV12(content, key); + return undefined; } private extractV12(content: string, entries: ExtractedEntry[]): void { - const regex = new RegExp(TRANS_UNIT_RE.source, TRANS_UNIT_RE.flags); - let match: RegExpExecArray | null; - while ((match = regex.exec(content)) !== null) { - const id = match[1]!; - const block = match[2]!; - - const sourceMatch = SOURCE_RE.exec(block); - if (!sourceMatch) continue; - - const value = unescapeXml(sourceMatch[1]!); - const noteMatch = NOTE_RE.exec(block); - const context = noteMatch ? unescapeXml(noteMatch[1]!) : undefined; - - const entry: ExtractedEntry = { key: id, value }; - if (context !== undefined) { - entry.context = context; - } - entries.push(entry); + for (const element of scanElements(content, TRANS_UNIT_EL)) { + const source = findElement(element.inner, SOURCE_EL); + if (!source) continue; + entries.push(this.toEntry(element.groups[0]!, source.inner, element.inner)); } } private extractV2(content: string, entries: ExtractedEntry[]): void { - const regex = new RegExp(UNIT_RE.source, UNIT_RE.flags); - let match: RegExpExecArray | null; - while ((match = regex.exec(content)) !== null) { - const id = match[1]!; - const block = match[2]!; - - const segmentMatch = SEGMENT_RE.exec(block); - if (!segmentMatch) continue; - - const segment = segmentMatch[1]!; - const sourceMatch = SOURCE_RE.exec(segment); - if (!sourceMatch) continue; - - const value = unescapeXml(sourceMatch[1]!); - const noteMatch = NOTE_RE.exec(block); - const context = noteMatch ? unescapeXml(noteMatch[1]!) : undefined; - - const entry: ExtractedEntry = { key: id, value }; - if (context !== undefined) { - entry.context = context; - } - entries.push(entry); + for (const element of scanElements(content, UNIT_EL)) { + const segment = findElement(element.inner, SEGMENT_EL); + if (!segment) continue; + + const source = findElement(segment.inner, SOURCE_EL); + if (!source) continue; + entries.push(this.toEntry(element.groups[0]!, source.inner, element.inner)); } } - private reconstructV12( - content: string, - translations: Map, - ): string { - const regex = new RegExp(TRANS_UNIT_RE.source, TRANS_UNIT_RE.flags); - let result = content.replace(regex, (fullMatch, id: string, block: string) => { - const translation = translations.get(id); - if (translation === undefined) return ''; - - const escaped = escapeXml(translation); - const targetMatch = TARGET_RE.exec(block); - - let newBlock: string; - if (targetMatch) { - const ns = targetMatch[1] ?? ''; - const attrs = targetMatch[2] ?? ''; - newBlock = block.replace(TARGET_RE, () => `<${ns}target${attrs}>${escaped}`); - } else { - const sourceNsMatch = /<(\w+:)?source>/i.exec(block); - const ns = sourceNsMatch?.[1] ?? ''; - newBlock = block.replace( - /(<\/(?:\w+:)?source>)/i, - (_match, src: string) => `${src}\n <${ns}target>${escaped}`, - ); - } - - return fullMatch.replace(block, () => newBlock); - }); - result = result.replace(/\n{3,}/g, '\n\n'); - return result; + private toEntry(id: string, rawSource: string, block: string): ExtractedEntry { + const entry: ExtractedEntry = { key: id, value: unescapeXml(rawSource) }; + const note = findElement(block, NOTE_EL); + if (note) { + entry.context = unescapeXml(note.inner); + } + return entry; } - private reconstructV2( - content: string, - translations: Map, - ): string { - const regex = new RegExp(UNIT_RE.source, UNIT_RE.flags); - let result = content.replace(regex, (fullMatch, id: string, block: string) => { - const translation = translations.get(id); + private reconstructV12(content: string, translations: Map): string { + const result = replaceElements(content, TRANS_UNIT_EL, (element) => { + const translation = translations.get(element.groups[0]!); if (translation === undefined) return ''; - - const escaped = escapeXml(translation); - const segmentMatch = SEGMENT_RE.exec(block); - if (!segmentMatch) return fullMatch; - - const segment = segmentMatch[1]!; - const targetMatch = TARGET_RE.exec(segment); - - let newSegment: string; - if (targetMatch) { - const ns = targetMatch[1] ?? ''; - const attrs = targetMatch[2] ?? ''; - newSegment = segment.replace(TARGET_RE, () => `<${ns}target${attrs}>${escaped}`); - } else { - const sourceNsMatch = /<(\w+:)?source>/i.exec(segment); - const ns = sourceNsMatch?.[1] ?? ''; - newSegment = segment.replace( - /(<\/(?:\w+:)?source>)/i, - (_match, src: string) => `${src}\n <${ns}target>${escaped}`, - ); - } - - const newBlock = block.replace(segment, () => newSegment); - return fullMatch.replace(block, () => newBlock); + return rewriteInner(element, applyTarget(element.inner, escapeXml(translation))); }); - result = result.replace(/\n{3,}/g, '\n\n'); - return result; + return result.replace(/\n{3,}/g, '\n\n'); } - private extractContextV12(content: string, key: string): string | undefined { - const escaped = key.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); - const regex = new RegExp( - `<(?:\\w+:)?trans-unit\\s+id=["']${escaped}["'][^>]*>([\\s\\S]*?)<\\/(?:\\w+:)?trans-unit>`, - 'i', - ); - const match = regex.exec(content); - if (!match) return undefined; - - const block = match[1]!; - const noteMatch = NOTE_RE.exec(block); - return noteMatch ? unescapeXml(noteMatch[1]!) : undefined; - } + private reconstructV2(content: string, translations: Map): string { + const result = replaceElements(content, UNIT_EL, (element) => { + const translation = translations.get(element.groups[0]!); + if (translation === undefined) return ''; - private extractContextV2(content: string, key: string): string | undefined { - const escaped = key.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); - const regex = new RegExp( - `<(?:\\w+:)?unit\\s+id=["']${escaped}["'][^>]*>([\\s\\S]*?)<\\/(?:\\w+:)?unit>`, - 'i', - ); - const match = regex.exec(content); - if (!match) return undefined; + const segment = findElement(element.inner, SEGMENT_EL); + if (!segment) return element.text; - const block = match[1]!; - const noteMatch = NOTE_RE.exec(block); - return noteMatch ? unescapeXml(noteMatch[1]!) : undefined; + const inner = + element.inner.slice(0, segment.start) + + rewriteInner(segment, applyTarget(segment.inner, escapeXml(translation))) + + element.inner.slice(segment.end); + return rewriteInner(element, inner); + }); + return result.replace(/\n{3,}/g, '\n\n'); } } diff --git a/src/formats/xml-scan.ts b/src/formats/xml-scan.ts new file mode 100644 index 0000000..b5cc881 --- /dev/null +++ b/src/formats/xml-scan.ts @@ -0,0 +1,172 @@ +/** + * Bounded element scanning for the XML-shaped format parsers. + * + * Matching an element as one regex (`([\s\S]*?)`) costs a scan + * of the remaining input for every opening tag that never closes, which is + * quadratic in file size. The scanners here walk the input once: opening tags + * are matched sticky (anchored, so no scan), and the search for a closing tag + * moves a single cursor forward. Once no closing tag remains, no later opening + * tag can have one either, so scanning stops. + * + * Every quantifier in a pattern passed here must exclude `<`, so a match + * attempt cannot run past the following tag. XML forbids `<` in attribute + * values, so this only rejects input that is already malformed. + */ + +const CDATA_OPEN = ''; +const WHITESPACE_RE = /\s/; + +export interface ElementPattern { + /** Sticky pattern for an opening tag: `<` followed by an element name. */ + open: RegExp; + /** Sticky pattern for a closing tag: ` string | null, +): string { + const elements = scanElements(content, pattern); + if (elements.length === 0) return content; + + const parts: string[] = []; + let copied = 0; + + for (const element of elements) { + const replacement = replace(element); + if (replacement === null) { + let from = element.start; + while (from > copied && (content[from - 1] === ' ' || content[from - 1] === '\t')) from--; + let to = element.end; + while (to < content.length && WHITESPACE_RE.test(content[to]!)) to++; + parts.push(content.slice(copied, from)); + copied = to; + } else { + parts.push(content.slice(copied, element.start), replacement); + copied = element.end; + } + } + + parts.push(content.slice(copied)); + return parts.join(''); +} diff --git a/src/formats/yaml.ts b/src/formats/yaml.ts index f735ee2..377872e 100644 --- a/src/formats/yaml.ts +++ b/src/formats/yaml.ts @@ -1,6 +1,16 @@ import * as YAML from 'yaml'; import type { FormatParser, ExtractedEntry, TranslatedEntry } from './format.js'; +// Aliases are resolved while walking the AST, where the yaml package's own +// maxAliasCount guard does not apply, so expansion is bounded here instead. +const MAX_ALIAS_EXPANDED_NODES = 200000; +const MAX_ALIAS_DEPTH = 20; + +interface AliasBudget { + expandedNodes: number; + depth: number; +} + export class YamlFormatParser implements FormatParser { readonly name = 'YAML'; readonly configKey = 'yaml'; @@ -23,7 +33,10 @@ export class YamlFormatParser implements FormatParser { } const entries: ExtractedEntry[] = []; - this.walkNode(doc.contents, [], entries, doc); + this.walkNode(doc.contents, [], entries, doc, { + expandedNodes: 0, + depth: 0, + }); return entries; } @@ -43,37 +56,44 @@ export class YamlFormatParser implements FormatParser { } const existingPaths: string[][] = []; + const budget: AliasBudget = { expandedNodes: 0, depth: 0 }; + const walkAlias = (alias: YAML.Alias, path: string[]): void => { + const resolved = this.resolveAlias(alias, doc, budget); + if (YAML.isScalar(resolved) && typeof resolved.value === 'string') { + existingPaths.push(path); + } else if (YAML.isMap(resolved) || YAML.isSeq(resolved)) { + budget.depth++; + walkDoc(resolved, path); + budget.depth--; + } + }; const walkDoc = (node: unknown, path: string[]): void => { if (YAML.isMap(node)) { for (const item of node.items) { + if (budget.depth > 0) { + this.chargeAliasBudget(budget); + } const key = String(YAML.isScalar(item.key) ? item.key.value : item.key); if (YAML.isScalar(item.value) && typeof item.value.value === 'string') { existingPaths.push([...path, key]); } else if (YAML.isMap(item.value) || YAML.isSeq(item.value)) { walkDoc(item.value, [...path, key]); } else if (YAML.isAlias(item.value)) { - const resolved = item.value.resolve(doc); - if (YAML.isScalar(resolved) && typeof resolved.value === 'string') { - existingPaths.push([...path, key]); - } else if (YAML.isMap(resolved) || YAML.isSeq(resolved)) { - walkDoc(resolved, [...path, key]); - } + walkAlias(item.value, [...path, key]); } } } else if (YAML.isSeq(node)) { for (let i = 0; i < node.items.length; i++) { + if (budget.depth > 0) { + this.chargeAliasBudget(budget); + } const item = node.items[i]; if (YAML.isScalar(item) && typeof item.value === 'string') { existingPaths.push([...path, String(i)]); } else if (YAML.isMap(item) || YAML.isSeq(item)) { walkDoc(item, [...path, String(i)]); } else if (YAML.isAlias(item)) { - const resolved = item.resolve(doc); - if (YAML.isScalar(resolved) && typeof resolved.value === 'string') { - existingPaths.push([...path, String(i)]); - } else if (YAML.isMap(resolved) || YAML.isSeq(resolved)) { - walkDoc(resolved, [...path, String(i)]); - } + walkAlias(item, [...path, String(i)]); } } } @@ -106,25 +126,64 @@ export class YamlFormatParser implements FormatParser { pathParts: string[], entries: ExtractedEntry[], doc: YAML.Document, + budget: AliasBudget, ): void { + if (budget.depth > 0) { + this.chargeAliasBudget(budget); + } + if (YAML.isMap(node)) { for (const item of node.items) { const key = String(YAML.isScalar(item.key) ? item.key.value : item.key); - this.walkNode(item.value, [...pathParts, key], entries, doc); + this.walkNode(item.value, [...pathParts, key], entries, doc, budget); } } else if (YAML.isSeq(node)) { for (let i = 0; i < node.items.length; i++) { - this.walkNode(node.items[i], [...pathParts, String(i)], entries, doc); + this.walkNode( + node.items[i], + [...pathParts, String(i)], + entries, + doc, + budget, + ); } } else if (YAML.isScalar(node) && typeof node.value === 'string') { entries.push({ key: pathParts.join('\0'), value: node.value }); } else if (YAML.isAlias(node)) { - const resolved = node.resolve(doc); + const resolved = this.resolveAlias(node, doc, budget); if (YAML.isScalar(resolved) && typeof resolved.value === 'string') { entries.push({ key: pathParts.join('\0'), value: resolved.value }); } else if (YAML.isMap(resolved) || YAML.isSeq(resolved)) { - this.walkNode(resolved, pathParts, entries, doc); + budget.depth++; + this.walkNode(resolved, pathParts, entries, doc, budget); + budget.depth--; } } } + + private resolveAlias( + alias: YAML.Alias, + doc: YAML.Document, + budget: AliasBudget, + ): unknown { + if (budget.depth >= MAX_ALIAS_DEPTH) { + throw new Error( + `YAML parse error: alias nesting is too deep (limit ${MAX_ALIAS_DEPTH}); ` + + 'an anchor in this document may reference itself', + ); + } + this.chargeAliasBudget(budget); + return alias.resolve(doc); + } + + private chargeAliasBudget(budget: AliasBudget): void { + budget.expandedNodes++; + if (budget.expandedNodes > MAX_ALIAS_EXPANDED_NODES) { + throw new Error( + `YAML parse error: aliases expand to more than ${MAX_ALIAS_EXPANDED_NODES} ` + + 'nodes; the anchors and aliases in this document expand to more content ' + + 'than can be processed', + ); + } + } } diff --git a/src/services/git-hooks.ts b/src/services/git-hooks.ts index 7e85072..2c78588 100644 --- a/src/services/git-hooks.ts +++ b/src/services/git-hooks.ts @@ -6,6 +6,7 @@ import * as fs from 'fs'; import * as path from 'path'; import * as crypto from 'crypto'; +import { execFileSync } from 'child_process'; import { ValidationError } from '../utils/errors.js'; export type HookType = 'pre-commit' | 'pre-push' | 'commit-msg' | 'post-commit'; @@ -14,6 +15,14 @@ export interface HookStatus { [key: string]: boolean; } +export interface InstallResult { + hookPath: string; + /** Path the pre-existing non-DeepL hook was copied to, or null if nothing was backed up. */ + backupPath: string | null; +} + +const MAX_BACKUP_SLOTS = 100; + export interface HookIntegrity { installed: boolean; markerVersion: null | 'legacy' | 1; @@ -34,13 +43,46 @@ export class GitHooksService { throw new ValidationError('Git directory not found: ' + gitDir); } - this.hooksDir = path.join(gitDir, 'hooks'); + this.hooksDir = GitHooksService.resolveHooksDir(gitDir); + } + + /** + * Resolve the directory git actually reads hooks from. + * + * `git rev-parse --git-path hooks` honours core.hooksPath (husky and friends) + * and resolves the `gitdir:` pointer used by linked worktrees and submodules, + * where `/.git` is a file rather than a directory. + */ + private static resolveHooksDir(gitDir: string): string { + const workDir = path.dirname(gitDir); + + try { + const resolved = execFileSync('git', ['rev-parse', '--git-path', 'hooks'], { + cwd: workDir, + encoding: 'utf-8', + stdio: ['ignore', 'pipe', 'ignore'], + }).trim(); + if (resolved) { + return path.resolve(workDir, resolved); + } + } catch { + // git unavailable, or workDir is not a working tree — fall back below. + } + + if (!fs.statSync(gitDir).isDirectory()) { + throw new ValidationError( + `Cannot resolve the hooks directory for "${gitDir}": it is a gitdir pointer file and git is not available to resolve it.`, + 'Install git, or run this command from the repository that owns the worktree.', + ); + } + + return path.join(gitDir, 'hooks'); } /** * Install a git hook */ - install(hookType: HookType): void { + install(hookType: HookType): InstallResult { this.validateHookType(hookType); const hookPath = this.getHookPath(hookType); @@ -52,10 +94,11 @@ export class GitHooksService { } // Backup existing hook if it exists and is not a DeepL hook + let backupPath: string | null = null; if (fs.existsSync(hookPath)) { const existingContent = fs.readFileSync(hookPath, 'utf-8'); if (!this.isDeepLHook(existingContent)) { - const backupPath = hookPath + '.backup'; + backupPath = GitHooksService.nextBackupPath(hookPath); fs.copyFileSync(hookPath, backupPath); } } @@ -65,6 +108,29 @@ export class GitHooksService { // Make it executable fs.chmodSync(hookPath, 0o755); + + return { hookPath, backupPath }; + } + + /** + * Pick a backup path that does not already exist, so a repeat install after + * another tool rewrote the hook cannot destroy the first backup. + */ + private static nextBackupPath(hookPath: string): string { + const primary = hookPath + '.backup'; + if (!fs.existsSync(primary)) { + return primary; + } + for (let slot = 1; slot < MAX_BACKUP_SLOTS; slot++) { + const candidate = `${primary}.${slot}`; + if (!fs.existsSync(candidate)) { + return candidate; + } + } + throw new ValidationError( + `Refusing to install: ${MAX_BACKUP_SLOTS} backups of "${path.basename(hookPath)}" already exist.`, + `Remove the unneeded ${path.basename(primary)}* files and retry.`, + ); } /** @@ -137,7 +203,9 @@ export class GitHooksService { * Find git root directory from current path */ static findGitRoot(startPath?: string): string | null { - let currentPath = startPath ?? process.cwd(); + // Resolve first: path.dirname() of a relative path bottoms out at '.', + // which never equals path.parse().root and loops forever. + let currentPath = path.resolve(startPath ?? process.cwd()); // Traverse up the directory tree while (currentPath !== path.parse(currentPath).root) { diff --git a/src/services/glossary.ts b/src/services/glossary.ts index 300d81b..1150d00 100644 --- a/src/services/glossary.ts +++ b/src/services/glossary.ts @@ -20,6 +20,34 @@ function hasSuspiciousChars(name: string): boolean { const LIST_CACHE_TTL_MS = 60_000; +/** + * Characters the glossary TSV wire format reserves as column and row + * separators. A term containing one of them cannot survive the round trip: + * it either shifts columns or fabricates extra entries. + */ +const TERM_SEPARATOR_NAMES: Array<[string, string]> = [ + ['\t', 'tab'], + ['\r', 'carriage return'], + ['\n', 'newline'], +]; + +function findSeparatorChar(text: string): string | null { + for (const [char, name] of TERM_SEPARATOR_NAMES) { + if (text.includes(char)) return name; + } + return null; +} + +function assertNoSeparatorChars(label: string, text: string): void { + const found = findSeparatorChar(text); + if (found) { + throw new ValidationError( + `${label} cannot contain a ${found} character`, + 'Glossary entries are stored as tab-separated rows; split the term into separate entries instead.', + ); + } +} + export class GlossaryService { private client: DeepLClient; private resolutionCache = new Map(); @@ -197,6 +225,8 @@ export class GlossaryService { if (!targetText || targetText.trim() === '') { throw new ValidationError('Target text cannot be empty'); } + assertNoSeparatorChars('Source text', sourceText); + assertNoSeparatorChars('Target text', targetText); await this.mutateEntries(glossaryId, sourceLang, targetLang, (entries) => { if (entries[sourceText] !== undefined) { throw new ValidationError(`Entry "${sourceText}" already exists in glossary`); @@ -221,6 +251,8 @@ export class GlossaryService { if (!newTargetText || newTargetText.trim() === '') { throw new ValidationError('Target text cannot be empty'); } + assertNoSeparatorChars('Source text', sourceText); + assertNoSeparatorChars('Target text', newTargetText); await this.mutateEntries(glossaryId, sourceLang, targetLang, (entries) => { if (entries[sourceText] === undefined) { throw new ConfigError(`Entry "${sourceText}" not found in glossary`); @@ -415,6 +447,10 @@ export class GlossaryService { } const lines = content.split('\n'); + // Pick the dialect once for the whole file. Sniffing per line tab-splits a + // quoted CSV field that happens to contain a tab into garbage columns. + const firstDataLine = lines.map(line => line.trim()).find(line => line !== '') ?? ''; + const isTabSeparated = firstDataLine.includes('\t'); let lineNumber = 0; for (const line of lines) { @@ -426,10 +462,9 @@ export class GlossaryService { continue; } - // Try tab-separated first (TSV is preferred), then comma-separated (CSV) let parts: string[]; - if (trimmed.includes('\t')) { + if (isTabSeparated) { parts = trimmed.split('\t'); } else if (trimmed.includes(',')) { // Use proper CSV parsing for comma-separated values (handles quoted fields) @@ -460,6 +495,14 @@ export class GlossaryService { continue; } + // A quoted CSV field may carry a tab or newline that the TSV wire format + // cannot represent; keeping it would corrupt every following entry. + const separator = findSeparatorChar(source) ?? findSeparatorChar(target); + if (separator) { + Logger.warn(`Line ${lineNumber}: Source or target contains a ${separator} character, skipping`); + continue; + } + // Add to entries (duplicates will overwrite earlier entries) if (Object.hasOwn(entries, source)) { Logger.warn(`Line ${lineNumber}: Duplicate source "${source}", overwriting previous entry`); diff --git a/src/services/voice-stream-session.ts b/src/services/voice-stream-session.ts index 8150c02..abc9aff 100644 --- a/src/services/voice-stream-session.ts +++ b/src/services/voice-stream-session.ts @@ -39,6 +39,8 @@ export class VoiceStreamSession { private streamEnded = false; private ws!: WebSocket; private chunkStreamingResolve: (() => void) | null = null; + private transportError: Error | undefined; + private chunks: AsyncGenerator | undefined; constructor( client: VoiceClient, @@ -71,6 +73,7 @@ export class VoiceStreamSession { } run(chunks: AsyncGenerator): Promise { + this.chunks = chunks; return new Promise((resolve, reject) => { const internalCallbacks = this.createInternalCallbacks(resolve, reject); @@ -84,11 +87,23 @@ export class VoiceStreamSession { this.streamChunks(chunks, reject); }); - this.ws.on('close', () => { this.handleClose(internalCallbacks, reject); }); - this.ws.on('error', (error: Error) => { this.handleError(error, reject); }); + this.attachSocketHandlers(internalCallbacks, reject); }); } + /** + * A socket error is recorded rather than acted on: `ws` always follows an + * error with a close event, and only `handleClose` knows whether a + * reconnect is still available. + */ + private attachSocketHandlers( + internalCallbacks: VoiceStreamCallbacks, + reject: (reason: unknown) => void, + ): void { + this.ws.on('close', () => { this.handleClose(internalCallbacks, reject); }); + this.ws.on('error', (error: Error) => { this.transportError = error; }); + } + private createInternalCallbacks( resolve: (value: VoiceSessionResult) => void, reject: (reason: unknown) => void, @@ -119,6 +134,7 @@ export class VoiceStreamSession { this.streamEnded = true; this.callbacks?.onEndOfStream?.(); this.ws.close(); + this.closeInput(); this.finalizeTranscripts(); resolve({ sessionId: this.session.session_id, @@ -127,10 +143,11 @@ export class VoiceStreamSession { }); }, onError: (error) => { - this.streamEnded = true; this.callbacks?.onError?.(error); - this.ws.close(); - reject(new VoiceError(`Voice streaming error: ${error.error_message} (${error.error_code})`)); + this.fail( + reject, + new VoiceError(`Voice streaming error: ${error.error_message} (${error.error_code})`), + ); }, }; } @@ -148,9 +165,15 @@ export class VoiceStreamSession { this.callbacks?.onReconnecting?.(this.reconnectAttempts); void this.reconnect(internalCallbacks, reject); - } else if (!this.streamEnded) { - reject(new VoiceError('WebSocket closed unexpectedly')); + return; } + + this.fail( + reject, + this.transportError + ? new VoiceError(`WebSocket connection failed: ${this.transportError.message}`) + : new VoiceError('WebSocket closed unexpectedly'), + ); } private async reconnect( @@ -160,6 +183,7 @@ export class VoiceStreamSession { try { const reconnectResponse = await this.client.reconnectSession(this.currentToken); this.currentToken = reconnectResponse.token; + this.transportError = undefined; this.ws = this.client.createWebSocket( reconnectResponse.streaming_url, @@ -167,8 +191,7 @@ export class VoiceStreamSession { internalCallbacks, ); - this.ws.on('close', () => { this.handleClose(internalCallbacks, reject); }); - this.ws.on('error', (error: Error) => { this.handleError(error, reject); }); + this.attachSocketHandlers(internalCallbacks, reject); this.ws.on('open', () => { if (this.chunkStreamingResolve) { @@ -177,12 +200,30 @@ export class VoiceStreamSession { } }); } catch (error) { - reject(error instanceof Error ? error : new VoiceError(String(error))); + this.fail(reject, error instanceof Error ? error : new VoiceError(String(error))); + } + } + + /** + * Settle the session as failed. Waking the chunk pump and closing the + * input generator are both required: without them a library consumer's + * audio source stays suspended at a `yield` forever, holding its fd. + */ + private fail(reject: (reason: unknown) => void, error: Error): void { + this.streamEnded = true; + this.ws.close(); + if (this.chunkStreamingResolve) { + this.chunkStreamingResolve(); + this.chunkStreamingResolve = null; } + this.closeInput(); + reject(error); } - private handleError(error: Error, reject: (reason: unknown) => void): void { - reject(new VoiceError(`WebSocket connection failed: ${error.message}`)); + private closeInput(): void { + const chunks = this.chunks; + this.chunks = undefined; + void chunks?.return(undefined).catch(() => undefined); } private streamChunks( @@ -202,8 +243,7 @@ export class VoiceStreamSession { } this.client.sendEndOfSource(this.ws); } catch (error) { - this.ws.close(); - reject(error instanceof Error ? error : new VoiceError(String(error))); + this.fail(reject, error instanceof Error ? error : new VoiceError(String(error))); } })(); } diff --git a/src/storage/cache.ts b/src/storage/cache.ts index cfa85b8..dd6ef87 100644 --- a/src/storage/cache.ts +++ b/src/storage/cache.ts @@ -16,6 +16,7 @@ export interface CacheServiceOptions { maxSize?: number; // in bytes ttl?: number; // in milliseconds, 0 = disabled busyTimeoutMs?: number; // how long SQLite waits on a locked DB before erroring + enabled?: boolean; // starting state of the cache; defaults to true } export interface CacheStats { @@ -78,7 +79,9 @@ export class CacheService { private maxSize: number; private ttl: number; private busyTimeoutMs: number; - private enabled: boolean = true; + // Seeded from options so the in-memory flag matches the persisted + // `cache.enabled` config; a bare `new CacheService()` stays enabled. + private enabled: boolean; private isClosed: boolean = false; // Seeded to 0 so the first operation of every process sweeps expired rows; // seeding to Date.now() would keep the sweep from ever running in a process @@ -95,6 +98,7 @@ export class CacheService { this.maxSize = options.maxSize ?? DEFAULT_MAX_SIZE; this.ttl = options.ttl ?? DEFAULT_TTL; this.busyTimeoutMs = options.busyTimeoutMs ?? DEFAULT_BUSY_TIMEOUT_MS; + this.enabled = options.enabled ?? true; try { this.openDatabase(dbPath); diff --git a/src/sync/sync-config.ts b/src/sync/sync-config.ts index 3d0cd24..f09f81d 100644 --- a/src/sync/sync-config.ts +++ b/src/sync/sync-config.ts @@ -521,14 +521,27 @@ export function validateSyncConfig(raw: unknown): SyncConfig { }; } -// Single source of truth for merging CLI overrides (--formality, --glossary, -// --model-type, --scan-context, --batch/--no-batch) into a loaded +// Single source of truth for merging CLI overrides (--locale, --formality, +// --glossary, --model-type, --scan-context, --batch/--no-batch) into a loaded // SyncConfig. All guards that cross the YAML-vs-CLI layer boundary live here // so they cannot be bypassed by callers that assemble the config elsewhere. +// --locale is a filter over target_locales, not a specifier, so a value +// outside target_locales is a ConfigError rather than a silent no-op. export function applyCliOverrides( config: SyncConfig, overrides: SyncConfigOverrides, ): SyncConfig { + if (overrides.localeFilter !== undefined) { + const unconfigured = overrides.localeFilter.filter( + (locale) => !config.target_locales.includes(locale), + ); + if (unconfigured.length > 0) { + throw new ConfigError( + `--locale ${unconfigured.join(', ')} not in target_locales (configured: ${config.target_locales.join(', ')})`, + 'Pass only locales listed in target_locales, or add the locale to target_locales in .deepl-sync.yaml.', + ); + } + } if (overrides.formality !== undefined) { config.translation = config.translation ?? {}; config.translation.formality = overrides.formality as Formality; diff --git a/src/sync/sync-glossary-report.ts b/src/sync/sync-glossary-report.ts index aab5d51..c23742a 100644 Binary files a/src/sync/sync-glossary-report.ts and b/src/sync/sync-glossary-report.ts differ diff --git a/src/sync/sync-glossary.ts b/src/sync/sync-glossary.ts index 466853b..cadb2a6 100644 --- a/src/sync/sync-glossary.ts +++ b/src/sync/sync-glossary.ts @@ -1,6 +1,7 @@ import type { GlossaryService } from '../services/glossary.js'; import type { Language } from '../types/index.js'; import { Logger } from '../utils/logger.js'; +import { errorMessage } from '../utils/error-message.js'; export interface SyncGlossaryManagerOptions { sourceLocale: string; @@ -10,13 +11,36 @@ export interface SyncGlossaryManagerOptions { const MAX_TERM_LENGTH = 50; const MIN_KEY_COUNT = 3; +const TERM_FORBIDDEN_CHARS = /[\t\r\n]/; + +/** + * A term the glossary TSV format cannot carry: tabs and newlines are the + * column and row separators, so uploading one either splits the term or + * fabricates a different entry pair. + */ +function isUnusableTerm(text: string): boolean { + return text.trim() === '' || TERM_FORBIDDEN_CHARS.test(text); +} + +/** + * Normalize the way the glossary TSV round trip does — the API returns entries + * parsed back out of TSV, which trims each field. Comparing raw local terms + * against that could never be equal, so every sync re-uploaded the dictionary. + */ +function normalizeForComparison(entries: Record): Map { + const normalized = new Map(); + for (const [source, target] of Object.entries(entries)) { + normalized.set(source.trim(), target.trim()); + } + return normalized; +} function entriesEqual(a: Record, b: Record): boolean { - const aKeys = Object.keys(a); - const bKeys = Object.keys(b); - if (aKeys.length !== bKeys.length) return false; - for (const key of aKeys) { - if (a[key] !== b[key]) return false; + const normalizedA = normalizeForComparison(a); + const normalizedB = normalizeForComparison(b); + if (normalizedA.size !== normalizedB.size) return false; + for (const [key, value] of normalizedA) { + if (normalizedB.get(key) !== value) return false; } return true; } @@ -74,9 +98,17 @@ export class SyncGlossaryManager { } } - if (isConsistent && consistentTranslation !== undefined) { - terms[sourceText] = consistentTranslation; + if (!isConsistent || consistentTranslation === undefined) continue; + + if (isUnusableTerm(sourceText) || isUnusableTerm(consistentTranslation)) { + const sampleKey = keys.values().next().value; + Logger.warn( + `Skipping glossary term from key "${sampleKey}" (${locale}): source or translation is empty or contains a tab, carriage return or newline.`, + ); + continue; } + + terms[sourceText] = consistentTranslation; } if (Object.keys(terms).length > 0) { @@ -105,40 +137,48 @@ export class SyncGlossaryManager { } const name = this.getGlossaryName(targetLocale); - const existing = await this.options.glossaryService.getGlossaryByName(name); const sourceLang = this.options.sourceLocale as Language; const targetLang = targetLocale as Language; - - if (existing) { - const currentEntries = await this.options.glossaryService.getGlossaryEntries( - existing.glossary_id, - sourceLang, - targetLang, - ); - - const localePair = `${this.options.sourceLocale}-${targetLocale}`; - glossaryIds[localePair] = existing.glossary_id; - - if (!entriesEqual(currentEntries, localeTerms)) { - await this.options.glossaryService.updateGlossary(existing.glossary_id, { - dictionaries: [{ - sourceLang, - targetLang, - entries: localeTerms, - }], - }); - Logger.info(`Updated glossary "${name}" (${existing.glossary_id})`); + const localePair = `${this.options.sourceLocale}-${targetLocale}`; + + // One rejected dictionary must not abandon the remaining locales. + try { + const existing = await this.options.glossaryService.getGlossaryByName(name); + + if (existing) { + const currentEntries = await this.options.glossaryService.getGlossaryEntries( + existing.glossary_id, + sourceLang, + targetLang, + ); + + glossaryIds[localePair] = existing.glossary_id; + + if (!entriesEqual(currentEntries, localeTerms)) { + await this.options.glossaryService.updateGlossary(existing.glossary_id, { + dictionaries: [{ + sourceLang, + targetLang, + entries: localeTerms, + }], + }); + Logger.info(`Updated glossary "${name}" (${existing.glossary_id})`); + } + } else { + const created = await this.options.glossaryService.createGlossary( + name, + sourceLang, + [targetLang], + localeTerms, + ); + glossaryIds[localePair] = created.glossary_id; + Logger.info(`Created glossary "${name}" (${created.glossary_id})`); } - } else { - const created = await this.options.glossaryService.createGlossary( - name, - sourceLang, - [targetLang], - localeTerms, + } catch (error) { + delete glossaryIds[localePair]; + Logger.warn( + `Glossary sync failed for ${localePair} (glossary "${name}", ${Object.keys(localeTerms).length} terms): ${errorMessage(error)}`, ); - const localePair = `${this.options.sourceLocale}-${targetLocale}`; - glossaryIds[localePair] = created.glossary_id; - Logger.info(`Created glossary "${name}" (${created.glossary_id})`); } } diff --git a/src/sync/sync-init.ts b/src/sync/sync-init.ts index 05ed50d..2d69d6b 100644 --- a/src/sync/sync-init.ts +++ b/src/sync/sync-init.ts @@ -4,6 +4,7 @@ import * as YAML from 'yaml'; import fg from 'fast-glob'; import { SYNC_CONFIG_FILENAME } from './sync-config.js'; import { safeReadFileSync } from '../utils/safe-read-file.js'; +import { atomicWriteFile } from '../utils/atomic-write.js'; import { createDefaultRegistry, type FormatRegistry } from '../formats/index.js'; export interface DetectedProject { @@ -343,12 +344,22 @@ export function generateSyncConfig(opts: { return YAML.stringify(config); } -export async function writeSyncConfig(rootDir: string, content: string): Promise { - const configPath = path.join(rootDir, SYNC_CONFIG_FILENAME); - await fs.promises.writeFile(configPath, content, 'utf-8'); +/** + * Resolve the config file `deepl sync init` should create. `--sync-config` + * names the file itself (any basename, matching what `loadSyncConfig` accepts), + * so its directory becomes the project root that detection and the generated + * globs are relative to. + */ +export function resolveInitConfigPath(cwd: string, configPath?: string): string { + return configPath ? path.resolve(cwd, configPath) : path.join(cwd, SYNC_CONFIG_FILENAME); +} + +export async function writeSyncConfig(configPath: string, content: string): Promise { + await fs.promises.mkdir(path.dirname(configPath), { recursive: true }); + await atomicWriteFile(configPath, content, 'utf-8'); return configPath; } -export function configExists(rootDir: string): boolean { - return fs.existsSync(path.join(rootDir, SYNC_CONFIG_FILENAME)); +export function configExists(configPath: string): boolean { + return fs.existsSync(configPath); } diff --git a/src/sync/sync-service.ts b/src/sync/sync-service.ts index f432e89..edfe3d6 100644 --- a/src/sync/sync-service.ts +++ b/src/sync/sync-service.ts @@ -24,6 +24,7 @@ import { SyncGlossaryManager } from './sync-glossary.js'; import { resolveTranslationMemoryId } from '../services/translation-memory.js'; import { TmCache } from './tm-cache.js'; import { Logger } from '../utils/logger.js'; +import { errorMessage } from '../utils/error-message.js'; export type SyncProgressEvent = | { type: 'locale-complete'; locale: string; file: string; translated: number; failed: number; totalKeys: number; charactersBilled: number } @@ -321,7 +322,9 @@ export class SyncService { lockFile.glossary_ids = { ...lockFile.glossary_ids, ...glossaryIds }; lockDirty = true; } catch (error) { - Logger.warn('Auto-glossary sync failed:', error); + Logger.warn( + `Auto-glossary sync failed for locales ${effectiveLocales.join(', ')}: ${errorMessage(error)}. Translations were written; the project glossaries were not updated.`, + ); } } diff --git a/src/sync/tms-client.ts b/src/sync/tms-client.ts index 2cf1d1e..ccb2745 100644 --- a/src/sync/tms-client.ts +++ b/src/sync/tms-client.ts @@ -1,11 +1,13 @@ -import { ConfigError, ValidationError } from '../utils/errors.js'; +import { ConfigError, NetworkError, ValidationError } from '../utils/errors.js'; import { sanitizeForTerminal } from '../utils/control-chars.js'; +import { sanitizeUrl } from '../utils/sanitize-url.js'; import { Logger } from '../utils/logger.js'; import type { ExtractedEntry } from '../formats/format.js'; import type { SyncTmsConfig } from './types.js'; const MAX_PULL_VALUE_BYTES = 64 * 1024; export const MAX_PULL_KEY_COUNT = 50000; +export const MAX_PULL_BODY_BYTES = 32 * 1024 * 1024; // eslint-disable-next-line no-control-regex -- intentional: checking for control chars in untrusted TMS-returned keys const KEY_FORBIDDEN_CHARS = /[\x00-\x1f\x7f/\\]/; // eslint-disable-next-line no-control-regex -- intentional: strip control chars from untrusted TMS-returned values before they reach the filesystem @@ -54,6 +56,14 @@ export function sanitizePullKeysResponse(raw: unknown): Record { return result; } +/** A TMS request that hit the client-side timeout; classified as a network error (exit 5). */ +export class TmsTimeoutError extends NetworkError { + constructor(message: string) { + super(message, 'Raise timeout_ms in the tms: block of .deepl-sync.yaml, or check that the TMS server is reachable.'); + this.name = 'TmsTimeoutError'; + } +} + export interface TmsClientRetryOptions { maxAttempts?: number; baseDelayMs?: number; @@ -124,6 +134,47 @@ async function readErrorBody(response: Response): Promise { } } +function oversizedBodyError(): ValidationError { + return new ValidationError( + `TMS pull response exceeds the ${MAX_PULL_BODY_BYTES / (1024 * 1024)}MiB response size limit`, + 'Partition the TMS export by locale, or paginate the pull.', + ); +} + +/** + * Read and parse a JSON body without buffering more than the cap. The key and + * value limits in sanitizePullKeysResponse only apply after a parse, so an + * unbounded body would already have been materialized by then. + */ +async function readJsonBounded(response: Response): Promise { + const declared = Number(response.headers?.get('content-length')); + if (Number.isFinite(declared) && declared > MAX_PULL_BODY_BYTES) { + throw oversizedBodyError(); + } + + const body = response.body; + if (!body || typeof body.getReader !== 'function') { + return await response.json(); + } + + const reader = body.getReader(); + const chunks: Uint8Array[] = []; + let total = 0; + for (;;) { + const { done, value } = await reader.read(); + if (done) break; + if (!value) continue; + total += value.byteLength; + if (total > MAX_PULL_BODY_BYTES) { + await reader.cancel(); + throw oversizedBodyError(); + } + chunks.push(value); + } + + return JSON.parse(Buffer.concat(chunks).toString('utf-8')); +} + export class TmsClient { private readonly timeoutMs: number; private readonly retry: Required; @@ -157,11 +208,9 @@ export class TmsClient { }); } catch (err) { if (isAbortError(err)) { - const timeoutErr = new Error( - `TMS request timed out after ${this.timeoutMs}ms: ${method} ${url}`, + throw new TmsTimeoutError( + `TMS request timed out after ${this.timeoutMs}ms: ${method} ${sanitizeUrl(url)}`, ); - timeoutErr.name = 'TmsTimeoutError'; - throw timeoutErr; } throw err; } finally { @@ -169,18 +218,36 @@ export class TmsClient { } } - private async request(method: string, path: string, body?: unknown): Promise { + /** + * Join the configured server URL with an API path through the URL API, so a + * trailing slash cannot produce "//" and a base path is preserved. + */ + private buildUrl(path: string): string { let parsedUrl: URL; try { parsedUrl = new URL(this.options.serverUrl); } catch { - throw new ConfigError(`Invalid TMS server URL: ${this.options.serverUrl}`); + throw new ConfigError(`Invalid TMS server URL: ${sanitizeUrl(this.options.serverUrl)}`); } + const isLocalhost = parsedUrl.hostname === 'localhost' || parsedUrl.hostname === '127.0.0.1'; if (parsedUrl.protocol !== 'https:' && !isLocalhost) { throw new ConfigError('TMS server URL must use HTTPS'); } - const url = `${this.options.serverUrl}/api/projects/${encodeURIComponent(this.options.projectId)}${path}`; + if (parsedUrl.search || parsedUrl.hash) { + throw new ConfigError( + `TMS server URL must not contain a query string or fragment: ${sanitizeUrl(this.options.serverUrl)}`, + 'Set server: to the bare origin (plus a base path if your TMS is mounted under one).', + ); + } + + const basePath = parsedUrl.pathname.replace(/\/+$/, ''); + const projectPath = `${basePath}/api/projects/${encodeURIComponent(this.options.projectId)}`; + return new URL(`${projectPath}${path}`, parsedUrl).toString(); + } + + private async request(method: string, path: string, body?: unknown): Promise { + const url = this.buildUrl(path); let lastError: unknown; for (let attempt = 0; attempt < this.retry.maxAttempts; attempt++) { @@ -189,8 +256,7 @@ export class TmsClient { response = await this.fetchOnce(url, method, body); } catch (err) { lastError = err; - const isTimeout = err instanceof Error && err.name === 'TmsTimeoutError'; - const retriable = isTimeout || isRetriableNetworkError(err); + const retriable = err instanceof TmsTimeoutError || isRetriableNetworkError(err); if (!retriable || attempt === this.retry.maxAttempts - 1) throw err; const delay = computeBackoffDelay(attempt, this.retry.baseDelayMs, this.retry.maxDelayMs, this.retry.jitter); await sleep(delay); @@ -240,7 +306,7 @@ export class TmsClient { async pullKeys(locale: string): Promise> { const resp = await this.request('GET', `/keys/export?format=json&locale=${encodeURIComponent(locale)}`); - return sanitizePullKeysResponse(await resp.json()); + return sanitizePullKeysResponse(await readJsonBounded(resp)); } async getProjectStatus(): Promise { diff --git a/src/utils/sanitize-url.ts b/src/utils/sanitize-url.ts new file mode 100644 index 0000000..3f43df8 --- /dev/null +++ b/src/utils/sanitize-url.ts @@ -0,0 +1,16 @@ +/** + * Redact userinfo before a URL reaches a log line or an error message. + * Unparseable input is reported as such rather than echoed back. + */ +export function sanitizeUrl(url: string): string { + try { + const parsed = new URL(url); + if (parsed.username || parsed.password) { + parsed.username = '***'; + parsed.password = '***'; + } + return parsed.toString(); + } catch { + return '[invalid URL]'; + } +} diff --git a/tests/e2e/cli-cache.e2e.test.ts b/tests/e2e/cli-cache.e2e.test.ts index fbca295..117a21a 100644 --- a/tests/e2e/cli-cache.e2e.test.ts +++ b/tests/e2e/cli-cache.e2e.test.ts @@ -13,6 +13,45 @@ describe('Cache Command E2E', () => { testConfig.cleanup(); }); + // Each assertion below runs in a process separate from the one that + // toggled the cache, so an in-memory-only toggle cannot satisfy them. + describe('enable/disable persistence across processes', () => { + const toggleConfig = createTestConfigDir('e2e-cache-toggle'); + const toggle = makeNodeRunCLI(toggleConfig.path); + + afterAll(() => { + toggleConfig.cleanup(); + }); + + it('should report disabled from a later invocation after cache disable', () => { + expect(toggle.runCLIAll('cache disable')).toContain('Cache disabled'); + + expect(toggle.runCLI('cache stats')).toContain('Cache Status: disabled'); + expect(toggle.runCLI('config get cache.enabled').trim()).toBe('false'); + }); + + it('should report enabled from a later invocation after cache enable', () => { + toggle.runCLIAll('cache disable'); + expect(toggle.runCLIAll('cache enable')).toContain('Cache enabled'); + + expect(toggle.runCLI('cache stats')).toContain('Cache Status: enabled'); + expect(toggle.runCLI('config get cache.enabled').trim()).toBe('true'); + }); + + it('should report disabled when cache.enabled is set through config directly', () => { + toggle.runCLIAll('config set cache.enabled false'); + + expect(toggle.runCLI('cache stats')).toContain('Cache Status: disabled'); + }); + + it('should preserve --max-size while persisting the enabled flag', () => { + toggle.runCLIAll('cache enable --max-size 100M'); + + expect(toggle.runCLI('config get cache.enabled').trim()).toBe('true'); + expect(toggle.runCLI('config get cache.maxSize').trim()).toBe(String(100 * 1024 * 1024)); + }); + }); + describe('cache --help', () => { it('should display help text', () => { const output = runCLI('cache --help'); diff --git a/tests/e2e/cli-http-options.e2e.test.ts b/tests/e2e/cli-http-options.e2e.test.ts new file mode 100644 index 0000000..ee294dc --- /dev/null +++ b/tests/e2e/cli-http-options.e2e.test.ts @@ -0,0 +1,114 @@ +/** + * E2E Tests for the --timeout / --max-retries global options. + * Uses a server that accepts requests and never answers, so the client + * aborts locally — the case that used to re-submit a billable POST. + */ + +import { spawn, ChildProcess } from 'child_process'; +import * as fs from 'fs'; +import * as path from 'path'; +import { createTestConfigDir, createTestDir, makeNodeRunCLI } from '../helpers'; + +describe('CLI HTTP options E2E', () => { + const testConfig = createTestConfigDir('e2e-http-options'); + const testFiles = createTestDir('e2e-http-options-files'); + let runner: ReturnType; + let stallServer: ChildProcess; + let baseUrl: string; + let countFile: string; + + function startStallServer(): Promise { + return new Promise((resolve, reject) => { + const serverScript = path.join(__dirname, 'stall-server.cjs'); + const child = spawn('node', [serverScript, countFile], { + stdio: ['pipe', 'pipe', 'pipe'], + env: { ...process.env }, + }); + + stallServer = child; + let output = ''; + + child.stdout.on('data', (data: Buffer) => { + output += data.toString(); + const match = output.match(/PORT=(\d+)/); + if (match) { + resolve(parseInt(match[1]!, 10)); + } + }); + + child.on('error', reject); + setTimeout(() => reject(new Error('Stall server did not start within 15s')), 15000); + }); + } + + function requestCount(): number { + return parseInt(fs.readFileSync(countFile, 'utf-8'), 10); + } + + beforeAll(async () => { + runner = makeNodeRunCLI(testConfig.path, { apiKey: 'test-api-key' }); + countFile = path.join(testFiles.path, 'requests.txt'); + const port = await startStallServer(); + baseUrl = `http://127.0.0.1:${port}`; + }); + + afterAll(() => { + stallServer.kill(); + testConfig.cleanup(); + testFiles.cleanup(); + }); + + it('exits 5 (network error) on a client-side timeout instead of 6', () => { + const result = runner.runCLIExpectError( + `--timeout 500 translate "Hello" --to es --api-url ${baseUrl}`, + { timeout: 30000 }, + ); + + expect(result.status).toBe(5); + expect(result.output).toMatch(/timeout|network/i); + }); + + it('does not re-submit the translate POST after a client-side timeout', () => { + const before = requestCount(); + + runner.runCLIExpectError( + `--timeout 500 translate "Bonjour" --to es --api-url ${baseUrl}`, + { timeout: 30000 }, + ); + + expect(requestCount() - before).toBe(1); + }); + + it('honours --timeout so the run is bounded well below the 30s default', () => { + const start = Date.now(); + const result = runner.runCLIExpectError( + `--timeout 500 --max-retries 0 translate "Guten Tag" --to es --api-url ${baseUrl}`, + { timeout: 30000 }, + ); + const elapsed = Date.now() - start; + + expect(result.status).toBe(5); + expect(elapsed).toBeLessThan(20000); + }); + + it('rejects a non-numeric --timeout with exit 6', () => { + const result = runner.runCLIExpectError('--timeout abc translate "Hello" --to es'); + + expect(result.status).toBe(6); + expect(result.output).toMatch(/--timeout/); + }); + + it('rejects a negative --max-retries with exit 6', () => { + const result = runner.runCLIExpectError('--max-retries -1 translate "Hello" --to es'); + + expect(result.status).toBe(6); + expect(result.output).toMatch(/--max-retries/); + }); + + it('documents both options in the top-level help', () => { + const output = runner.runCLIAll('--help'); + + expect(output).toContain('--timeout'); + expect(output).toContain('--max-retries'); + }); +}); diff --git a/tests/e2e/cli-sync-option-routing.e2e.test.ts b/tests/e2e/cli-sync-option-routing.e2e.test.ts new file mode 100644 index 0000000..2d6927d --- /dev/null +++ b/tests/e2e/cli-sync-option-routing.e2e.test.ts @@ -0,0 +1,309 @@ +/** + * E2E tests for --locale / --sync-config routing across `deepl sync` and its + * subcommands. + * + * Commander binds an invocation-line flag to the nearest command that declares + * it, and the parent `sync` command is reached first even when the flag trails + * the subcommand name. Both flags are declared on the parent and on the + * subcommands, so a subcommand that reads only its own option store sees + * `undefined`. These tests drive the real CLI so the parent/child binding is + * exercised — asserting on the handlers directly would pass either way. + * + * Also covers the ConfigError documented at docs/API.md for a --locale value + * that is not in `target_locales`. + */ + +import { spawnSync, SpawnSyncReturns } from 'child_process'; +import * as fs from 'fs'; +import * as path from 'path'; +import { createTestConfigDir, createTestDir } from '../helpers'; + +const CLI_PATH = path.join(process.cwd(), 'dist/cli/index.js'); + +describe('CLI sync option routing E2E', () => { + const testConfig = createTestConfigDir('e2e-sync-routing'); + const testFiles = createTestDir('e2e-sync-routing-files'); + + function writeConfig(configDir: string): void { + const config = { + auth: { apiKey: 'mock-api-key-for-testing:fx' }, + api: { baseUrl: 'http://127.0.0.1:1/', usePro: false }, + defaults: { targetLangs: [], formality: 'default', preserveFormatting: true }, + cache: { enabled: false, maxSize: 1048576, ttl: 2592000 }, + output: { format: 'text', verbose: false, color: false }, + watch: { debounceMs: 500, autoCommit: false, pattern: '*.md' }, + }; + fs.writeFileSync(path.join(configDir, 'config.json'), JSON.stringify(config, null, 2)); + } + + function writeSyncConfig(projectDir: string, locales: string[] = ['de', 'fr']): string { + const yaml = [ + 'version: 1', + 'source_locale: en', + 'target_locales:', + ...locales.map((l) => ` - ${l}`), + 'buckets:', + ' json:', + ' include:', + ' - "locales/en.json"', + ].join('\n') + '\n'; + const configPath = path.join(projectDir, '.deepl-sync.yaml'); + fs.writeFileSync(configPath, yaml); + return configPath; + } + + function writeSourceFile(projectDir: string): void { + const dir = path.join(projectDir, 'locales'); + fs.mkdirSync(dir, { recursive: true }); + fs.writeFileSync( + path.join(dir, 'en.json'), + JSON.stringify({ greeting: 'Hello', farewell: 'Goodbye' }, null, 2) + '\n', + ); + } + + interface Run { + status: number; + stdout: string; + stderr: string; + output: string; + } + + function runCli(args: string[], cwd: string = testFiles.path): Run { + const result: SpawnSyncReturns = spawnSync('node', [CLI_PATH, ...args], { + encoding: 'utf-8', + cwd, + env: { + ...process.env, + DEEPL_CONFIG_DIR: testConfig.path, + DEEPL_API_KEY: 'mock-api-key-for-testing:fx', + NO_COLOR: '1', + CI: undefined, + }, + stdio: ['ignore', 'pipe', 'pipe'], + timeout: 15000, + }); + const stdout = result.stdout ?? ''; + const stderr = result.stderr ?? ''; + return { status: result.status ?? 1, stdout, stderr, output: stdout + stderr }; + } + + beforeAll(() => { + writeConfig(testConfig.path); + }); + + beforeEach(() => { + for (const entry of fs.readdirSync(testFiles.path)) { + fs.rmSync(path.join(testFiles.path, entry), { recursive: true, force: true }); + } + }); + + afterAll(() => { + testConfig.cleanup(); + testFiles.cleanup(); + }); + + describe('--locale reaches the subcommand handler', () => { + it('narrows `sync status` output to the requested locale', () => { + writeSyncConfig(testFiles.path, ['de', 'fr']); + writeSourceFile(testFiles.path); + + const run = runCli(['sync', 'status', '--locale', 'de']); + + expect(run.status).toBe(0); + expect(run.output).toMatch(/\bde\b/); + expect(run.output).not.toMatch(/^\s+fr\s/m); + }); + + it('narrows `sync export` XLIFF output to the requested locale', () => { + writeSyncConfig(testFiles.path, ['de', 'fr']); + writeSourceFile(testFiles.path); + + const run = runCli(['sync', 'export', '--locale', 'de']); + + expect(run.status).toBe(0); + expect(run.stdout).toContain('target-language="de"'); + expect(run.stdout).not.toContain('target-language="fr"'); + }); + }); + + describe('--sync-config reaches the subcommand handler', () => { + for (const sub of ['status', 'validate', 'export', 'audit', 'resolve', 'push', 'pull']) { + it(`\`sync ${sub} --sync-config \` fails with ConfigError (exit 7)`, () => { + writeSyncConfig(testFiles.path, ['de', 'fr']); + writeSourceFile(testFiles.path); + + const missing = path.join(testFiles.path, 'nope', 'absent.yaml'); + const run = runCli(['sync', sub, '--sync-config', missing]); + + expect(run.status).toBe(7); + expect(run.output).toContain(missing); + }); + } + + it('honors --sync-config pointing at a config outside the auto-detected one', () => { + writeSyncConfig(testFiles.path, ['de', 'fr']); + writeSourceFile(testFiles.path); + + const altRoot = path.join(testFiles.path, 'alt'); + fs.mkdirSync(altRoot, { recursive: true }); + writeSyncConfig(altRoot, ['it']); + writeSourceFile(altRoot); + const altConfig = path.join(altRoot, '.deepl-sync.yaml'); + + const run = runCli(['sync', 'status', '--sync-config', altConfig]); + + expect(run.status).toBe(0); + expect(run.output).toMatch(/\bit\b/); + expect(run.output).not.toMatch(/^\s+de\s/m); + }); + }); + + describe('--locale must name a configured target locale', () => { + it('exits 7 on `sync --locale ` naming the offending and configured locales', () => { + writeSyncConfig(testFiles.path, ['de', 'fr']); + writeSourceFile(testFiles.path); + + const run = runCli(['sync', '--locale', 'es', '--dry-run']); + + expect(run.status).toBe(7); + expect(run.output).toContain('es'); + expect(run.output).toContain('de, fr'); + expect(run.output).not.toContain('Sync complete'); + }); + + it('exits 7 on `sync status --locale `', () => { + writeSyncConfig(testFiles.path, ['de', 'fr']); + writeSourceFile(testFiles.path); + + const run = runCli(['sync', 'status', '--locale', 'es']); + + expect(run.status).toBe(7); + expect(run.output).toContain('es'); + expect(run.output).toContain('de, fr'); + }); + + it('exits 7 on `sync validate --locale ` instead of reporting all-passed', () => { + writeSyncConfig(testFiles.path, ['de', 'fr']); + writeSourceFile(testFiles.path); + + const run = runCli(['sync', 'validate', '--locale', 'zz']); + + expect(run.status).toBe(7); + expect(run.output.toLowerCase()).not.toContain('passed validation'); + }); + + it('exits 7 on `sync export --locale ` instead of exporting every locale', () => { + writeSyncConfig(testFiles.path, ['de', 'fr']); + writeSourceFile(testFiles.path); + + const run = runCli(['sync', 'export', '--locale', 'zz']); + + expect(run.status).toBe(7); + expect(run.stdout).not.toContain(' { + writeSyncConfig(testFiles.path, ['de', 'fr']); + writeSourceFile(testFiles.path); + + const run = runCli(['sync', 'status', '--locale', 'es', '--format', 'json']); + + expect(run.status).toBe(7); + const envelope = JSON.parse(run.stderr.trim()) as { + ok: boolean; + error: { code: string }; + exitCode: number; + }; + expect(envelope.ok).toBe(false); + expect(envelope.error.code).toBe('ConfigError'); + expect(envelope.exitCode).toBe(7); + }); + + it('still accepts a configured locale on the root command', () => { + writeSyncConfig(testFiles.path, ['de', 'fr']); + writeSourceFile(testFiles.path); + + const run = runCli(['sync', '--locale', 'de', '--dry-run']); + + expect(run.status).toBe(0); + expect(run.output).toContain('dry-run'); + }); + + it('still accepts a configured locale on a subcommand', () => { + writeSyncConfig(testFiles.path, ['de', 'fr']); + writeSourceFile(testFiles.path); + + const run = runCli(['sync', 'status', '--locale', 'de,fr']); + + expect(run.status).toBe(0); + expect(run.output).toMatch(/\bde\b/); + expect(run.output).toMatch(/\bfr\b/); + }); + }); + + describe('sync init --sync-config', () => { + it('writes the config at the requested path', () => { + const altRoot = path.join(testFiles.path, 'alt'); + fs.mkdirSync(altRoot, { recursive: true }); + writeSourceFile(altRoot); + + const target = path.join(altRoot, 'custom-sync.yaml'); + const run = runCli([ + 'sync', 'init', + '--sync-config', target, + '--source-locale', 'en', + '--target-locales', 'de', + '--file-format', 'json', + '--path', 'locales/en.json', + ]); + + expect(run.status).toBe(0); + expect(fs.existsSync(target)).toBe(true); + expect(fs.existsSync(path.join(testFiles.path, '.deepl-sync.yaml'))).toBe(false); + expect(fs.readFileSync(target, 'utf-8')).toContain('source_locale: en'); + }); + + it('does not treat an unrelated cwd config as the already-exists case', () => { + writeSyncConfig(testFiles.path, ['de', 'fr']); + writeSourceFile(testFiles.path); + + const altRoot = path.join(testFiles.path, 'alt'); + fs.mkdirSync(altRoot, { recursive: true }); + writeSourceFile(altRoot); + + const target = path.join(altRoot, '.deepl-sync.yaml'); + const run = runCli([ + 'sync', 'init', + '--sync-config', target, + '--source-locale', 'en', + '--target-locales', 'it', + '--file-format', 'json', + '--path', 'locales/en.json', + ]); + + expect(run.status).toBe(0); + expect(run.output).not.toContain('already exists'); + expect(fs.existsSync(target)).toBe(true); + expect(fs.readFileSync(target, 'utf-8')).toContain('it'); + }); + + it('reports the already-exists case against the --sync-config path', () => { + const altRoot = path.join(testFiles.path, 'alt'); + fs.mkdirSync(altRoot, { recursive: true }); + writeSourceFile(altRoot); + const target = writeSyncConfig(altRoot, ['de']); + + const run = runCli([ + 'sync', 'init', + '--sync-config', target, + '--source-locale', 'en', + '--target-locales', 'it', + '--file-format', 'json', + '--path', 'locales/en.json', + ]); + + expect(run.output).toContain('already exists'); + expect(fs.readFileSync(target, 'utf-8')).not.toContain('it'); + }); + }); +}); diff --git a/tests/e2e/cli-sync.e2e.test.ts b/tests/e2e/cli-sync.e2e.test.ts index d2180e5..e137836 100644 --- a/tests/e2e/cli-sync.e2e.test.ts +++ b/tests/e2e/cli-sync.e2e.test.ts @@ -1069,6 +1069,76 @@ describe('CLI Sync E2E', () => { // Clean up the nested dirs created above so the next test's beforeEach // doesn't inherit them (beforeEach only wipes locales/ at the top). }); + + it('reports a not-yet-created locale file as missing instead of an inconsistency', () => { + const sourceDir = path.join(testFiles.path, 'locales', 'en'); + fs.mkdirSync(sourceDir, { recursive: true }); + fs.writeFileSync(path.join(sourceDir, 'common.json'), JSON.stringify({ greeting: 'Dashboard' }, null, 2)); + fs.writeFileSync(path.join(sourceDir, 'admin.json'), JSON.stringify({ header: 'Dashboard' }, null, 2)); + + // Only one of the two German files exists yet. + const targetDir = path.join(testFiles.path, 'locales', 'de'); + fs.mkdirSync(targetDir, { recursive: true }); + fs.writeFileSync(path.join(targetDir, 'common.json'), JSON.stringify({ greeting: 'Armaturenbrett' }, null, 2)); + + fs.writeFileSync( + path.join(testFiles.path, '.deepl-sync.yaml'), + [ + 'version: 1', + 'source_locale: en', + 'target_locales:', + ' - de', + 'buckets:', + ' json:', + ' include:', + ' - "locales/en/*.json"', + '', + ].join('\n'), + ); + + const lockContent = { + _comment: 'test', + version: 1, + generated_at: '2026-04-19T00:00:00Z', + source_locale: 'en', + entries: { + 'locales/en/common.json': { + greeting: { + source_hash: 'sh', + source_text: 'Dashboard', + translations: { + de: { hash: 'de-hash-a', translated_at: '2026-04-19T00:00:00Z', status: 'translated' }, + }, + }, + }, + 'locales/en/admin.json': { + header: { + source_hash: 'sh', + source_text: 'Dashboard', + translations: { + de: { hash: 'de-hash-a', translated_at: '2026-04-19T00:00:00Z', status: 'translated' }, + }, + }, + }, + }, + stats: { total_keys: 2, total_translations: 2, last_sync: '2026-04-19T00:00:00Z' }, + }; + fs.writeFileSync(path.join(testFiles.path, '.deepl-sync.lock'), JSON.stringify(lockContent, null, 2)); + + const output = runSyncAll('audit --format json'); + const jsonMatch = output.match(/\{[\s\S]*\}/); + expect(jsonMatch).not.toBeNull(); + const parsed = JSON.parse(jsonMatch![0]) as { + inconsistencies: Array<{ translations: string[] }>; + missingTargets: Array<{ filePath: string; locale: string }>; + }; + + expect(parsed.inconsistencies).toHaveLength(0); + expect(parsed.missingTargets).toEqual([ + { filePath: 'locales/en/admin.json', locale: 'de' }, + ]); + expect(output).not.toContain('de-hash-a'); + }); }); describe('sync export output safety', () => { diff --git a/tests/e2e/stall-server.cjs b/tests/e2e/stall-server.cjs new file mode 100644 index 0000000..704d1c2 --- /dev/null +++ b/tests/e2e/stall-server.cjs @@ -0,0 +1,38 @@ +/** + * A server that accepts requests and never answers them. + * + * Prints `PORT=` on stdout once listening, and rewrites the file given + * as argv[2] with the number of requests received so far. Runs in its own + * process because the test driver blocks its own event loop on execSync and + * would otherwise never accept the connection. + * + * Usage: node tests/e2e/stall-server.cjs /path/to/count-file + */ + +const http = require('http'); +const fs = require('fs'); + +const countFile = process.argv[2]; +let requests = 0; + +function record() { + requests++; + if (countFile) { + fs.writeFileSync(countFile, String(requests)); + } +} + +const server = http.createServer(() => { + record(); + // Never respond: the client must abort on its own timeout. +}); + +// Ignore mid-flight socket errors from clients that abort. +server.on('clientError', () => {}); + +server.listen(0, '127.0.0.1', () => { + if (countFile) { + fs.writeFileSync(countFile, '0'); + } + process.stdout.write(`PORT=${server.address().port}\n`); +}); diff --git a/tests/integration/deepl-client.integration.test.ts b/tests/integration/deepl-client.integration.test.ts index e602658..786585e 100644 --- a/tests/integration/deepl-client.integration.test.ts +++ b/tests/integration/deepl-client.integration.test.ts @@ -309,9 +309,7 @@ describe('DeepLClient Integration', () => { const client = new DeepLClient(API_KEY, { maxRetries: 2 }); clients.push(client); - // Mock all retry attempts (initial + 2 retries = 3 total) - nock(FREE_API_URL).post('/v2/translate').reply(503, { message: 'Service temporarily unavailable' }); - nock(FREE_API_URL).post('/v2/translate').reply(503, { message: 'Service temporarily unavailable' }); + // A translate POST is not replayed, so one attempt is all the server sees. nock(FREE_API_URL).post('/v2/translate').reply(503, { message: 'Service temporarily unavailable' }); await expect(client.translate('Hello', { targetLang: 'es' })).rejects.toThrow( @@ -341,21 +339,35 @@ describe('DeepLClient Integration', () => { ); }); - it('should retry on 500 errors', async () => { + it('should retry an idempotent request on 500 errors', async () => { const client = new DeepLClient(API_KEY, { maxRetries: 2 }); clients.push(client); // First two attempts fail with 500, third succeeds - nock(FREE_API_URL).post('/v2/translate').reply(500, 'Internal Server Error'); - nock(FREE_API_URL).post('/v2/translate').reply(500, 'Internal Server Error'); + nock(FREE_API_URL).get('/v2/usage').reply(500, 'Internal Server Error'); + nock(FREE_API_URL).get('/v2/usage').reply(500, 'Internal Server Error'); nock(FREE_API_URL) + .get('/v2/usage') + .reply(200, { character_count: 10, character_limit: 100 }); + + const usage = await client.getUsage(); + expect(usage.characterCount).toBe(10); + }); + + it('should not replay a translate request on 500 errors', async () => { + const client = new DeepLClient(API_KEY, { maxRetries: 2 }); + clients.push(client); + + const scope = nock(FREE_API_URL) .post('/v2/translate') - .reply(200, { - translations: [{ text: 'Hola' }], - }); + .reply(500, { message: 'Internal Server Error' }); - const result = await client.translate('Hello', { targetLang: 'es' }); - expect(result.text).toBe('Hola'); + await expect( + client.translate('Hello', { targetLang: 'es' }) + ).rejects.toThrow(/Server error \(500\)/); + + expect(scope.isDone()).toBe(true); + expect(nock.pendingMocks()).toHaveLength(0); }); it('should NOT retry on 4xx errors', async () => { diff --git a/tests/integration/file-translation.integration.test.ts b/tests/integration/file-translation.integration.test.ts index 454a402..c1eadbc 100644 --- a/tests/integration/file-translation.integration.test.ts +++ b/tests/integration/file-translation.integration.test.ts @@ -119,7 +119,6 @@ describe('FileTranslation Integration', () => { nock(DEEPL_FREE_API_URL) .post('/v2/translate') - .times(4) .reply(503, { message: 'Service unavailable' }); await expect( diff --git a/tests/integration/git-hooks-resolution.integration.test.ts b/tests/integration/git-hooks-resolution.integration.test.ts new file mode 100644 index 0000000..7e770d2 --- /dev/null +++ b/tests/integration/git-hooks-resolution.integration.test.ts @@ -0,0 +1,196 @@ +/** + * Integration Tests for git hooks directory resolution + * Covers core.hooksPath repos, linked worktrees, submodules and backup safety + * against real git repositories created in temp directories. + */ + +import * as fs from 'fs'; +import * as path from 'path'; +import * as os from 'os'; +import { execFileSync } from 'child_process'; +import { GitHooksService } from '../../src/services/git-hooks.js'; + +function git(cwd: string, ...args: string[]): string { + return execFileSync('git', args, { + cwd, + encoding: 'utf-8', + env: { + ...process.env, + GIT_CONFIG_GLOBAL: '/dev/null', + GIT_CONFIG_SYSTEM: '/dev/null', + GIT_AUTHOR_NAME: 'Test', + GIT_AUTHOR_EMAIL: 'test@example.com', + GIT_COMMITTER_NAME: 'Test', + GIT_COMMITTER_EMAIL: 'test@example.com', + }, + }); +} + +function initRepo(dir: string): void { + fs.mkdirSync(dir, { recursive: true }); + git(dir, 'init', '-q', '.'); +} + +describe('GitHooksService hooks directory resolution', () => { + let tmpRoot: string; + + beforeEach(() => { + tmpRoot = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), 'deepl-hooks-res-'))); + }); + + afterEach(() => { + fs.rmSync(tmpRoot, { recursive: true, force: true }); + }); + + describe('core.hooksPath', () => { + it('should install into the effective hooks path when core.hooksPath is set', () => { + const repo = path.join(tmpRoot, 'husky-repo'); + initRepo(repo); + fs.mkdirSync(path.join(repo, '.husky', '_'), { recursive: true }); + git(repo, 'config', 'core.hooksPath', '.husky/_'); + + const service = new GitHooksService(path.join(repo, '.git')); + service.install('pre-commit'); + + expect(fs.existsSync(path.join(repo, '.husky', '_', 'pre-commit'))).toBe(true); + expect(fs.existsSync(path.join(repo, '.git', 'hooks', 'pre-commit'))).toBe(false); + }); + + it('should report the effective hooks path from getHookPath', () => { + const repo = path.join(tmpRoot, 'husky-path'); + initRepo(repo); + git(repo, 'config', 'core.hooksPath', '.husky/_'); + + const service = new GitHooksService(path.join(repo, '.git')); + + expect(service.getHookPath('pre-push')).toBe(path.join(repo, '.husky', '_', 'pre-push')); + }); + + it('should have list() reflect hooks installed at the effective path', () => { + const repo = path.join(tmpRoot, 'husky-list'); + initRepo(repo); + git(repo, 'config', 'core.hooksPath', 'hooks-dir'); + + const service = new GitHooksService(path.join(repo, '.git')); + service.install('commit-msg'); + + expect(fs.existsSync(path.join(repo, 'hooks-dir', 'commit-msg'))).toBe(true); + expect(service.list()['commit-msg']).toBe(true); + expect(service.isInstalled('commit-msg')).toBe(true); + }); + + it('should uninstall from the effective hooks path', () => { + const repo = path.join(tmpRoot, 'husky-uninstall'); + initRepo(repo); + git(repo, 'config', 'core.hooksPath', '.husky/_'); + + const service = new GitHooksService(path.join(repo, '.git')); + service.install('pre-commit'); + service.uninstall('pre-commit'); + + expect(fs.existsSync(path.join(repo, '.husky', '_', 'pre-commit'))).toBe(false); + }); + }); + + describe('.git as a file', () => { + it('should install into the shared hooks directory from a linked worktree', () => { + const repo = path.join(tmpRoot, 'main-repo'); + initRepo(repo); + git(repo, 'commit', '-q', '--allow-empty', '-m', 'init'); + const linked = path.join(tmpRoot, 'linked-wt'); + git(repo, 'worktree', 'add', '-q', linked, '-b', 'feature'); + + const gitFile = path.join(linked, '.git'); + expect(fs.statSync(gitFile).isFile()).toBe(true); + + const service = new GitHooksService(gitFile); + expect(() => service.install('pre-commit')).not.toThrow(); + + expect(fs.existsSync(path.join(repo, '.git', 'hooks', 'pre-commit'))).toBe(true); + expect(service.isInstalled('pre-commit')).toBe(true); + }); + + it('should install into the submodule git directory when .git is a pointer file', () => { + const upstream = path.join(tmpRoot, 'upstream'); + initRepo(upstream); + fs.writeFileSync(path.join(upstream, 'README.md'), '# upstream\n'); + git(upstream, 'add', '.'); + git(upstream, 'commit', '-q', '-m', 'init'); + + const parent = path.join(tmpRoot, 'parent'); + initRepo(parent); + git(parent, 'commit', '-q', '--allow-empty', '-m', 'init'); + git(parent, '-c', 'protocol.file.allow=always', 'submodule', 'add', '-q', upstream, 'sub'); + + const subGitFile = path.join(parent, 'sub', '.git'); + expect(fs.statSync(subGitFile).isFile()).toBe(true); + + const service = new GitHooksService(subGitFile); + expect(() => service.install('pre-commit')).not.toThrow(); + + const expected = path.join(parent, '.git', 'modules', 'sub', 'hooks', 'pre-commit'); + expect(fs.existsSync(expected)).toBe(true); + expect(service.isInstalled('pre-commit')).toBe(true); + }); + }); + + describe('backup safety', () => { + it('should not clobber an existing .backup on repeat install', () => { + const repo = path.join(tmpRoot, 'backup-repo'); + initRepo(repo); + const hookPath = path.join(repo, '.git', 'hooks', 'pre-commit'); + fs.writeFileSync(hookPath, '#!/bin/sh\n# original user hook\nexit 0\n'); + + const service = new GitHooksService(path.join(repo, '.git')); + const first = service.install('pre-commit'); + expect(first.backupPath).toBe(hookPath + '.backup'); + expect(fs.readFileSync(hookPath + '.backup', 'utf-8')).toContain('original user hook'); + + // A third-party tool rewrites the hook, then deepl installs again. + fs.writeFileSync(hookPath, '#!/bin/sh\n# husky wrapper\nexit 0\n'); + const second = service.install('pre-commit'); + + expect(fs.readFileSync(hookPath + '.backup', 'utf-8')).toContain('original user hook'); + expect(second.backupPath).not.toBe(hookPath + '.backup'); + expect(second.backupPath).toBeTruthy(); + expect(fs.readFileSync(second.backupPath!, 'utf-8')).toContain('husky wrapper'); + }); + + it('should report no backup path when no pre-existing hook is present', () => { + const repo = path.join(tmpRoot, 'no-backup-repo'); + initRepo(repo); + + const service = new GitHooksService(path.join(repo, '.git')); + const result = service.install('pre-push'); + + expect(result.backupPath).toBeNull(); + expect(result.hookPath).toBe(path.join(repo, '.git', 'hooks', 'pre-push')); + }); + + it('should not create a backup when replacing an existing DeepL hook', () => { + const repo = path.join(tmpRoot, 'reinstall-repo'); + initRepo(repo); + + const service = new GitHooksService(path.join(repo, '.git')); + service.install('pre-commit'); + const second = service.install('pre-commit'); + + expect(second.backupPath).toBeNull(); + expect(fs.existsSync(second.hookPath + '.backup')).toBe(false); + }); + }); + + describe('findGitRoot', () => { + it('should resolve a relative start path without hanging', () => { + const repo = path.join(tmpRoot, 'relative-repo'); + initRepo(repo); + const originalCwd = process.cwd(); + process.chdir(repo); + try { + expect(GitHooksService.findGitRoot('.')).toBe(path.join(fs.realpathSync(repo), '.git')); + } finally { + process.chdir(originalCwd); + } + }); + }); +}); diff --git a/tests/integration/sync.integration.test.ts b/tests/integration/sync.integration.test.ts index 30bc49d..cc6f8b5 100644 --- a/tests/integration/sync.integration.test.ts +++ b/tests/integration/sync.integration.test.ts @@ -1347,7 +1347,7 @@ buckets: retryClient.destroy(); }); - it('should retry on 503 and complete successfully', async () => { + it('should not replay a translate POST on 503', async () => { const retryClient = new DeepLClient(TEST_API_KEY, { maxRetries: 1 }); const mockConfig = createMockConfigService({ get: jest.fn(() => ({ @@ -1373,28 +1373,19 @@ buckets: const scope503 = nock(DEEPL_FREE_API_URL) .post('/v2/translate') .reply(503, { message: 'Service unavailable' }, { 'Retry-After': '0' }); - const scope200 = nock(DEEPL_FREE_API_URL) - .post('/v2/translate') - .reply(200, { - translations: [ - { text: 'Hallo', detected_source_language: 'EN', billed_characters: 5 }, - ], - }); - const start = Date.now(); const config = await loadSyncConfig(tmpDir); const result = await retrySyncService.sync(config); - const elapsed = Date.now() - start; - expect(result.success).toBe(true); - expect(result.totalKeys).toBe(1); expect(scope503.isDone()).toBe(true); - expect(scope200.isDone()).toBe(true); - // Guard against runaway backoff — with Retry-After: 0 the whole retry should finish fast. - expect(elapsed).toBeLessThan(3000); + expect(nock.pendingMocks()).toHaveLength(0); + + const deResult = result.fileResults.find((r) => r.locale === 'de'); + expect(deResult?.written).toBe(false); + expect(deResult?.failed).toBeGreaterThan(0); const targetFile = path.join(tmpDir, 'locales', 'de.json'); - expect(fs.existsSync(targetFile)).toBe(true); + expect(fs.existsSync(targetFile)).toBe(false); retryClient.destroy(); }); diff --git a/tests/unit/cache-command.test.ts b/tests/unit/cache-command.test.ts index 6b77737..dcc9398 100644 --- a/tests/unit/cache-command.test.ts +++ b/tests/unit/cache-command.test.ts @@ -100,6 +100,14 @@ describe('CacheCommand', () => { expect(mockCacheService.enable).toHaveBeenCalledTimes(1); }); + it('should persist cache.enabled so the next process sees it', async () => { + mockCacheService.enable.mockReturnValue(undefined); + + await cacheCommand.enable(); + + expect(mockConfigService.set).toHaveBeenCalledWith('cache.enabled', true); + }); + it('should not throw error if cache is already enabled', async () => { mockCacheService.enable.mockReturnValue(undefined); @@ -124,7 +132,7 @@ describe('CacheCommand', () => { await cacheCommand.enable(); - expect(mockConfigService.set).not.toHaveBeenCalled(); + expect(mockConfigService.set).not.toHaveBeenCalledWith('cache.maxSize', expect.anything()); expect(mockCacheService.setMaxSize).not.toHaveBeenCalled(); expect(mockCacheService.enable).toHaveBeenCalledTimes(1); }); @@ -155,6 +163,14 @@ describe('CacheCommand', () => { await expect(cacheCommand.disable()).resolves.not.toThrow(); }); + + it('should persist cache.enabled so the next process sees it', async () => { + mockCacheService.disable.mockReturnValue(undefined); + + await cacheCommand.disable(); + + expect(mockConfigService.set).toHaveBeenCalledWith('cache.enabled', false); + }); }); describe('formatStats()', () => { diff --git a/tests/unit/cache-loader.test.ts b/tests/unit/cache-loader.test.ts index 9c15a57..0914dc3 100644 --- a/tests/unit/cache-loader.test.ts +++ b/tests/unit/cache-loader.test.ts @@ -14,9 +14,14 @@ jest.mock('../../src/utils/logger', () => ({ }, })); -import { createCacheServiceGetter } from '../../src/cli/cache-loader'; +import { createCacheServiceGetter, resolveCacheOptions } from '../../src/cli/cache-loader'; import { Logger } from '../../src/utils/logger'; import type { CacheService } from '../../src/storage/cache'; +import type { ConfigService } from '../../src/storage/config'; + +function fakeConfig(values: Record): ConfigService { + return { getValue: (key: string) => values[key] } as unknown as ConfigService; +} function makeError(message: string, code?: string): Error { const error = new Error(message); @@ -26,6 +31,31 @@ function makeError(message: string, code?: string): Error { return error; } +describe('resolveCacheOptions', () => { + it('carries cache.enabled through to the cache service', () => { + expect(resolveCacheOptions(fakeConfig({ 'cache.enabled': false }), '/tmp/cache.db').enabled).toBe(false); + expect(resolveCacheOptions(fakeConfig({ 'cache.enabled': true }), '/tmp/cache.db').enabled).toBe(true); + }); + + it('converts the configured TTL from seconds to milliseconds', () => { + const options = resolveCacheOptions(fakeConfig({ 'cache.ttl': 90 }), '/tmp/cache.db'); + expect(options.ttl).toBe(90_000); + }); + + it('passes the db path and max size through unchanged', () => { + const options = resolveCacheOptions(fakeConfig({ 'cache.maxSize': 4096 }), '/tmp/cache.db'); + expect(options.dbPath).toBe('/tmp/cache.db'); + expect(options.maxSize).toBe(4096); + }); + + it('leaves unset keys undefined so the service defaults apply', () => { + const options = resolveCacheOptions(fakeConfig({}), '/tmp/cache.db'); + expect(options.ttl).toBeUndefined(); + expect(options.maxSize).toBeUndefined(); + expect(options.enabled).toBeUndefined(); + }); +}); + describe('createCacheServiceGetter', () => { beforeEach(() => { jest.clearAllMocks(); diff --git a/tests/unit/cache-service.test.ts b/tests/unit/cache-service.test.ts index 5dfb3f0..cd63d4f 100644 --- a/tests/unit/cache-service.test.ts +++ b/tests/unit/cache-service.test.ts @@ -294,6 +294,43 @@ describe('CacheService', () => { }); }); + describe('enabled option', () => { + it('should default to enabled when the option is omitted', () => { + expect(cacheService.stats().enabled).toBe(true); + }); + + it('should start disabled when constructed with enabled: false', () => { + const disabled = new CacheService({ dbPath: path.join(testCacheDir, 'disabled.db'), enabled: false }); + try { + expect(disabled.stats().enabled).toBe(false); + } finally { + disabled.close(); + } + }); + + it('should not read or write entries when constructed disabled', () => { + const disabled = new CacheService({ dbPath: path.join(testCacheDir, 'disabled-io.db'), enabled: false }); + try { + disabled.set('test', { text: 'Hello' }); + expect(disabled.get('test')).toBeNull(); + expect(disabled.stats().entries).toBe(0); + } finally { + disabled.close(); + } + }); + + it('should allow enable() to override a disabled start', () => { + const disabled = new CacheService({ dbPath: path.join(testCacheDir, 'reenable.db'), enabled: false }); + try { + disabled.enable(); + disabled.set('test', { text: 'Hello' }); + expect(disabled.get('test')).toEqual({ text: 'Hello' }); + } finally { + disabled.close(); + } + }); + }); + describe('enable() / disable()', () => { it('should enable cache', () => { cacheService.disable(); diff --git a/tests/unit/cli/register-sync-init.test.ts b/tests/unit/cli/register-sync-init.test.ts index 2949112..c1f6635 100644 --- a/tests/unit/cli/register-sync-init.test.ts +++ b/tests/unit/cli/register-sync-init.test.ts @@ -1,10 +1,10 @@ /** * Unit tests for `deepl sync init`'s flag vocabulary. * - * Exercises the --source-locale / --target-locales rename and the - * --source-lang / --target-langs deprecation aliases. The new primary - * flags work silently; the old aliases continue to work but emit a - * stderr deprecation warning pointing at the replacement. + * --source-locale / --target-locales are the only accepted spellings. The + * --source-lang / --target-langs aliases were removed at the 2.0 cut per the + * documented deprecation policy, so they must now be rejected outright rather + * than silently accepted. */ import * as fs from 'fs'; @@ -82,65 +82,31 @@ describe('deepl sync init flag vocabulary', () => { expect(stderrText()).not.toMatch(/\[deprecated\]/); }); - it('--source-lang works but emits a stderr deprecation warning naming --source-locale', async () => { + it.each([ + ['--source-lang', 'en'], + ['--target-langs', 'de,fr'], + ])('rejects the removed %s alias as an unknown option', async (flag, value) => { const deps = makeDeps(handleError); - await runSyncInit( - [ - '--source-lang', 'en', - '--target-locales', 'de,fr', - '--file-format', 'json', - '--path', 'locales/en.json', - ], - deps, - ); - expect(handleError).not.toHaveBeenCalled(); - expect(fs.existsSync(path.join(tmpDir, '.deepl-sync.yaml'))).toBe(true); - const err = stderrText(); - expect(err).toMatch(/\[deprecated\] --source-lang is renamed to --source-locale/); - expect(err).toMatch(/next major release/); - }); - - it('--target-langs works but emits a stderr deprecation warning naming --target-locales', async () => { - const deps = makeDeps(handleError); - await runSyncInit( - [ - '--source-locale', 'en', - '--target-langs', 'de,fr', - '--file-format', 'json', - '--path', 'locales/en.json', - ], - deps, - ); - expect(handleError).not.toHaveBeenCalled(); - expect(fs.existsSync(path.join(tmpDir, '.deepl-sync.yaml'))).toBe(true); - const err = stderrText(); - expect(err).toMatch(/\[deprecated\] --target-langs is renamed to --target-locales/); - expect(err).toMatch(/next major release/); - }); - - it('both deprecated aliases together emit both warnings', async () => { - const deps = makeDeps(handleError); - await runSyncInit( - [ - '--source-lang', 'en', - '--target-langs', 'de,fr', - '--file-format', 'json', - '--path', 'locales/en.json', - ], - deps, - ); - expect(handleError).not.toHaveBeenCalled(); - const err = stderrText(); - expect(err).toMatch(/--source-lang is renamed to --source-locale/); - expect(err).toMatch(/--target-langs is renamed to --target-locales/); + await expect( + runSyncInit( + [ + '--source-locale', 'en', + '--target-locales', 'de,fr', + '--file-format', 'json', + '--path', 'locales/en.json', + flag, value, + ], + deps, + ), + ).rejects.toThrow(new RegExp(`unknown option '${flag}'`)); + expect(fs.existsSync(path.join(tmpDir, '.deepl-sync.yaml'))).toBe(false); }); - it('new flag wins when both new and old are supplied (no warning for the flag with the new form)', async () => { + it('no longer emits a deprecation warning for the canonical flags', async () => { const deps = makeDeps(handleError); await runSyncInit( [ '--source-locale', 'en', - '--source-lang', 'xx', '--target-locales', 'de', '--file-format', 'json', '--path', 'locales/en.json', @@ -148,10 +114,8 @@ describe('deepl sync init flag vocabulary', () => { deps, ); expect(handleError).not.toHaveBeenCalled(); - expect(fs.existsSync(path.join(tmpDir, '.deepl-sync.yaml'))).toBe(true); const yaml = fs.readFileSync(path.join(tmpDir, '.deepl-sync.yaml'), 'utf-8'); expect(yaml).toMatch(/source_locale:\s*en/); - expect(yaml).not.toMatch(/source_locale:\s*xx/); - expect(stderrText()).not.toMatch(/--source-lang is renamed/); + expect(stderrText()).not.toMatch(/\[deprecated\]/); }); }); diff --git a/tests/unit/cli/register-sync.commander-snapshot.test.ts b/tests/unit/cli/register-sync.commander-snapshot.test.ts index 92c9fd2..f2a2e66 100644 --- a/tests/unit/cli/register-sync.commander-snapshot.test.ts +++ b/tests/unit/cli/register-sync.commander-snapshot.test.ts @@ -512,17 +512,6 @@ const EXPECTED: CommandSnapshot = { variadic: false, mandatory: false, }, - { - flags: '--source-lang ', - description: '', - defaultValue: undefined, - negate: false, - choices: undefined, - required: true, - optional: false, - variadic: false, - mandatory: false, - }, { flags: '--source-locale ', description: 'Source locale', @@ -535,17 +524,6 @@ const EXPECTED: CommandSnapshot = { mandatory: false, }, SYNC_CONFIG_OPT, - { - flags: '--target-langs ', - description: '', - defaultValue: undefined, - negate: false, - choices: undefined, - required: true, - optional: false, - variadic: false, - mandatory: false, - }, { flags: '--target-locales ', description: 'Target locales (comma-separated)', diff --git a/tests/unit/cli/sync-options.test.ts b/tests/unit/cli/sync-options.test.ts new file mode 100644 index 0000000..96c7858 --- /dev/null +++ b/tests/unit/cli/sync-options.test.ts @@ -0,0 +1,56 @@ +import { Command } from 'commander'; +import { + parseLocaleFilter, + resolveLocale, + resolveSyncConfig, +} from '../../../src/cli/commands/sync/sync-options'; + +describe('sync-options', () => { + function childOf(parentOpts: Record): Command { + const parent = new Command('sync'); + const child = parent.command('status'); + Object.assign(parent.opts(), parentOpts); + return child; + } + + describe('resolveLocale', () => { + it('prefers the subcommand value', () => { + expect(resolveLocale({ locale: 'de' }, childOf({ locale: 'fr' }))).toBe('de'); + }); + + it('falls back to the parent value', () => { + expect(resolveLocale({}, childOf({ locale: 'fr' }))).toBe('fr'); + }); + + it('returns undefined when neither scope has a value', () => { + expect(resolveLocale({}, childOf({}))).toBeUndefined(); + }); + }); + + describe('resolveSyncConfig', () => { + it('prefers the subcommand value', () => { + expect(resolveSyncConfig({ syncConfig: 'a.yaml' }, childOf({ syncConfig: 'b.yaml' }))).toBe( + 'a.yaml', + ); + }); + + it('falls back to the parent value', () => { + expect(resolveSyncConfig({}, childOf({ syncConfig: 'b.yaml' }))).toBe('b.yaml'); + }); + + it('returns undefined when neither scope has a value', () => { + expect(resolveSyncConfig({}, childOf({}))).toBeUndefined(); + }); + }); + + describe('parseLocaleFilter', () => { + it('splits and trims a comma-separated list', () => { + expect(parseLocaleFilter('de, fr ,ja')).toEqual(['de', 'fr', 'ja']); + }); + + it('returns undefined for an absent or empty value', () => { + expect(parseLocaleFilter(undefined)).toBeUndefined(); + expect(parseLocaleFilter('')).toBeUndefined(); + }); + }); +}); diff --git a/tests/unit/deepl-client.test.ts b/tests/unit/deepl-client.test.ts index 4f55a6f..d613d9d 100644 --- a/tests/unit/deepl-client.test.ts +++ b/tests/unit/deepl-client.test.ts @@ -618,7 +618,6 @@ describe('DeepLClient', () => { it('should handle 503 service unavailable', async () => { nock(baseUrl) .post(path) - .times(4) .reply(503, { message: 'Service temporarily unavailable' }); await expect(call(client)).rejects.toThrow('Service temporarily unavailable'); @@ -627,7 +626,6 @@ describe('DeepLClient', () => { it('should handle network errors', async () => { nock(baseUrl) .post(path) - .times(4) .replyWithError('Network error'); await expect(call(client)).rejects.toThrow(); @@ -667,7 +665,7 @@ describe('DeepLClient', () => { it('should strip ANSI escape sequences from a 5xx server message before interpolating into the thrown error', async () => { const maliciousBody = { message: '\x1b[31mInternal failure\x1b[0m' }; - nock(baseUrl).post('/v2/translate').times(4).reply(500, maliciousBody); + nock(baseUrl).post('/v2/translate').reply(500, maliciousBody); await expect(client.translate('Hello', { targetLang: 'es' })).rejects.toThrow( expect.objectContaining({ message: expect.not.stringContaining('\x1b') }), @@ -856,31 +854,38 @@ describe('DeepLClient', () => { }); describe('retry logic', () => { - it('should retry on transient failures', async () => { + it('should retry an idempotent request on transient failures', async () => { nock(baseUrl) - .post('/v2/translate') + .get('/v2/usage') .reply(503) - .post('/v2/translate') + .get('/v2/usage') .reply(503) - .post('/v2/translate') - .reply(200, { - translations: [{ text: 'Hola' }], - }); + .get('/v2/usage') + .reply(200, { character_count: 1, character_limit: 2 }); - const result = await client.translate('Hello', { targetLang: 'es' }); + const usage = await client.getUsage(); - expect(result.text).toBe('Hola'); + expect(usage.characterCount).toBe(1); + }); + + it('should not replay a translate request on transient failures', async () => { + const scope = nock(baseUrl).post('/v2/translate').reply(503); + + await expect( + client.translate('Hello', { targetLang: 'es' }) + ).rejects.toThrow('Service temporarily unavailable'); + + expect(scope.isDone()).toBe(true); + expect(nock.pendingMocks()).toHaveLength(0); }); it('should give up after max retries', async () => { nock(baseUrl) - .post('/v2/translate') + .get('/v2/usage') .times(4) .reply(503); - await expect( - client.translate('Hello', { targetLang: 'es' }) - ).rejects.toThrow(); + await expect(client.getUsage()).rejects.toThrow(); }); it('should not retry on 4xx errors', async () => { @@ -993,13 +998,11 @@ describe('DeepLClient', () => { const customClient = new DeepLClient(apiKey, { maxRetries: 1 }); nock(baseUrl) - .post('/v2/translate') + .get('/v2/usage') .times(2) // Should only retry 1 time + initial = 2 requests total .reply(503); - await expect( - customClient.translate('Hello', { targetLang: 'es' }) - ).rejects.toThrow(); + await expect(customClient.getUsage()).rejects.toThrow(); expect(nock.isDone()).toBe(true); }); @@ -1008,13 +1011,11 @@ describe('DeepLClient', () => { const customClient = new DeepLClient(apiKey, { maxRetries: 0 }); nock(baseUrl) - .post('/v2/translate') + .get('/v2/usage') .times(1) // Should not retry, only 1 request .reply(503); - await expect( - customClient.translate('Hello', { targetLang: 'es' }) - ).rejects.toThrow(); + await expect(customClient.getUsage()).rejects.toThrow(); expect(nock.isDone()).toBe(true); }); @@ -1023,13 +1024,11 @@ describe('DeepLClient', () => { const customClient = new DeepLClient(apiKey, { maxRetries: 2 }); nock(baseUrl) - .post('/v2/translate') + .get('/v2/usage') .times(3) // Should retry 2 times + initial = 3 requests total .reply(503); - await expect( - customClient.translate('Hello', { targetLang: 'es' }) - ).rejects.toThrow(); + await expect(customClient.getUsage()).rejects.toThrow(); expect(nock.isDone()).toBe(true); }); diff --git a/tests/unit/document-client.test.ts b/tests/unit/document-client.test.ts index d39d17b..d3b252f 100644 --- a/tests/unit/document-client.test.ts +++ b/tests/unit/document-client.test.ts @@ -256,4 +256,85 @@ describe('DocumentClient', () => { ).rejects.toThrow(); }); }); + + describe('transfer policy', () => { + // The result endpoint is effectively single-use: a replay can consume the + // download of an already-billed translation and lose it permanently. + const unsentError = { + isAxiosError: true, + code: 'ECONNREFUSED', + message: 'connect ECONNREFUSED 127.0.0.1:443', + }; + + beforeEach(() => { + jest.spyOn(axios, 'isAxiosError').mockReturnValue(true); + }); + + it('should never retry the download endpoint, even on a refused connection', async () => { + mockAxiosInstance.request.mockRejectedValue(unsentError); + + await expect( + client.downloadDocument({ documentId: 'doc-1', documentKey: 'key-1' }) + ).rejects.toThrow(); + + expect(mockAxiosInstance.request).toHaveBeenCalledTimes(1); + }); + + it('should still retry an upload whose connection was refused', async () => { + mockAxiosInstance.request.mockRejectedValue(unsentError); + + await expect( + client.uploadDocument(Buffer.from('content'), { + targetLang: 'de', + filename: 'test.txt', + }) + ).rejects.toThrow(); + + expect(mockAxiosInstance.request).toHaveBeenCalledTimes(4); + }); + + it('should give document transfers a larger timeout than the default request timeout', async () => { + mockAxiosInstance.request.mockResolvedValue({ + data: Buffer.from('content'), + status: 200, + headers: {}, + }); + + await client.downloadDocument({ documentId: 'doc-1', documentKey: 'key-1' }); + + expect(mockAxiosInstance.request).toHaveBeenCalledWith( + expect.objectContaining({ timeout: 300000 }), + ); + }); + + it('should honour an explicitly configured timeout larger than the transfer default', async () => { + const slowClient = new DocumentClient('test-api-key', { timeout: 600000 }); + mockAxiosInstance.request.mockResolvedValue({ + data: Buffer.from('content'), + status: 200, + headers: {}, + }); + + await slowClient.downloadDocument({ documentId: 'doc-1', documentKey: 'key-1' }); + + expect(mockAxiosInstance.request).toHaveBeenCalledWith( + expect.objectContaining({ timeout: 600000 }), + ); + slowClient.destroy(); + }); + + it('should leave status polling on the default request timeout', async () => { + mockAxiosInstance.request.mockResolvedValue({ + data: { document_id: 'doc-1', status: 'done' }, + status: 200, + headers: {}, + }); + + await client.getDocumentStatus({ documentId: 'doc-1', documentKey: 'key-1' }); + + expect(mockAxiosInstance.request).toHaveBeenCalledWith( + expect.objectContaining({ timeout: 30000 }), + ); + }); + }); }); diff --git a/tests/unit/formats-android-cdata.test.ts b/tests/unit/formats-android-cdata.test.ts index c944a0a..1174b5c 100644 --- a/tests/unit/formats-android-cdata.test.ts +++ b/tests/unit/formats-android-cdata.test.ts @@ -1,16 +1,14 @@ /** - * Tests that a translation cannot break out of an Android CDATA section. + * A translation cannot break out of an Android CDATA section. * - * escapeForReconstruct wrapped the translation in `` with no - * escaping, so a value containing `]]>` closed the section early and the - * remainder was parsed as XML — allowing extra elements to be - * injected into a generated resource file. This is reachable without a - * malicious API, because on translation failure the source string is written - * through verbatim and the source file is used as the template when the - * target locale file does not exist yet. + * The regex round-trip is asymmetric inside CDATA — nothing in the body is + * entity-escaped — so a value carrying `]]>` would close the section and have + * its remainder parsed as XML. Such values are refused, matching the XLIFF + * parser's stance on CDATA in translatable content. */ import { AndroidXmlFormatParser } from '../../src/formats/android-xml'; +import { ValidationError } from '../../src/utils/errors'; const WITH_CDATA = ` @@ -19,6 +17,9 @@ const WITH_CDATA = ` `; +const BREAKOUT = + ']]>https://evil.example.com { it('should extract CDATA content without the wrapper', () => { const entries = new AndroidXmlFormatParser().extract(WITH_CDATA); @@ -27,38 +28,91 @@ describe('Android XML CDATA safety', () => { expect(byKey.get('body')).toBe('Bold text'); }); - it('should keep injected markup inside the CDATA section', () => { + it('should refuse a translation that closes the CDATA section', () => { const parser = new AndroidXmlFormatParser(); const entries = parser.extract(WITH_CDATA); - const attack = - ']]>https://evil.example.com ({ ...e, translation: e.key === 'body' ? attack : e.value })), - ); + const translated = entries.map((e) => ({ + ...e, + translation: e.key === 'body' ? BREAKOUT : e.value, + })); - // Strip every CDATA section: whatever remains is real markup, and the - // attacker's element must not be part of it. - const markupOnly = out.replace(//g, ''); - expect(markupOnly).not.toContain('name="injected"'); - // Known limitation: extract() is regex-based, so literal `` - // text inside a CDATA body counts as an element. A real Android build - // reads one element here. + expect(() => parser.reconstruct(WITH_CDATA, translated)).toThrow(ValidationError); + expect(() => parser.reconstruct(WITH_CDATA, translated)).toThrow(/CDATA/); }); - it('should preserve "]]>" as literal text through a round-trip', () => { + it('should refuse a bare "]]>" sequence rather than splitting the section', () => { const parser = new AndroidXmlFormatParser(); const entries = parser.extract(WITH_CDATA); - const literal = 'array]]> end'; - const out = parser.reconstruct( - WITH_CDATA, - entries.map((e) => ({ ...e, translation: e.key === 'body' ? literal : e.value })), - ); + expect(() => + parser.reconstruct( + WITH_CDATA, + entries.map((e) => ({ ...e, translation: e.key === 'body' ? 'array]]> end' : e.value })), + ), + ).toThrow(/"\]\]>"/); + }); + + it('should refuse a breakout inside a plural item', () => { + const xml = ` + + + 1 item]]> + %d items]]> + + +`; + const parser = new AndroidXmlFormatParser(); + + expect(() => + parser.reconstruct(xml, [ + { + key: 'items', + value: '%d items', + translation: '%d Elemente', + metadata: { + plurals: [ + { quantity: 'one', value: '1 Element' }, + { quantity: 'other', value: `]]>x { + const xml = ` + + + + + +`; + const parser = new AndroidXmlFormatParser(); + + expect(() => + parser.reconstruct(xml, [ + { key: 'labels.0', value: 'Less < More', translation: ']]>injected [e.key, e.value])); - expect(roundTripped.get('body')).toBe(literal); + it('should escape "]]>" as entities outside a CDATA section', () => { + const xml = ` + + Plain + +`; + const parser = new AndroidXmlFormatParser(); + + const out = parser.reconstruct(xml, [ + { key: 'plain', value: 'Plain', translation: 'array]]> end' }, + ]); + + expect(out).toContain('array]]> end'); + expect(new Map(parser.extract(out).map((e) => [e.key, e.value])).get('plain')).toBe( + 'array]]> end', + ); }); it('should keep CDATA output well-formed for ordinary translations', () => { @@ -75,4 +129,27 @@ describe('Android XML CDATA safety', () => { 'Fett Text', ); }); + + it('should never re-extract more entries than the template declares', () => { + const parser = new AndroidXmlFormatParser(); + const entries = parser.extract(WITH_CDATA); + + const out = parser.reconstruct( + WITH_CDATA, + entries.map((e) => ({ ...e, translation: e.key === 'body' ? 'x' : e.value })), + ); + + expect(parser.extract(out)).toHaveLength(entries.length); + }); + + it('should concatenate adjacent CDATA sections when extracting', () => { + const xml = ` + + + +`; + const entries = new AndroidXmlFormatParser().extract(xml); + + expect(entries).toEqual([{ key: 'split', value: 'firstsecond' }]); + }); }); diff --git a/tests/unit/formats/android-xml.test.ts b/tests/unit/formats/android-xml.test.ts index 538db4b..e865eb1 100644 --- a/tests/unit/formats/android-xml.test.ts +++ b/tests/unit/formats/android-xml.test.ts @@ -273,6 +273,22 @@ describe('android-xml parser', () => { expect(result).not.toContain('name="sizes"'); }); + it('should keep a self-closing string element untouched', () => { + const xml = ` + + + Hello +`; + const entries = parser.extract(xml); + expect(entries.map((e) => e.key)).toEqual(['greeting']); + + const result = parser.reconstruct(xml, [ + { key: 'greeting', value: 'Hello', translation: 'Hallo' }, + ]); + expect(result).toContain(''); + expect(result).toContain('Hallo'); + }); + it('should preserve translatable="false" strings even when not in entries', () => { const xml = ` @@ -287,4 +303,63 @@ describe('android-xml parser', () => { expect(result).toContain('name="greeting"'); }); }); + + describe('elements with no closing tag', () => { + const unclosed = ` + + Hello + Never closed +`; + + it('should still extract the well-formed entries before the dangling tag', () => { + expect(parser.extract(unclosed).map((e) => e.key)).toEqual(['greeting']); + }); + + it('should leave the dangling element untouched on reconstruct', () => { + const result = parser.reconstruct(unclosed, [ + { key: 'greeting', value: 'Hello', translation: 'Hallo' }, + ]); + expect(result).toContain('Hallo'); + expect(result).toContain('Never closed'); + }); + + it('should not let a CDATA body swallow the closing tag', () => { + const xml = ` + + y here]]> + Plain +`; + expect(parser.extract(xml)).toEqual([ + { key: 'snippet', value: 'use y here' }, + { key: 'plain', value: 'Plain' }, + ]); + }); + }); + + describe('adversarial input scaling', () => { + // A file of opening tags that never close: the parser must not rescan the + // remainder of the input once per opening tag. + function unclosedOpeners(bytes: number): string { + const head = '\n\n'; + const opener = + ' v\n \n \n'; + return head + opener.repeat(Math.ceil((bytes - head.length) / opener.length)); + } + + const content = unclosedOpeners(4 * 1024 * 1024); + + it('should extract a 4 MiB file of unclosed openers in linear time', () => { + const start = process.hrtime.bigint(); + parser.extract(content); + const ms = Number(process.hrtime.bigint() - start) / 1e6; + expect(ms).toBeLessThan(2000); + }); + + it('should reconstruct a 4 MiB file of unclosed openers in linear time', () => { + const start = process.hrtime.bigint(); + parser.reconstruct(content, [{ key: 'k', value: 'v', translation: 'w' }]); + const ms = Number(process.hrtime.bigint() - start) / 1e6; + expect(ms).toBeLessThan(2000); + }); + }); }); diff --git a/tests/unit/formats/xliff.test.ts b/tests/unit/formats/xliff.test.ts index ec76eb0..1f334b7 100644 --- a/tests/unit/formats/xliff.test.ts +++ b/tests/unit/formats/xliff.test.ts @@ -381,5 +381,87 @@ describe('xliff parser', () => { `; expect(() => parser.extract(xliff)).not.toThrow(); }); + + it('accepts a CDATA that sits between and ', () => { + const xliff = ` + +Hello]]>Hallo +`; + expect(() => parser.extract(xliff)).not.toThrow(); + expect(parser.extract(xliff).map((e) => e.key)).toEqual(['a']); + }); + }); + + describe('elements with no closing tag', () => { + const unclosed = ` + + + + + Hello + Hallo + + + Never closed + + +`; + + it('should still extract the well-formed units before the dangling tag', () => { + expect(parser.extract(unclosed).map((e) => e.key)).toEqual(['greeting']); + }); + + it('should leave the dangling unit untouched on reconstruct', () => { + const result = parser.reconstruct(unclosed, [ + { key: 'greeting', value: 'Hello', translation: 'Guten Tag' }, + ]); + expect(result).toContain('Guten Tag'); + expect(result).toContain(''); + }); + }); + + describe('adversarial input scaling', () => { + // A file of opening tags that never close: the parser must not rescan the + // remainder of the input once per opening tag. + function unclosedOpeners(bytes: number, version: '1.2' | '2.0'): string { + const head = `\n\n\n`; + const opener = + version === '2.0' + ? ' v\n' + : ' v\n'; + return head + opener.repeat(Math.ceil((bytes - head.length) / opener.length)); + } + + const v12 = unclosedOpeners(4 * 1024 * 1024, '1.2'); + const v20 = unclosedOpeners(4 * 1024 * 1024, '2.0'); + + it('should extract a 4 MiB v1.2 file of unclosed openers in linear time', () => { + const start = process.hrtime.bigint(); + parser.extract(v12); + const ms = Number(process.hrtime.bigint() - start) / 1e6; + expect(ms).toBeLessThan(2000); + }); + + it('should reconstruct a 4 MiB v1.2 file of unclosed openers in linear time', () => { + const start = process.hrtime.bigint(); + parser.reconstruct(v12, [{ key: 'k', value: 'v', translation: 'w' }]); + const ms = Number(process.hrtime.bigint() - start) / 1e6; + expect(ms).toBeLessThan(2000); + }); + + it('should extract a 4 MiB v2.0 file of unclosed openers in linear time', () => { + const start = process.hrtime.bigint(); + parser.extract(v20); + const ms = Number(process.hrtime.bigint() - start) / 1e6; + expect(ms).toBeLessThan(2000); + }); + + it('should scan a 4 MiB file for CDATA in translatable content in linear time', () => { + const withTrailingCdata = v12 + '\n'; + const start = process.hrtime.bigint(); + parser.extract(withTrailingCdata); + const ms = Number(process.hrtime.bigint() - start) / 1e6; + expect(ms).toBeLessThan(2000); + }); }); }); diff --git a/tests/unit/formats/xml-scan.test.ts b/tests/unit/formats/xml-scan.test.ts new file mode 100644 index 0000000..086df49 --- /dev/null +++ b/tests/unit/formats/xml-scan.test.ts @@ -0,0 +1,89 @@ +import { + findElement, + replaceElements, + scanElements, + type ElementPattern, +} from '../../../src/formats/xml-scan'; + +const TAG: ElementPattern = { + open: //y, + close: /<\/tag>/y, +}; + +describe('xml-scan', () => { + describe('scanElements', () => { + it('should return elements in document order with their parts split out', () => { + const elements = scanElements('aonebtwo', TAG); + + expect(elements.map((e) => [e.groups[0], e.inner, e.openTag, e.closeTag])).toEqual([ + ['x', 'one', '', ''], + [undefined, 'two', '', ''], + ]); + expect(elements[0]!.text).toBe('one'); + }); + + it('should stop at an opening tag that never closes', () => { + const elements = scanElements('onetwothree', TAG); + + expect(elements.map((e) => e.inner)).toEqual(['one']); + }); + + it('should skip a closing tag quoted inside a CDATA section', () => { + const elements = scanElements(']]>rest', TAG); + + expect(elements.map((e) => e.inner)).toEqual([']]>rest']); + }); + + it('should stop at an unterminated CDATA section', () => { + expect(scanElements('', TAG)).toEqual([]); + }); + + it('should ignore a self-closing tag', () => { + const elements = scanElements('one', TAG); + + expect(elements.map((e) => e.inner)).toEqual(['one']); + }); + + it('should not let an unterminated attribute value match across the next tag', () => { + const elements = scanElements(' { + const csv = 'API,API\n"bad\tterm",valor\nHello,Hola'; + + const entries = GlossaryService.tsvToEntries(csv); + + expect(entries).toEqual({ API: 'API', Hello: 'Hola' }); + }); + + it('should keep parsing a TSV file as TSV when a later value contains commas', () => { + const tsv = 'API\tAPI\nHello\tHola, mundo'; + + const entries = GlossaryService.tsvToEntries(tsv); + + expect(entries).toEqual({ + API: 'API', + Hello: 'Hola, mundo', + }); + }); + }); }); describe('addEntry()', () => { @@ -742,6 +773,29 @@ describe('GlossaryService', () => { glossaryService.addEntry('test-123', 'en', 'es', 'source', '') ).rejects.toThrow('Target text cannot be empty'); }); + + it('should reject source text containing a tab', async () => { + await expect( + glossaryService.addEntry('test-123', 'en', 'es', 'Col\tumn', 'Columna') + ).rejects.toThrow(ValidationError); + await expect( + glossaryService.addEntry('test-123', 'en', 'es', 'Col\tumn', 'Columna') + ).rejects.toThrow(/tab/i); + expect(mockDeepLClient.updateGlossaryEntries).not.toHaveBeenCalled(); + }); + + it('should reject target text containing a newline', async () => { + await expect( + glossaryService.addEntry('test-123', 'en', 'es', 'Line', 'Lí\nnea') + ).rejects.toThrow(/newline/i); + expect(mockDeepLClient.updateGlossaryEntries).not.toHaveBeenCalled(); + }); + + it('should reject source text containing a carriage return', async () => { + await expect( + glossaryService.addEntry('test-123', 'en', 'es', 'Line\r', 'Línea') + ).rejects.toThrow(/carriage return/i); + }); }); describe('updateEntry()', () => { @@ -779,6 +833,23 @@ describe('GlossaryService', () => { glossaryService.updateEntry('test-123', 'en', 'es', 'source', '') ).rejects.toThrow('Target text cannot be empty'); }); + + it('should reject a new target text containing a tab', async () => { + mockDeepLClient.getGlossaryEntries.mockResolvedValue('API\tAPI'); + + await expect( + glossaryService.updateEntry('test-123', 'en', 'es', 'API', 'Inter\tface') + ).rejects.toThrow(/tab/i); + expect(mockDeepLClient.updateGlossaryEntries).not.toHaveBeenCalled(); + }); + + it('should reject a new target text containing a newline', async () => { + mockDeepLClient.getGlossaryEntries.mockResolvedValue('API\tAPI'); + + await expect( + glossaryService.updateEntry('test-123', 'en', 'es', 'API', 'Inter\nface') + ).rejects.toThrow(/newline/i); + }); }); describe('removeEntry()', () => { diff --git a/tests/unit/hooks-command.test.ts b/tests/unit/hooks-command.test.ts index be18cd0..86875e1 100644 --- a/tests/unit/hooks-command.test.ts +++ b/tests/unit/hooks-command.test.ts @@ -73,17 +73,36 @@ describe('HooksCommand', () => { describe('install()', () => { it('should install pre-commit hook', () => { - mockGitHooksService.install.mockReturnValue(undefined); + mockGitHooksService.install.mockReturnValue({ + hookPath: '/path/to/.git/hooks/pre-commit', + backupPath: null, + }); const command = new HooksCommand('/path/to/.git'); const result = command.install('pre-commit'); expect(mockGitHooksService.install).toHaveBeenCalledWith('pre-commit'); expect(result).toContain('Installed pre-commit hook'); + expect(result).toContain('/path/to/.git/hooks/pre-commit'); + }); + + it('should report the backup path when an existing hook was backed up', () => { + mockGitHooksService.install.mockReturnValue({ + hookPath: '/path/to/.git/hooks/pre-commit', + backupPath: '/path/to/.git/hooks/pre-commit.backup', + }); + const command = new HooksCommand('/path/to/.git'); + + const result = command.install('pre-commit'); + + expect(result).toContain('backed up to: /path/to/.git/hooks/pre-commit.backup'); }); it('should install pre-push hook', () => { - mockGitHooksService.install.mockReturnValue(undefined); + mockGitHooksService.install.mockReturnValue({ + hookPath: '/path/to/.git/hooks/pre-push', + backupPath: null, + }); const command = new HooksCommand('/path/to/.git'); const result = command.install('pre-push'); diff --git a/tests/unit/http-retry-policy.test.ts b/tests/unit/http-retry-policy.test.ts new file mode 100644 index 0000000..f9daf05 --- /dev/null +++ b/tests/unit/http-retry-policy.test.ts @@ -0,0 +1,345 @@ +/** + * Retry-policy and transport-error classification tests for HttpClient. + * + * Every assertion here counts SERVER-SIDE requests: a replay of a + * non-idempotent request is only observable from the server's side, so + * `.rejects.toThrow()` alone cannot catch it. + */ + +import nock from 'nock'; +import { HttpClient } from '../../src/api/http-client'; +import { + AuthError, + NetworkError, + RateLimitError, + ValidationError, +} from '../../src/utils/errors'; +import { ExitCode, exitCodeForError } from '../../src/utils/exit-codes'; + +class TestHttpClient extends HttpClient { + get(path: string): Promise { + return this.makeRequest('GET', path); + } + + post(path: string, data?: Record): Promise { + return this.makeRequest('POST', path, data); + } + + put(path: string, data?: Record): Promise { + return this.makeRequest('PUT', path, data); + } + + delete(path: string): Promise { + return this.makeRequest('DELETE', path); + } + + classify(error: unknown): Error { + return this.handleError(error); + } +} + +const BASE_URL = 'https://api-free.deepl.com'; + +describe('HttpClient retry policy', () => { + const apiKey = 'test-api-key'; + let clients: TestHttpClient[]; + let sleepSpy: jest.SpyInstance; + + function makeClient(options: Record = {}): TestHttpClient { + const client = new TestHttpClient(apiKey, { + timeout: 200, + maxRetries: 3, + totalTimeout: 10_000, + ...options, + }); + sleepSpy = jest + .spyOn(client as unknown as { sleep: (ms: number) => Promise }, 'sleep') + .mockResolvedValue(); + clients.push(client); + return client; + } + + /** nock only produces a coded transport error from a real Error instance. */ + function transportError(code: string, message: string): Error { + return Object.assign(new Error(message), { code }); + } + + /** Counts requests the server actually received for `path`. */ + function countRequests(scope: nock.Scope): () => number { + let count = 0; + scope.on('request', () => { + count++; + }); + return () => count; + } + + beforeEach(() => { + clients = []; + }); + + // Interceptors left unconsumed are the point of several tests here (a + // request that must NOT be replayed leaves its spare interceptors + // pending), so clean up before the global pending-mock assertion runs. + // The replyWithError-based blocks sit at the end of the file: nock v14 + // emits async socket errors from them that leak into later tests. + afterEach(() => { + for (const client of clients) { + client.destroy(); + } + nock.abortPendingRequests(); + nock.cleanAll(); + }); + + describe('client-side timeout', () => { + it('sends a POST exactly once when the client aborts on timeout', async () => { + const scope = nock(BASE_URL) + .post('/v2/translate') + .times(4) + .delayConnection(2000) + .reply(200, { translations: [{ text: 'Hola' }] }); + const requests = countRequests(scope); + + await expect(makeClient().post('/v2/translate', { text: 'Hello' })).rejects.toThrow( + NetworkError + ); + + expect(requests()).toBe(1); + }); + + it('classifies a client-side timeout as a network error (exit 5), not invalid input', async () => { + nock(BASE_URL) + .post('/v2/translate') + .times(4) + .delayConnection(2000) + .reply(200, { translations: [{ text: 'Hola' }] }); + + const error = await makeClient() + .post('/v2/translate', { text: 'Hello' }) + .catch((e: unknown) => e); + + expect(error).toBeInstanceOf(NetworkError); + expect(exitCodeForError(error)).toBe(ExitCode.NetworkError); + }); + + it('retries an idempotent GET that times out', async () => { + const scope = nock(BASE_URL) + .get('/v2/usage') + .times(4) + .delayConnection(2000) + .reply(200, {}); + const requests = countRequests(scope); + + await expect(makeClient().get('/v2/usage')).rejects.toThrow(NetworkError); + + expect(requests()).toBe(4); + }); + + it('retries an idempotent PUT that times out', async () => { + const scope = nock(BASE_URL) + .put('/v3/glossaries/g-1') + .times(4) + .delayConnection(2000) + .reply(200, {}); + const requests = countRequests(scope); + + await expect(makeClient().put('/v3/glossaries/g-1', { name: 'x' })).rejects.toThrow( + NetworkError + ); + + expect(requests()).toBe(4); + }); + }); + + describe('server errors', () => { + it('retries a 500 on an idempotent GET', async () => { + const scope = nock(BASE_URL).get('/v2/usage').times(4).reply(500, { message: 'boom' }); + const requests = countRequests(scope); + + await expect(makeClient().get('/v2/usage')).rejects.toThrow(NetworkError); + + expect(requests()).toBe(4); + }); + + it('does not retry a 500 on a POST', async () => { + const scope = nock(BASE_URL) + .post('/v2/translate') + .times(4) + .reply(500, { message: 'boom' }); + const requests = countRequests(scope); + + await expect(makeClient().post('/v2/translate', { text: 'Hello' })).rejects.toThrow( + /Server error \(500\)/ + ); + + expect(requests()).toBe(1); + }); + + it('does not retry a 503 on a POST', async () => { + const scope = nock(BASE_URL) + .post('/v2/translate') + .times(4) + .reply(503, { message: 'Service Unavailable' }); + const requests = countRequests(scope); + + await expect(makeClient().post('/v2/translate', { text: 'Hello' })).rejects.toThrow( + /Service temporarily unavailable/ + ); + + expect(requests()).toBe(1); + }); + + // A 429 proves the server received and rejected the request without + // processing it, so replaying it cannot duplicate billable work — the + // idempotency restriction does not apply. + it('retries a POST on 429 and honours Retry-After', async () => { + const scope = nock(BASE_URL) + .post('/v2/translate') + .reply(429, { message: 'Too many requests' }, { 'Retry-After': '2' }); + const requests = countRequests(scope); + const retryScope = nock(BASE_URL) + .post('/v2/translate') + .reply(200, { translations: [{ text: 'Hola' }] }); + const retryRequests = countRequests(retryScope); + + const client = makeClient(); + const result = await client.post<{ translations: { text: string }[] }>( + '/v2/translate', + { text: 'Hello' } + ); + + expect(result.translations[0]!.text).toBe('Hola'); + expect(requests()).toBe(1); + expect(retryRequests()).toBe(1); + expect(sleepSpy).toHaveBeenCalledTimes(1); + expect(sleepSpy).toHaveBeenCalledWith(2000); + }); + + it('still retries a POST on 429 — the server rejected it without processing', async () => { + const scope = nock(BASE_URL) + .post('/v2/translate') + .times(4) + .reply(429, { message: 'Too many requests' }); + const requests = countRequests(scope); + + await expect(makeClient().post('/v2/translate', { text: 'Hello' })).rejects.toThrow( + RateLimitError + ); + + expect(requests()).toBe(4); + }); + }); + + describe('401 handling', () => { + it('maps 401 to AuthError without retrying', async () => { + const scope = nock(BASE_URL).get('/v2/usage').times(4).reply(401, { message: 'Unauthorized' }); + const requests = countRequests(scope); + + const error = await makeClient() + .get('/v2/usage') + .catch((e: unknown) => e); + + expect(error).toBeInstanceOf(AuthError); + expect(exitCodeForError(error)).toBe(ExitCode.AuthError); + expect(requests()).toBe(1); + }); + }); + + describe('total deadline', () => { + it('stops retrying an idempotent request once the overall budget is spent', async () => { + const scope = nock(BASE_URL) + .get('/v2/usage') + .times(6) + .delayConnection(2000) + .reply(200, {}); + const requests = countRequests(scope); + + const client = makeClient({ timeout: 200, maxRetries: 5, totalTimeout: 500 }); + const start = Date.now(); + await expect(client.get('/v2/usage')).rejects.toThrow(NetworkError); + const elapsed = Date.now() - start; + + // Retries happen, but the budget cuts them short of maxRetries + 1 = 6. + expect(requests()).toBeGreaterThanOrEqual(2); + expect(requests()).toBeLessThanOrEqual(3); + expect(elapsed).toBeLessThan(1500); + }); + + it('derives a default deadline from the request timeout', async () => { + const scope = nock(BASE_URL) + .get('/v2/usage') + .times(6) + .delayConnection(2000) + .reply(200, {}); + const requests = countRequests(scope); + + const client = makeClient({ timeout: 200, maxRetries: 5, totalTimeout: undefined }); + await expect(client.get('/v2/usage')).rejects.toThrow(NetworkError); + + // Default budget is twice the timeout, so far short of six attempts. + expect(requests()).toBeGreaterThanOrEqual(2); + expect(requests()).toBeLessThanOrEqual(3); + }); + }); + + describe('transport failures', () => { + it('retries a POST when the connection was refused', async () => { + const scope = nock(BASE_URL) + .post('/v2/translate') + .times(4) + .replyWithError(transportError('ECONNREFUSED', 'connect ECONNREFUSED 127.0.0.1:443')); + const requests = countRequests(scope); + + await expect(makeClient().post('/v2/translate', { text: 'Hello' })).rejects.toThrow( + NetworkError + ); + + expect(requests()).toBe(4); + }); + + it('retries a POST when DNS resolution failed', async () => { + const scope = nock(BASE_URL) + .post('/v2/translate') + .times(4) + .replyWithError(transportError('ENOTFOUND', 'getaddrinfo ENOTFOUND api-free.deepl.com')); + const requests = countRequests(scope); + + await expect(makeClient().post('/v2/translate', { text: 'Hello' })).rejects.toThrow( + NetworkError + ); + + expect(requests()).toBe(4); + }); + + it('does not retry a POST reset mid-flight — the request may have been accepted', async () => { + const scope = nock(BASE_URL) + .post('/v2/translate') + .times(4) + .replyWithError(transportError('ECONNRESET', 'socket hang up')); + const requests = countRequests(scope); + + await expect(makeClient().post('/v2/translate', { text: 'Hello' })).rejects.toThrow( + NetworkError + ); + + expect(requests()).toBe(1); + }); + }); + + describe('error messages', () => { + it('does not double the "Network error" prefix', async () => { + nock(BASE_URL).post('/v2/translate').replyWithError('Network error'); + + const error = await makeClient() + .post('/v2/translate', { text: 'Hello' }) + .catch((e: unknown) => e); + + expect((error as Error).message).not.toMatch(/Network error: Network error/); + }); + + it('leaves an already-classified error untouched', () => { + const classified = new ValidationError('API error: Tone is not supported'); + + expect(makeClient().classify(classified)).toBe(classified); + }); + }); +}); diff --git a/tests/unit/sync/sync-config.test.ts b/tests/unit/sync/sync-config.test.ts index a726851..2cad2cc 100644 --- a/tests/unit/sync/sync-config.test.ts +++ b/tests/unit/sync/sync-config.test.ts @@ -1198,6 +1198,28 @@ translation: buckets: { json: { include: ['locales/en.json'] } }, }); + it('should accept a localeFilter whose entries are all in target_locales', () => { + const config = baseConfig(); + config.target_locales = ['de', 'fr']; + expect(() => applyCliOverrides(config, { localeFilter: ['de', 'fr'] })).not.toThrow(); + }); + + it('should throw ConfigError naming the offending and configured locales', () => { + const config = baseConfig(); + config.target_locales = ['de', 'fr']; + expect(() => applyCliOverrides(config, { localeFilter: ['es'] })).toThrow(ConfigError); + expect(() => applyCliOverrides(config, { localeFilter: ['es'] })).toThrow(/es/); + expect(() => applyCliOverrides(config, { localeFilter: ['es'] })).toThrow(/de, fr/); + }); + + it('should report every unconfigured locale in a mixed filter', () => { + const config = baseConfig(); + config.target_locales = ['de', 'fr']; + expect(() => applyCliOverrides(config, { localeFilter: ['de', 'es', 'zz'] })).toThrow( + /es, zz/, + ); + }); + it('should merge formality into existing translation block', () => { const config = baseConfig(); config.translation = { glossary: 'g' }; diff --git a/tests/unit/sync/sync-glossary-report.test.ts b/tests/unit/sync/sync-glossary-report.test.ts index 81e10ac..a6b0765 100644 --- a/tests/unit/sync/sync-glossary-report.test.ts +++ b/tests/unit/sync/sync-glossary-report.test.ts @@ -1,4 +1,5 @@ import { generateGlossaryReport } from '../../../src/sync/sync-glossary-report'; +import type { TargetTranslationIndex } from '../../../src/sync/sync-glossary-report'; import type { SyncLockFile } from '../../../src/sync/types'; function makeLockFile(entries: SyncLockFile['entries'] = {}): SyncLockFile { @@ -12,166 +13,130 @@ function makeLockFile(entries: SyncLockFile['entries'] = {}): SyncLockFile { }; } +function translated(hash: string) { + return { hash, translated_at: '2026-01-01T00:00:00Z', status: 'translated' as const }; +} + +function index( + spec: Record>>, +): TargetTranslationIndex { + const result: TargetTranslationIndex = new Map(); + for (const [file, locales] of Object.entries(spec)) { + const localeMap = new Map>(); + for (const [locale, keys] of Object.entries(locales)) { + localeMap.set(locale, new Map(Object.entries(keys))); + } + result.set(file, localeMap); + } + return result; +} + describe('generateGlossaryReport()', () => { - it('should report no inconsistencies for consistent translations', () => { + it('should report no inconsistencies when the same source text is translated identically', () => { const lock = makeLockFile({ 'en/common.json': { - greeting: { - source_hash: 'abc123', - source_text: 'Hello', - translations: { - de: { hash: 'hash_de_1', translated_at: '2026-01-01T00:00:00Z', status: 'translated' }, - fr: { hash: 'hash_fr_1', translated_at: '2026-01-01T00:00:00Z', status: 'translated' }, - }, - }, + greeting: { source_hash: 'abc123', source_text: 'Hello', translations: { de: translated('h1') } }, }, 'en/other.json': { - greeting2: { - source_hash: 'abc123', - source_text: 'Hello', - translations: { - de: { hash: 'hash_de_1', translated_at: '2026-01-01T00:00:00Z', status: 'translated' }, - fr: { hash: 'hash_fr_1', translated_at: '2026-01-01T00:00:00Z', status: 'translated' }, - }, - }, + greeting2: { source_hash: 'abc123', source_text: 'Hello', translations: { de: translated('h1') } }, }, }); - const report = generateGlossaryReport(lock); + const report = generateGlossaryReport( + lock, + index({ + 'en/common.json': { de: { greeting: 'Hallo' } }, + 'en/other.json': { de: { greeting2: 'Hallo' } }, + }), + ); expect(report.totalTerms).toBe(1); expect(report.inconsistencies).toHaveLength(0); + expect(report.missingTargets).toHaveLength(0); }); - it('should detect inconsistency when same source_text has different hashes for same locale', () => { + it('should detect an inconsistency when the same source text has different translations', () => { const lock = makeLockFile({ 'en/common.json': { - greeting: { - source_hash: 'abc123', - source_text: 'Hello', - translations: { - de: { hash: 'hash_de_v1', translated_at: '2026-01-01T00:00:00Z', status: 'translated' }, - }, - }, + greeting: { source_hash: 'abc123', source_text: 'Hello', translations: { de: translated('h1') } }, }, 'en/other.json': { - welcome: { - source_hash: 'abc123', - source_text: 'Hello', - translations: { - de: { hash: 'hash_de_v2', translated_at: '2026-01-02T00:00:00Z', status: 'translated' }, - }, - }, + welcome: { source_hash: 'abc123', source_text: 'Hello', translations: { de: translated('h1') } }, }, }); - const report = generateGlossaryReport(lock); + const report = generateGlossaryReport( + lock, + index({ + 'en/common.json': { de: { greeting: 'Hallo' } }, + 'en/other.json': { de: { welcome: 'Guten Tag' } }, + }), + ); expect(report.inconsistencies).toHaveLength(1); expect(report.inconsistencies[0]).toEqual({ sourceText: 'Hello', locale: 'de', - translations: expect.arrayContaining(['hash_de_v1', 'hash_de_v2']), + translations: expect.arrayContaining(['Hallo', 'Guten Tag']), files: expect.arrayContaining(['en/common.json', 'en/other.json']), }); }); it('should return zero terms and no inconsistencies for empty lock file', () => { - const lock = makeLockFile({}); - - const report = generateGlossaryReport(lock); + const report = generateGlossaryReport(makeLockFile({})); expect(report.totalTerms).toBe(0); expect(report.inconsistencies).toHaveLength(0); + expect(report.missingTargets).toHaveLength(0); }); it('should populate files array correctly for multiple files with same source text', () => { const lock = makeLockFile({ 'file-a.json': { - key1: { - source_hash: 'h1', - source_text: 'Save', - translations: { - de: { hash: 'same_hash', translated_at: '2026-01-01T00:00:00Z', status: 'translated' }, - }, - }, + key1: { source_hash: 'h1', source_text: 'Save', translations: { de: translated('h') } }, }, 'file-b.json': { - key2: { - source_hash: 'h1', - source_text: 'Save', - translations: { - de: { hash: 'same_hash', translated_at: '2026-01-01T00:00:00Z', status: 'translated' }, - }, - }, + key2: { source_hash: 'h1', source_text: 'Save', translations: { de: translated('h') } }, }, 'file-c.json': { - key3: { - source_hash: 'h1', - source_text: 'Save', - translations: { - de: { hash: 'same_hash', translated_at: '2026-01-01T00:00:00Z', status: 'translated' }, - }, - }, + key3: { source_hash: 'h1', source_text: 'Save', translations: { de: translated('h') } }, }, }); - const report = generateGlossaryReport(lock); + const report = generateGlossaryReport( + lock, + index({ + 'file-a.json': { de: { key1: 'Speichern' } }, + 'file-b.json': { de: { key2: 'Speichern' } }, + 'file-c.json': { de: { key3: 'Sichern' } }, + }), + ); expect(report.totalTerms).toBe(1); - expect(report.inconsistencies).toHaveLength(0); - - // Even with no inconsistency, verify the internal grouping worked by - // introducing a divergent hash and checking the files list - const lockWithDivergence = makeLockFile({ - ...lock.entries, - 'file-c.json': { - key3: { - source_hash: 'h1', - source_text: 'Save', - translations: { - de: { hash: 'different_hash', translated_at: '2026-01-01T00:00:00Z', status: 'translated' }, - }, - }, - }, - }); - - const report2 = generateGlossaryReport(lockWithDivergence); - expect(report2.inconsistencies).toHaveLength(1); - expect(report2.inconsistencies[0]!.files).toEqual( + expect(report.inconsistencies).toHaveLength(1); + expect(report.inconsistencies[0]!.files).toEqual( expect.arrayContaining(['file-a.json', 'file-b.json', 'file-c.json']), ); - expect(report2.inconsistencies[0]!.files).toHaveLength(3); + expect(report.inconsistencies[0]!.files).toHaveLength(3); }); it('surfaces actual translated text (not hashes) when a target-translation index is provided', () => { const lock = makeLockFile({ 'en/common.json': { - greeting: { - source_hash: 'h', - source_text: 'Dashboard', - translations: { - de: { hash: 'de-v1', translated_at: '2026-01-01T00:00:00Z', status: 'translated' }, - }, - }, + greeting: { source_hash: 'h', source_text: 'Dashboard', translations: { de: translated('de-v1') } }, }, 'en/admin.json': { - header: { - source_hash: 'h', - source_text: 'Dashboard', - translations: { - de: { hash: 'de-v2', translated_at: '2026-01-01T00:00:00Z', status: 'translated' }, - }, - }, + header: { source_hash: 'h', source_text: 'Dashboard', translations: { de: translated('de-v2') } }, }, }); - const targets = new Map([ - ['en/common.json', new Map([['de', new Map([['greeting', 'Armaturenbrett']])]])], - ['en/admin.json', new Map([['de', new Map([['header', 'Dashboard']])]])], - ]); - - const report = generateGlossaryReport(lock, targets); + const report = generateGlossaryReport( + lock, + index({ + 'en/common.json': { de: { greeting: 'Armaturenbrett' } }, + 'en/admin.json': { de: { header: 'Dashboard' } }, + }), + ); expect(report.inconsistencies).toHaveLength(1); const inc = report.inconsistencies[0]!; @@ -182,69 +147,133 @@ describe('generateGlossaryReport()', () => { expect(inc.translations).not.toContain('de-v2'); }); - it('falls back to the hash when the target-translation index is missing an entry', () => { - const lock = makeLockFile({ - 'en/common.json': { - greeting: { - source_hash: 'h', - source_text: 'Hi', - translations: { - de: { hash: 'de-hashA', translated_at: '2026-01-01T00:00:00Z', status: 'translated' }, - }, + describe('unreadable targets', () => { + it('reports a missing locale file instead of inventing an inconsistency', () => { + const lock = makeLockFile({ + 'en/common.json': { + greeting: { source_hash: 'h', source_text: 'Hi', translations: { de: translated('de-hashA') } }, }, - }, - 'en/other.json': { - greeting2: { - source_hash: 'h', - source_text: 'Hi', - translations: { - de: { hash: 'de-hashB', translated_at: '2026-01-01T00:00:00Z', status: 'translated' }, - }, + 'en/other.json': { + greeting2: { source_hash: 'h', source_text: 'Hi', translations: { de: translated('de-hashB') } }, }, - }, + }); + + // en/other.json has no de file yet. + const report = generateGlossaryReport( + lock, + index({ 'en/common.json': { de: { greeting: 'Hallo' } } }), + ); + + expect(report.inconsistencies).toHaveLength(0); + expect(report.missingTargets).toEqual([{ filePath: 'en/other.json', locale: 'de' }]); }); - // Only en/common.json has a target; en/other.json is missing from the index. - const partialTargets = new Map([ - ['en/common.json', new Map([['de', new Map([['greeting', 'Hallo']])]])], - ]); + it('never emits a lock hash as a translation string', () => { + const lock = makeLockFile({ + 'en/common.json': { + greeting: { source_hash: 'h', source_text: 'Hi', translations: { de: translated('de-hashA') } }, + }, + 'en/other.json': { + greeting2: { source_hash: 'h', source_text: 'Hi', translations: { de: translated('de-hashB') } }, + }, + }); - const report = generateGlossaryReport(lock, partialTargets); + const report = generateGlossaryReport( + lock, + index({ 'en/common.json': { de: { greeting: 'Hallo' } } }), + ); - expect(report.inconsistencies).toHaveLength(1); - expect(report.inconsistencies[0]!.translations).toEqual(expect.arrayContaining(['Hallo', 'de-hashB'])); + const allTranslations = report.inconsistencies.flatMap(inc => inc.translations); + expect(allTranslations).not.toContain('de-hashA'); + expect(allTranslations).not.toContain('de-hashB'); + }); + + it('still detects divergence among the readable targets of a group', () => { + const lock = makeLockFile({ + 'a.json': { k1: { source_hash: 'h', source_text: 'Hi', translations: { de: translated('x') } } }, + 'b.json': { k2: { source_hash: 'h', source_text: 'Hi', translations: { de: translated('x') } } }, + 'c.json': { k3: { source_hash: 'h', source_text: 'Hi', translations: { de: translated('x') } } }, + }); + + const report = generateGlossaryReport( + lock, + index({ + 'a.json': { de: { k1: 'Hallo' } }, + 'b.json': { de: { k2: 'Servus' } }, + }), + ); + + expect(report.inconsistencies).toHaveLength(1); + expect(report.inconsistencies[0]!.translations).toEqual( + expect.arrayContaining(['Hallo', 'Servus']), + ); + expect(report.inconsistencies[0]!.files).toEqual(['a.json', 'b.json']); + expect(report.missingTargets).toEqual([{ filePath: 'c.json', locale: 'de' }]); + }); + + it('reports every unreadable target once per file and locale', () => { + const lock = makeLockFile({ + 'a.json': { + k1: { source_hash: 'h', source_text: 'Hi', translations: { de: translated('x'), fr: translated('x') } }, + k2: { source_hash: 'h2', source_text: 'Bye', translations: { de: translated('y') } }, + }, + }); + + const report = generateGlossaryReport(lock, index({})); + + expect(report.missingTargets).toEqual([ + { filePath: 'a.json', locale: 'de' }, + { filePath: 'a.json', locale: 'fr' }, + ]); + }); + + it('treats an omitted index as every target being unreadable', () => { + const lock = makeLockFile({ + 'a.json': { k1: { source_hash: 'h', source_text: 'Hi', translations: { de: translated('x') } } }, + 'b.json': { k2: { source_hash: 'h', source_text: 'Hi', translations: { de: translated('y') } } }, + }); + + const report = generateGlossaryReport(lock); + + expect(report.inconsistencies).toHaveLength(0); + expect(report.missingTargets).toHaveLength(2); + }); }); it('should count different source texts separately in totalTerms', () => { const lock = makeLockFile({ 'en/common.json': { - greeting: { - source_hash: 'h1', - source_text: 'Hello', - translations: { - de: { hash: 'hash1', translated_at: '2026-01-01T00:00:00Z', status: 'translated' }, - }, - }, - farewell: { - source_hash: 'h2', - source_text: 'Goodbye', - translations: { - de: { hash: 'hash2', translated_at: '2026-01-01T00:00:00Z', status: 'translated' }, - }, - }, - save: { - source_hash: 'h3', - source_text: 'Save', - translations: { - de: { hash: 'hash3', translated_at: '2026-01-01T00:00:00Z', status: 'translated' }, - }, - }, + greeting: { source_hash: 'h1', source_text: 'Hello', translations: { de: translated('hash1') } }, + farewell: { source_hash: 'h2', source_text: 'Goodbye', translations: { de: translated('hash2') } }, + save: { source_hash: 'h3', source_text: 'Save', translations: { de: translated('hash3') } }, }, }); - const report = generateGlossaryReport(lock); + const report = generateGlossaryReport( + lock, + index({ + 'en/common.json': { de: { greeting: 'Hallo', farewell: 'Auf Wiedersehen', save: 'Speichern' } }, + }), + ); expect(report.totalTerms).toBe(3); expect(report.inconsistencies).toHaveLength(0); }); + + it('should ignore translations that have not reached the translated status', () => { + const lock = makeLockFile({ + 'a.json': { + k1: { + source_hash: 'h', + source_text: 'Hi', + translations: { de: { hash: 'x', translated_at: '2026-01-01T00:00:00Z', status: 'pending' } }, + }, + }, + }); + + const report = generateGlossaryReport(lock, index({})); + + expect(report.inconsistencies).toHaveLength(0); + expect(report.missingTargets).toHaveLength(0); + }); }); diff --git a/tests/unit/sync/sync-glossary.test.ts b/tests/unit/sync/sync-glossary.test.ts index faf38e4..b0bf1be 100644 --- a/tests/unit/sync/sync-glossary.test.ts +++ b/tests/unit/sync/sync-glossary.test.ts @@ -1,7 +1,28 @@ import { SyncGlossaryManager } from '../../../src/sync/sync-glossary'; import type { SyncGlossaryManagerOptions } from '../../../src/sync/sync-glossary'; +import { GlossaryService } from '../../../src/services/glossary'; +import type { GlossaryInfo } from '../../../src/types/index'; +import { Logger } from '../../../src/utils/logger'; import { createMockGlossaryService } from '../../helpers/mock-factories'; +/** Mirror what a DeepL glossary returns after entries survive a TSV round trip. */ +function roundTrip(entries: Record): Record { + return GlossaryService.tsvToEntries(GlossaryService.entriesToTSV(entries)); +} + +function repeated(source: string, target: string, prefix: string): { + sourceEntries: Map; + localeEntries: Map; +} { + const sourceEntries = new Map(); + const localeEntries = new Map(); + for (let i = 0; i < 3; i++) { + sourceEntries.set(`${prefix}${i}`, source); + localeEntries.set(`${prefix}${i}`, target); + } + return { sourceEntries, localeEntries }; +} + describe('SyncGlossaryManager', () => { function createManager( overrides: Partial = {}, @@ -121,6 +142,95 @@ describe('SyncGlossaryManager', () => { }); }); + describe('extractTerms term validation', () => { + let warnSpy: jest.SpyInstance; + + beforeEach(() => { + warnSpy = jest.spyOn(Logger, 'warn').mockImplementation(() => undefined); + }); + + afterEach(() => { + warnSpy.mockRestore(); + }); + + it('should skip a term whose target translation is empty', () => { + const { manager } = createManager(); + const { sourceEntries, localeEntries } = repeated('OK', '', 'k'); + + const result = manager.extractTerms(sourceEntries, new Map([['de', localeEntries]])); + + expect(result.has('de')).toBe(false); + expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining('k0')); + }); + + it('should skip a term whose target translation is whitespace-only', () => { + const { manager } = createManager(); + const { sourceEntries, localeEntries } = repeated('OK', ' ', 'k'); + + const result = manager.extractTerms(sourceEntries, new Map([['de', localeEntries]])); + + expect(result.has('de')).toBe(false); + }); + + it('should skip a term whose source is empty', () => { + const { manager } = createManager(); + const { sourceEntries, localeEntries } = repeated('', 'Leer', 'k'); + + const result = manager.extractTerms(sourceEntries, new Map([['de', localeEntries]])); + + expect(result.has('de')).toBe(false); + }); + + it('should skip a term whose source contains a tab', () => { + const { manager } = createManager(); + const { sourceEntries, localeEntries } = repeated('Col\tumn', 'Spalte', 'k'); + + const result = manager.extractTerms(sourceEntries, new Map([['de', localeEntries]])); + + expect(result.has('de')).toBe(false); + expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining('k0')); + }); + + it('should skip a multi-line source string instead of uploading a wrong mapping', () => { + const { manager } = createManager(); + const { sourceEntries, localeEntries } = repeated( + 'Are you sure?\nThis cannot be undone.', + 'Sicher?', + 'k', + ); + + const result = manager.extractTerms(sourceEntries, new Map([['de', localeEntries]])); + + expect(result.has('de')).toBe(false); + }); + + it('should skip a term whose target contains a newline or carriage return', () => { + const { manager } = createManager(); + const withNewline = repeated('Save', 'Spei\nchern', 'a'); + const withCr = repeated('Open', 'Öff\rnen', 'b'); + + const sourceEntries = new Map([...withNewline.sourceEntries, ...withCr.sourceEntries]); + const localeEntries = new Map([...withNewline.localeEntries, ...withCr.localeEntries]); + + const result = manager.extractTerms(sourceEntries, new Map([['de', localeEntries]])); + + expect(result.has('de')).toBe(false); + }); + + it('should keep clean terms alongside skipped ones', () => { + const { manager } = createManager(); + const clean = repeated('Save', 'Speichern', 'a'); + const dirty = repeated('Bad\tterm', 'Schlecht', 'b'); + + const sourceEntries = new Map([...clean.sourceEntries, ...dirty.sourceEntries]); + const localeEntries = new Map([...clean.localeEntries, ...dirty.localeEntries]); + + const result = manager.extractTerms(sourceEntries, new Map([['de', localeEntries]])); + + expect(result.get('de')).toEqual({ Save: 'Speichern' }); + }); + }); + describe('syncGlossaries', () => { it('should create new glossary when none exists', async () => { const { manager, glossaryService } = createManager({ targetLocales: ['de'] }); @@ -305,6 +415,99 @@ describe('SyncGlossaryManager', () => { expect(glossaryService.getGlossaryByName).not.toHaveBeenCalled(); expect(result).toEqual({}); }); + + describe('idempotency', () => { + /** + * Run syncGlossaries twice against a service that stores what was + * uploaded and returns it the way the API does — through a TSV round trip. + */ + async function syncTwice( + sourceEntries: Map, + localeEntries: Map, + ): Promise<{ glossaryService: ReturnType }> { + const { manager, glossaryService } = createManager({ targetLocales: ['de'] }); + const info: GlossaryInfo = { + glossary_id: 'gid-1', + name: 'deepl-sync-en-de', + source_lang: 'en', + target_langs: ['de'], + dictionaries: [{ source_lang: 'en', target_lang: 'de', entry_count: 1 }], + creation_time: '2026-01-01T00:00:00Z', + }; + + let stored: Record | null = null; + glossaryService.getGlossaryByName.mockImplementation(async () => (stored ? info : null)); + glossaryService.createGlossary.mockImplementation(async (_n, _s, _t, entries) => { + stored = roundTrip(entries); + return info; + }); + glossaryService.getGlossaryEntries.mockImplementation(async () => stored ?? {}); + glossaryService.updateGlossary.mockResolvedValue(undefined); + + const targetEntries = new Map([['de', localeEntries]]); + await manager.syncGlossaries(sourceEntries, targetEntries); + await manager.syncGlossaries(sourceEntries, targetEntries); + + return { glossaryService }; + } + + it('should create once and never update when the source terms are unchanged', async () => { + const { sourceEntries, localeEntries } = repeated('Save', 'Speichern', 'k'); + + const { glossaryService } = await syncTwice(sourceEntries, localeEntries); + + expect(glossaryService.createGlossary).toHaveBeenCalledTimes(1); + expect(glossaryService.updateGlossary).toHaveBeenCalledTimes(0); + }); + + it('should not re-upload when a term differs only by surrounding whitespace', async () => { + const { sourceEntries, localeEntries } = repeated('Save ', 'Speichern', 'k'); + + const { glossaryService } = await syncTwice(sourceEntries, localeEntries); + + expect(glossaryService.createGlossary).toHaveBeenCalledTimes(1); + expect(glossaryService.updateGlossary).toHaveBeenCalledTimes(0); + }); + }); + + describe('per-locale failure isolation', () => { + it('should keep syncing other locales and name the failing locale in the warning', async () => { + const warnSpy = jest.spyOn(Logger, 'warn').mockImplementation(() => undefined); + try { + const { manager, glossaryService } = createManager({ targetLocales: ['de', 'fr'] }); + + glossaryService.getGlossaryByName.mockResolvedValue(null); + glossaryService.createGlossary.mockImplementation(async (name) => { + if (name === 'deepl-sync-en-de') { + throw new Error('glossary rejected by API'); + } + const info: GlossaryInfo = { + glossary_id: 'fr-id', + name, + source_lang: 'en', + target_langs: ['fr'], + dictionaries: [{ source_lang: 'en', target_lang: 'fr', entry_count: 1 }], + creation_time: '2026-01-01T00:00:00Z', + }; + return info; + }); + + const { sourceEntries, localeEntries } = repeated('Yes', 'Ja', 'k'); + const targetEntries = new Map([ + ['de', localeEntries], + ['fr', new Map([...localeEntries].map(([k]) => [k, 'Oui'] as [string, string]))], + ]); + + const result = await manager.syncGlossaries(sourceEntries, targetEntries); + + expect(result).toEqual({ 'en-fr': 'fr-id' }); + expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining('de')); + expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining('glossary rejected by API')); + } finally { + warnSpy.mockRestore(); + } + }); + }); }); describe('getProjectGlossary', () => { diff --git a/tests/unit/sync/sync-init.test.ts b/tests/unit/sync/sync-init.test.ts index 82c116f..e179b29 100644 --- a/tests/unit/sync/sync-init.test.ts +++ b/tests/unit/sync/sync-init.test.ts @@ -17,6 +17,7 @@ import { generateSyncConfig, writeSyncConfig, configExists, + resolveInitConfigPath, } from '../../../src/sync/sync-init'; const mockFg = fg as jest.MockedFunction; @@ -653,22 +654,69 @@ describe('sync-init', () => { it('should return true when config file exists', () => { fs.writeFileSync(path.join(testDir, '.deepl-sync.yaml'), 'version: 1\n'); - expect(configExists(testDir)).toBe(true); + expect(configExists(path.join(testDir, '.deepl-sync.yaml'))).toBe(true); }); it('should return false when config file does not exist', () => { - expect(configExists(testDir)).toBe(false); + expect(configExists(path.join(testDir, '.deepl-sync.yaml'))).toBe(false); + }); + }); + + describe('resolveInitConfigPath', () => { + it('should default to .deepl-sync.yaml in the cwd', () => { + expect(resolveInitConfigPath(testDir)).toBe(path.join(testDir, '.deepl-sync.yaml')); + }); + + it('should honor an explicit path with a custom basename', () => { + expect(resolveInitConfigPath(testDir, 'nested/custom.yaml')).toBe( + path.join(testDir, 'nested', 'custom.yaml'), + ); + }); + + it('should leave an absolute explicit path untouched', () => { + const abs = path.join(testDir, 'abs', 'custom.yaml'); + expect(resolveInitConfigPath(testDir, abs)).toBe(abs); }); }); describe('writeSyncConfig', () => { it('should write config file and return path', async () => { const content = 'version: 1\nsource_locale: en\n'; - const configPath = await writeSyncConfig(testDir, content); + const configPath = await writeSyncConfig( + path.join(testDir, '.deepl-sync.yaml'), + content, + ); expect(configPath).toBe(path.join(testDir, '.deepl-sync.yaml')); expect(fs.readFileSync(configPath, 'utf-8')).toBe(content); }); + + it('should leave no temp sibling behind and keep the previous config on write failure', async () => { + const configPath = path.join(testDir, '.deepl-sync.yaml'); + fs.writeFileSync(configPath, 'version: 1\nsource_locale: en\n', 'utf-8'); + + const renameSpy = jest + .spyOn(fs.promises, 'rename') + .mockRejectedValue(new Error('rename failed')); + try { + await expect( + writeSyncConfig(configPath, 'version: 1\nsource_locale: de\n'), + ).rejects.toThrow('rename failed'); + } finally { + renameSpy.mockRestore(); + } + + expect(fs.readFileSync(configPath, 'utf-8')).toBe('version: 1\nsource_locale: en\n'); + expect(fs.readdirSync(testDir).filter(name => name.includes('.tmp.'))).toEqual([]); + }); + + it('should create missing parent directories', async () => { + const target = path.join(testDir, 'deep', 'nested', 'custom.yaml'); + const configPath = await writeSyncConfig(target, 'version: 1\n'); + + expect(configPath).toBe(target); + expect(fs.existsSync(target)).toBe(true); + }); }); describe('non-interactive generation', () => { diff --git a/tests/unit/sync/tms-client.test.ts b/tests/unit/sync/tms-client.test.ts index c271d01..ef10b97 100644 --- a/tests/unit/sync/tms-client.test.ts +++ b/tests/unit/sync/tms-client.test.ts @@ -1,6 +1,13 @@ -import { TmsClient, resolveTmsCredentials, createTmsClient } from '../../../src/sync/tms-client'; +import { + TmsClient, + TmsTimeoutError, + MAX_PULL_BODY_BYTES, + resolveTmsCredentials, + createTmsClient, +} from '../../../src/sync/tms-client'; import type { SyncTmsConfig } from '../../../src/sync/types'; import { ConfigError, ValidationError } from '../../../src/utils/errors'; +import { ExitCode, exitCodeForError } from '../../../src/utils/exit-codes'; const mockFetch = jest.fn(); (global as unknown as Record)['fetch'] = mockFetch; @@ -113,6 +120,173 @@ describe('TmsClient HTTPS validation', () => { }); }); +describe('TmsClient URL construction', () => { + beforeEach(() => mockFetch.mockReset()); + + function clientFor(serverUrl: string): TmsClient { + return new TmsClient({ serverUrl, projectId: 'proj-1', apiKey: 'key', fetch: mockFetch }); + } + + it('should not double the separator when the server URL has a trailing slash', async () => { + mockFetch.mockResolvedValue({ ok: true, json: async () => ({}) }); + await clientFor('https://tms.example.com/').pushKey('greeting', 'de', 'Hallo'); + + expect(mockFetch).toHaveBeenCalledWith( + 'https://tms.example.com/api/projects/proj-1/keys/greeting', + expect.anything(), + ); + }); + + it('should preserve a base path on the server URL', async () => { + mockFetch.mockResolvedValue({ ok: true, json: async () => ({}) }); + await clientFor('https://tms.example.com/tms/').pushKey('greeting', 'de', 'Hallo'); + + expect(mockFetch).toHaveBeenCalledWith( + 'https://tms.example.com/tms/api/projects/proj-1/keys/greeting', + expect.anything(), + ); + }); + + it('should reject a server URL carrying a query string instead of truncating the path', async () => { + await expect(clientFor('https://tms.example.com/?token=abc').pushKey('k', 'de', 'v')).rejects.toThrow( + ConfigError, + ); + expect(mockFetch).not.toHaveBeenCalled(); + }); + + it('should reject a server URL carrying a fragment', async () => { + await expect(clientFor('https://tms.example.com/#frag').pushKey('k', 'de', 'v')).rejects.toThrow( + ConfigError, + ); + expect(mockFetch).not.toHaveBeenCalled(); + }); + + it('should keep the export query string intact for pullKeys', async () => { + mockFetch.mockResolvedValue({ ok: true, json: async () => ({}) }); + await clientFor('https://tms.example.com/').pullKeys('de'); + + expect(mockFetch).toHaveBeenCalledWith( + 'https://tms.example.com/api/projects/proj-1/keys/export?format=json&locale=de', + expect.anything(), + ); + }); +}); + +describe('TmsClient error message redaction', () => { + beforeEach(() => mockFetch.mockReset()); + + it('should redact credentials embedded in the server URL from timeout messages', async () => { + jest.useFakeTimers(); + try { + const stubFetch = jest.fn().mockImplementation((_url: string, init?: RequestInit) => { + return new Promise((_resolve, reject) => { + init?.signal?.addEventListener('abort', () => { + const err = new Error('aborted'); + err.name = 'AbortError'; + reject(err); + }); + }); + }); + const client = new TmsClient({ + serverUrl: 'https://alice:s3cret@tms.example.com', + projectId: 'p', + apiKey: 'k', + fetch: stubFetch, + timeoutMs: 1000, + retry: { maxAttempts: 1 }, + }); + + const promise = client.pushKey('k', 'de', 'v'); + promise.catch(() => undefined); + await jest.advanceTimersByTimeAsync(1001); + + await expect(promise).rejects.toThrow( + expect.objectContaining({ message: expect.not.stringContaining('s3cret') }), + ); + } finally { + jest.useRealTimers(); + } + }); + + it('should redact credentials from the invalid-server-URL error', async () => { + const client = new TmsClient({ + serverUrl: 'not a url with s3cret inside', + projectId: 'p', + apiKey: 'k', + fetch: mockFetch, + }); + + await expect(client.pushKey('k', 'de', 'v')).rejects.toThrow( + expect.objectContaining({ message: expect.not.stringContaining('s3cret') }), + ); + }); +}); + +describe('TmsTimeoutError classification', () => { + it('should exit with the network error code', () => { + expect(exitCodeForError(new TmsTimeoutError('TMS request timed out'))).toBe(ExitCode.NetworkError); + expect(exitCodeForError(new TmsTimeoutError('TMS request timed out'))).toBe(5); + }); +}); + +describe('TmsClient.pullKeys response size cap', () => { + beforeEach(() => mockFetch.mockReset()); + + const client = new TmsClient({ + serverUrl: 'https://tms.example.com', + projectId: 'proj-1', + apiKey: 'key', + }); + + it('should reject a response whose declared content-length exceeds the cap', async () => { + mockFetch.mockResolvedValue({ + ok: true, + headers: new Headers({ 'content-length': String(MAX_PULL_BODY_BYTES + 1) }), + json: async () => ({}), + }); + + await expect(client.pullKeys('de')).rejects.toThrow(ValidationError); + await expect(client.pullKeys('de')).rejects.toThrow(/size|large|bytes/i); + }); + + it('should reject a streamed response that exceeds the cap without declaring a length', async () => { + const chunk = new Uint8Array(1024 * 1024); + chunk.fill(0x61); + let emitted = 0; + mockFetch.mockResolvedValue({ + ok: true, + headers: new Headers(), + body: new ReadableStream({ + pull(controller) { + if (emitted > MAX_PULL_BODY_BYTES) { + controller.close(); + return; + } + emitted += chunk.byteLength; + controller.enqueue(chunk); + }, + }), + }); + + await expect(client.pullKeys('de')).rejects.toThrow(ValidationError); + }); + + it('should parse a streamed response that stays under the cap', async () => { + mockFetch.mockResolvedValue({ + ok: true, + headers: new Headers(), + body: new ReadableStream({ + start(controller) { + controller.enqueue(new TextEncoder().encode('{"greeting":"Hallo"}')); + controller.close(); + }, + }), + }); + + await expect(client.pullKeys('de')).resolves.toEqual({ greeting: 'Hallo' }); + }); +}); + describe('TmsClient fetch injection', () => { beforeEach(() => mockFetch.mockReset()); diff --git a/tests/unit/voice-client.test.ts b/tests/unit/voice-client.test.ts index 302b8e5..b6e470d 100644 --- a/tests/unit/voice-client.test.ts +++ b/tests/unit/voice-client.test.ts @@ -455,18 +455,22 @@ describe('VoiceClient', () => { ); }); - it('should handle WebSocket errors via callback', () => { + // Transport errors belong to whoever owns the socket lifecycle: routing + // them into onError made them indistinguishable from server error + // messages, and left every socket carrying two 'error' listeners. + it('should leave transport errors to the socket owner', () => { const onError = jest.fn(); const ws = client.createWebSocket('wss://voice.deepl.com/ws', 'token', { onError, }); + const transportErrors: Error[] = []; + ws.on('error', (error: Error) => transportErrors.push(error)); ws.emit('error', new Error('Connection failed')); - expect(onError).toHaveBeenCalledWith( - expect.objectContaining({ - error_message: 'Connection failed', - }) - ); + + expect(ws.listenerCount('error')).toBe(1); + expect(transportErrors).toHaveLength(1); + expect(onError).not.toHaveBeenCalled(); }); it('should ignore unparseable messages', () => { diff --git a/tests/unit/voice-service.test.ts b/tests/unit/voice-service.test.ts index 4f4d470..a004ed2 100644 --- a/tests/unit/voice-service.test.ts +++ b/tests/unit/voice-service.test.ts @@ -480,7 +480,10 @@ describe('VoiceService', () => { }); mockClient.createWebSocket.mockImplementation((_url: string, _token: string, _callbacks: any) => { process.nextTick(() => { + // ws always emits close after an error; the session reports the + // transport failure from there. mockWs.emit('error', new Error('Connection refused')); + mockWs.emit('close'); }); return mockWs as any; }); @@ -489,12 +492,14 @@ describe('VoiceService', () => { service.translateFile(testFile, { targetLangs: ['de'], chunkInterval: 0, + reconnect: false, }), ).rejects.toThrow(VoiceError); await expect( service.translateFile(testFile, { targetLangs: ['de'], chunkInterval: 0, + reconnect: false, }), ).rejects.toThrow(/WebSocket connection failed: Connection refused/); }); diff --git a/tests/unit/voice-stream-session.test.ts b/tests/unit/voice-stream-session.test.ts index ceb81e0..cb8e783 100644 --- a/tests/unit/voice-stream-session.test.ts +++ b/tests/unit/voice-stream-session.test.ts @@ -252,25 +252,31 @@ describe('VoiceStreamSession', () => { }); describe('error handling', () => { - it('should reject with VoiceError on WebSocket error', async () => { + // A socket error is always followed by a close event; the transport + // failure is reported from there so reconnect stays reachable. + it('should reject with VoiceError on WebSocket error when reconnect is disabled', async () => { const EventEmitter = require('events'); - const mockWs = new EventEmitter(); - mockWs.readyState = 1; - mockWs.send = jest.fn(); - mockWs.close = jest.fn(); mockClient.createWebSocket.mockImplementation(() => { + const mockWs = new EventEmitter(); + mockWs.readyState = 1; + mockWs.send = jest.fn(); + mockWs.close = jest.fn(); + process.nextTick(() => { mockWs.emit('error', new Error('Connection refused')); + mockWs.readyState = 3; + mockWs.emit('close'); }); return mockWs; }); - const streamSession = new VoiceStreamSession(mockClient, session, options); - - await expect(streamSession.run(emptyChunks())).rejects.toThrow(VoiceError); + const noReconnect = { ...options, reconnect: false }; + await expect( + new VoiceStreamSession(mockClient, session, noReconnect).run(emptyChunks()), + ).rejects.toThrow(VoiceError); await expect( - new VoiceStreamSession(mockClient, session, options).run(emptyChunks()), + new VoiceStreamSession(mockClient, session, noReconnect).run(emptyChunks()), ).rejects.toThrow(/WebSocket connection failed: Connection refused/); }); @@ -319,6 +325,117 @@ describe('VoiceStreamSession', () => { }); describe('reconnection', () => { + it('should reconnect after a transport error closes the socket', async () => { + const EventEmitter = require('events'); + const onReconnecting = jest.fn(); + + mockClient.reconnectSession.mockResolvedValue({ + streaming_url: 'wss://test-new.deepl.com/stream', + token: 'token-2', + }); + + let wsCallCount = 0; + mockClient.createWebSocket.mockImplementation((_url, _token, callbacks) => { + wsCallCount++; + const mockWs = new EventEmitter(); + mockWs.readyState = 1; + mockWs.send = jest.fn(); + mockWs.close = jest.fn(); + + if (wsCallCount === 1) { + process.nextTick(() => { + mockWs.emit('open'); + process.nextTick(() => { + mockWs.emit('error', new Error('read ECONNRESET')); + mockWs.readyState = 3; + mockWs.emit('close'); + }); + }); + } else { + process.nextTick(() => { + mockWs.emit('open'); + process.nextTick(() => callbacks.onEndOfStream?.()); + }); + } + return mockWs; + }); + + const streamSession = new VoiceStreamSession( + mockClient, session, options, { onReconnecting }, + ); + const result = await streamSession.run(emptyChunks()); + + expect(result.sessionId).toBe('session-1'); + expect(mockClient.reconnectSession).toHaveBeenCalledTimes(1); + expect(mockClient.reconnectSession).toHaveBeenCalledWith('token-1'); + expect(onReconnecting).toHaveBeenCalledWith(1); + }); + + it('should reject with the transport error once reconnect attempts are exhausted', async () => { + const EventEmitter = require('events'); + + mockClient.reconnectSession.mockResolvedValue({ + streaming_url: 'wss://test-new.deepl.com/stream', + token: 'token-2', + }); + + mockClient.createWebSocket.mockImplementation(() => { + const mockWs = new EventEmitter(); + mockWs.readyState = 1; + mockWs.send = jest.fn(); + mockWs.close = jest.fn(); + + process.nextTick(() => { + mockWs.emit('open'); + process.nextTick(() => { + mockWs.emit('error', new Error('read ECONNRESET')); + mockWs.readyState = 3; + mockWs.emit('close'); + }); + }); + return mockWs; + }); + + await expect( + new VoiceStreamSession(mockClient, session, { + ...options, + maxReconnectAttempts: 1, + }).run(emptyChunks()), + ).rejects.toThrow(/WebSocket connection failed: read ECONNRESET/); + expect(mockClient.reconnectSession).toHaveBeenCalledTimes(1); + }); + + it('should close the audio input when reconnect fails', async () => { + const EventEmitter = require('events'); + const input = trackedChunks(); + + mockClient.reconnectSession.mockRejectedValue( + new VoiceError('Voice API access denied.'), + ); + + mockClient.createWebSocket.mockImplementation(() => { + const mockWs = new EventEmitter(); + // Never open for writing, so the chunk pump parks waiting for a + // usable socket — the state that used to leak the generator. + mockWs.readyState = 3; + mockWs.send = jest.fn(); + mockWs.close = jest.fn(); + + process.nextTick(() => { + mockWs.emit('open'); + process.nextTick(() => { + process.nextTick(() => mockWs.emit('close')); + }); + }); + return mockWs; + }); + + const streamSession = new VoiceStreamSession(mockClient, session, options); + await expect(streamSession.run(input.chunks)).rejects.toThrow(VoiceError); + + expect(input.closed()).toBe(true); + }); + it('should reconnect on unexpected WebSocket close', async () => { const EventEmitter = require('events'); const onReconnecting = jest.fn(); @@ -659,3 +776,17 @@ async function* singleChunk(data: Buffer): AsyncGenerator { async function* throwingChunks(): AsyncGenerator { throw new Error('chunk error'); } + +/** A chunk source that reports whether the session closed it. */ +function trackedChunks(): { chunks: AsyncGenerator; closed: () => boolean } { + let closed = false; + async function* generate(): AsyncGenerator { + try { + yield Buffer.from('audio-1'); + yield Buffer.from('audio-2'); + } finally { + closed = true; + } + } + return { chunks: generate(), closed: () => closed }; +}