Skip to content

Commit f827117

Browse files
sjsyrekclaude
andauthored
fix: land the beads-issue fix batch (sync, formats, api, watch) (#104)
Squash of 21 commits; individual changes: - test: remove global p-limit and fast-glob mocks that disabled all concurrency coverage - fix(sync,formats): stop prototype-named keys destroying translations - fix(glossary,sync): null-prototype the remaining untrusted-keyed accumulators - fix(formats): handle CRLF line endings in line-based parsers - fix(formats): accept and preserve attributes on XLIFF segment and target - fix(formats): stop Android entity compounding and .properties emoji loss - fix(formats): write new TOML keys into their own section - fix(formats): prevent CDATA breakout in Android XML reconstruct - fix(formats): preserve xcstrings plural variations and strip ARB BOM - fix(formats): emit TOML multi-line blocks verbatim and mark their key used - fix(sync,cache): reject invalid concurrency and stop signal handlers reporting success - fix(api,watch): treat blank Retry-After as absent and fix debounce bookkeeping - fix(sync)!: re-translate instead of writing source text as a translation - fix(sync): judge staleness per target locale so --frozen can detect drift - fix(sync): fail the run when a locale fails completely, and never auto-commit it - fix(sync): preserve ICU messages with surrounding text and detect broken structure - fix(sync)!: contain bucket include globs and stop --dry-run touching the tree - fix(utils): make placeholder restoration terminate in a single pass - test(e2e): isolate the sync --watch config dir to fix an intermittent failure - test(harness): fail fast when tests run without a build - chore(comments): remove development narration from code comments 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
1 parent fb2ca2d commit f827117

114 files changed

Lines changed: 2613 additions & 393 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎CHANGELOG.md‎

Lines changed: 31 additions & 0 deletions
Large diffs are not rendered by default.

‎src/api/http-client.ts‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -371,7 +371,7 @@ export class HttpClient {
371371
// Sanitize the server-returned message before any interpolation into
372372
// user-facing error strings. Defense-in-depth against a malicious or
373373
// buggy server scribbling ANSI escape codes / control chars on the
374-
// user's terminal. Mirrors the TMS-client hardening (sync-pagq.7).
374+
// user's terminal, matching the sanitization in tms-client.ts.
375375
// Coalesce to '' before sanitizing — some axios error shapes have no
376376
// `.message` field, and sanitizeForTerminal expects a string.
377377
const message = sanitizeForTerminal(responseData?.message ?? error.message ?? '');
@@ -441,6 +441,12 @@ export class HttpClient {
441441
if (headerValue === undefined || headerValue === null) {
442442
return undefined;
443443
}
444+
// Number('') and Number(' ') are 0, which passes the finite check below
445+
// and returns a 0 ms delay — collapsing 429 backoff into a tight retry
446+
// loop against an endpoint that is already rate-limiting us.
447+
if (headerValue.trim() === '') {
448+
return undefined;
449+
}
444450

445451
const seconds = Number(headerValue);
446452
if (!isNaN(seconds) && isFinite(seconds)) {
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
import { InvalidArgumentError } from 'commander';
2+
3+
/**
4+
* Commander option parser for a bounded positive integer.
5+
*
6+
* A bare `parseInt` yields NaN for a non-numeric value, and NaN survives the
7+
* `??` default chains used downstream — so an invalid `--concurrency` reached
8+
* the worker-pool sizing and silently produced zero workers. Rejecting at the
9+
* boundary means the user is told instead.
10+
*/
11+
export function parsePositiveIntOption(value: string, name: string, max: number): number {
12+
const parsed = Number.parseInt(value, 10);
13+
if (!Number.isInteger(parsed) || parsed <= 0 || parsed > max) {
14+
throw new InvalidArgumentError(
15+
`--${name} must be an integer between 1 and ${max}, got '${value}'`,
16+
);
17+
}
18+
return parsed;
19+
}

‎src/cli/commands/sync-command.ts‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import { ValidationError } from '../../utils/errors.js';
66
import { ExitCode } from '../../utils/exit-codes.js';
77
import { LOCK_FILE_NAME } from '../../sync/types.js';
88
import { sweepStaleBackups as sweepStaleBackupsImpl, DEFAULT_BAK_SWEEP_MAX_AGE_SECONDS } from '../../sync/sync-bak-cleanup.js';
9+
import { claimGracefulShutdown } from '../../utils/signal-exit.js';
910

1011
export const STALE_BACKUP_AGE_MS = DEFAULT_BAK_SWEEP_MAX_AGE_SECONDS * 1000;
1112

@@ -269,7 +270,7 @@ export class SyncCommand {
269270
process.exitCode = ExitCode.PartialFailure;
270271
}
271272

272-
if (options.autoCommit && !result.dryRun && !result.driftDetected && result.fileResults.length > 0) {
273+
if (options.autoCommit && result.success && !result.dryRun && !result.driftDetected && result.fileResults.length > 0) {
273274
await this.autoCommitTranslations(result, config);
274275
}
275276

@@ -383,7 +384,7 @@ export class SyncCommand {
383384
backupTracker,
384385
});
385386
this.displayResult(result, 'text');
386-
if (options.autoCommit && !result.dryRun && !result.driftDetected && result.fileResults.length > 0) {
387+
if (options.autoCommit && result.success && !result.dryRun && !result.driftDetected && result.fileResults.length > 0) {
387388
await this.autoCommitTranslations(result, activeConfig);
388389
}
389390
},
@@ -400,6 +401,10 @@ export class SyncCommand {
400401
debounceMs,
401402
});
402403

404+
// Watch mode treats SIGINT/SIGTERM as its normal stop signal and shuts
405+
// down successfully, so the CLI's signal-exit must not force a failure code.
406+
claimGracefulShutdown();
407+
403408
await new Promise<void>((resolve) => {
404409
const onStop = (): void => {
405410
Logger.info('\nStopping watch...');

‎src/cli/commands/sync/register-sync-root.ts‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { ExitCode } from '../../../utils/exit-codes.js';
33
import { ValidationError } from '../../../utils/errors.js';
44
import { createSyncCommand, type ServiceDeps } from '../service-factory.js';
55
import { emitJsonErrorAndExit } from './sync-options.js';
6+
import { parsePositiveIntOption } from '../parse-int-option.js';
67

78
export function registerSyncRoot(program: Command, deps: ServiceDeps): Command {
89
return program
@@ -46,7 +47,8 @@ export function registerSyncRoot(program: Command, deps: ServiceDeps): Command {
4647
.addOption(new Option('--context').hideHelp())
4748
.addOption(new Option('--no-context').hideHelp())
4849
.optionsGroup('Performance:')
49-
.option('--concurrency <number>', 'Max parallel locale translations (default: 5)', parseInt)
50+
.option('--concurrency <number>', 'Max parallel locale translations (default: 5)', (v) =>
51+
parsePositiveIntOption(v, 'concurrency', 100))
5052
.option('--batch', 'Force batch mode (fastest, no context or instructions)')
5153
.option(
5254
'--no-batch',
@@ -61,7 +63,8 @@ export function registerSyncRoot(program: Command, deps: ServiceDeps): Command {
6163
)
6264
.optionsGroup('Watch:')
6365
.option('--watch', 'Watch source files and auto-sync on changes')
64-
.option('--debounce <ms>', 'Debounce delay for watch mode (default: 500ms)', parseInt)
66+
.option('--debounce <ms>', 'Debounce delay for watch mode (default: 500ms)', (v) =>
67+
parsePositiveIntOption(v, 'debounce', 600_000))
6568
.optionsGroup('Safety:')
6669
.option('-y, --yes', 'Skip --force confirmation prompt (required when CI=true)')
6770
.optionsGroup('Output:')

‎src/cli/commands/translate/translation-options-factory.ts‎

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,9 @@ import { buildTranslationOptions as buildBaseLegacy, resolveGlossaryId } from '.
1010
* flags — the shape all translate handlers (text, file, directory, document)
1111
* agreed on. Produces the same field set regardless of handler.
1212
*
13-
* Previously each handler called `buildTranslationOptions` (in translate-utils)
14-
* directly and then layered its own downstream shaping. That layering has been
15-
* extracted into `applySharedTmAndGlossary` so the shared surface area is
16-
* centralized; handlers only retain handler-specific shaping (custom
17-
* instructions, style id, XML tag handling, multi-target stripping of
18-
* `targetLang`, etc.).
13+
* Shared downstream shaping lives in `applySharedTmAndGlossary`; handlers
14+
* keep only handler-specific shaping (custom instructions, style id, XML tag
15+
* handling, multi-target stripping of `targetLang`, etc.).
1916
*
2017
* Intentional drift NOT folded in:
2118
* - `SyncCommand` builds `TranslateOptions` from resolved config, not CLI
@@ -48,7 +45,7 @@ export interface SharedTmAndGlossaryDeps {
4845
* default onto a base `TranslationParams`-compatible object. Mutates `base`
4946
* in place so handlers can compose additional downstream shaping.
5047
*
51-
* Consolidates 5 copies of the same block across text + file handlers. All
48+
* Shared by the text + file handlers. All
5249
* validation (required `--from`, TM-requires-quality_optimized, extended-lang
5350
* constraints) remains in the caller so per-handler error messages are
5451
* preserved; this helper is called only after validation passes.

‎src/cli/index.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ import { registerDetect } from './commands/register-detect.js';
4040
import { registerDescribe } from './commands/register-describe.js';
4141
import { validateApiUrl } from '../utils/validate-url.js';
4242
import { resolveEndpoint } from '../utils/resolve-endpoint.js';
43+
import { installSignalExit } from '../utils/signal-exit.js';
4344

4445
// Get __dirname equivalent in ESM
4546
const __filename = fileURLToPath(import.meta.url);
@@ -352,6 +353,10 @@ program.on('command:*', (operands: string[]) => {
352353
process.exit(ExitCode.InvalidInput);
353354
});
354355

356+
// Own process termination on signals, so no component's cleanup handler has to
357+
// call process.exit() and thereby preempt the others.
358+
installSignalExit();
359+
355360
// Show help and exit 0 if no arguments provided
356361
if (!process.argv.slice(2).length) {
357362
program.outputHelp();

‎src/formats/android-xml.ts‎

Lines changed: 51 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,35 @@ const STRING_ARRAY_RE =
2121

2222
const ARRAY_ITEM_RE = /<item>([\s\S]*?)<\/item>/g;
2323

24+
const XML_ENTITY_RE = /&(?:#x([0-9a-fA-F]+)|#(\d+)|(amp|lt|gt|quot|apos));/g;
25+
26+
/**
27+
* Decodes XML entities in a single pass, so a literal `&amp;lt;` decodes to
28+
* `&lt;` rather than collapsing all the way to `<`.
29+
*/
30+
function decodeXmlEntities(value: string): string {
31+
return value.replace(XML_ENTITY_RE, (match, hex: string | undefined, dec: string | undefined, named: string | undefined) => {
32+
if (hex !== undefined) {
33+
const code = Number.parseInt(hex, 16);
34+
return code >= 0 && code <= 0x10ffff ? String.fromCodePoint(code) : match;
35+
}
36+
if (dec !== undefined) {
37+
const code = Number.parseInt(dec, 10);
38+
return code >= 0 && code <= 0x10ffff ? String.fromCodePoint(code) : match;
39+
}
40+
switch (named) {
41+
case 'amp': return '&';
42+
case 'lt': return '<';
43+
case 'gt': return '>';
44+
case 'quot': return '"';
45+
case 'apos': return "'";
46+
default: return match;
47+
}
48+
});
49+
}
50+
2451
function unescapeAndroid(value: string): string {
25-
return value.replace(/\\(\\|'|"|n|t|r)/g, (_match, ch: string) => {
52+
const withoutBackslashEscapes = value.replace(/\\(\\|'|"|n|t|r)/g, (_match, ch: string) => {
2653
switch (ch) {
2754
case '\\': return '\\';
2855
case "'": return "'";
@@ -33,6 +60,7 @@ function unescapeAndroid(value: string): string {
3360
default: return ch;
3461
}
3562
});
63+
return decodeXmlEntities(withoutBackslashEscapes);
3664
}
3765

3866
function escapeAndroid(value: string): string {
@@ -41,9 +69,11 @@ function escapeAndroid(value: string): string {
4169
.replace(/\n/g, '\\n')
4270
.replace(/'/g, "\\'")
4371
.replace(/"/g, '\\"')
72+
// & must precede < and >, or the entities produced below get re-escaped
73+
// into &amp;lt; — which compounds on every subsequent sync run.
74+
.replace(/&/g, '&amp;')
4475
.replace(/</g, '&lt;')
45-
.replace(/>/g, '&gt;')
46-
.replace(/&/g, '&amp;');
76+
.replace(/>/g, '&gt;');
4777
}
4878

4979
export class AndroidXmlFormatParser implements FormatParser {
@@ -229,16 +259,30 @@ export class AndroidXmlFormatParser implements FormatParser {
229259
}
230260

231261
private decodeValue(raw: string): string {
232-
const cdataMatch = /^<!\[CDATA\[([\s\S]*)\]\]>$/.exec(raw);
233-
if (cdataMatch) {
234-
return cdataMatch[1]!;
262+
if (raw.startsWith('<![CDATA[')) {
263+
// Concatenate adjacent sections: escapeForReconstruct splits on "]]>"
264+
// into `]]]]><![CDATA[>`, so a literal "]]>" spans two sections.
265+
const sectionRe = /<!\[CDATA\[([\s\S]*?)\]\]>/g;
266+
let joined = '';
267+
let consumedTo = 0;
268+
let match: RegExpExecArray | null;
269+
while ((match = sectionRe.exec(raw)) !== null) {
270+
if (match.index !== consumedTo) break;
271+
joined += match[1]!;
272+
consumedTo = match.index + match[0].length;
273+
}
274+
if (consumedTo === raw.length) return joined;
235275
}
236276
return unescapeAndroid(raw);
237277
}
238278

239279
private escapeForReconstruct(originalInner: string, translation: string): string {
240280
if (/^<!\[CDATA\[/.test(originalInner)) {
241-
return `<![CDATA[${translation}]]>`;
281+
// A translation containing "]]>" would close the section early and the
282+
// remainder would be parsed as XML. Splitting into adjacent CDATA
283+
// sections keeps the text literal without escaping it.
284+
const safe = translation.replace(/]]>/g, ']]]]><![CDATA[>');
285+
return `<![CDATA[${safe}]]>`;
242286
}
243287
return escapeAndroid(translation);
244288
}

‎src/formats/arb.ts‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,18 @@
11
import type { ExtractedEntry, FormatParser, TranslatedEntry } from './format.js';
22
import { detectIndent } from './util/detect-indent.js';
33

4+
/** JSON.parse rejects a leading BOM, which editors on Windows add routinely. */
5+
function stripBom(content: string): string {
6+
return content.replace(/^\uFEFF/, '');
7+
}
8+
49
export class ArbFormatParser implements FormatParser {
510
readonly name = 'ARB (Flutter)';
611
readonly configKey = 'arb';
712
readonly extensions = ['.arb'];
813

914
extract(content: string): ExtractedEntry[] {
10-
const data = JSON.parse(content) as Record<string, unknown>;
15+
const data = JSON.parse(stripBom(content)) as Record<string, unknown>;
1116
const entries: ExtractedEntry[] = [];
1217

1318
for (const key of Object.keys(data)) {
@@ -41,7 +46,7 @@ export class ArbFormatParser implements FormatParser {
4146
}
4247

4348
reconstruct(content: string, entries: TranslatedEntry[]): string {
44-
const data = JSON.parse(content) as Record<string, unknown>;
49+
const data = JSON.parse(stripBom(content)) as Record<string, unknown>;
4550
const indent = detectIndent(content);
4651
const trailingNewline = content.endsWith('\n');
4752

@@ -83,7 +88,7 @@ export class ArbFormatParser implements FormatParser {
8388
}
8489

8590
extractContext(content: string, key: string): string | undefined {
86-
const data = JSON.parse(content) as Record<string, unknown>;
91+
const data = JSON.parse(stripBom(content)) as Record<string, unknown>;
8792
const metaKey = `@${key}`;
8893
const meta = data[metaKey];
8994
if (typeof meta === 'object' && meta !== null) {

‎src/formats/ios-strings.ts‎

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ export class IosStringsFormatParser implements FormatParser {
1212

1313
extract(content: string): ExtractedEntry[] {
1414
const entries: ExtractedEntry[] = [];
15-
const lines = content.split('\n');
15+
const lines = content.split(/\r?\n/);
1616
let pendingComment: string | undefined;
1717

1818
for (let i = 0; i < lines.length; i++) {
@@ -67,18 +67,13 @@ export class IosStringsFormatParser implements FormatParser {
6767
return entries;
6868
}
6969

70-
// Intentionally NOT migrated to PendingCommentBuffer: deferred alongside po
71-
// because the forward-only helper interface was frozen at the toml +
72-
// properties shape and not re-opened. ios-strings is a plausible future
73-
// migration once the helper grows a shape that also accommodates po's
74-
// backtrack idiom.
7570
reconstruct(content: string, entries: TranslatedEntry[]): string {
7671
const translations = new Map<string, string>();
7772
for (const entry of entries) {
7873
translations.set(entry.key, entry.translation);
7974
}
8075

81-
const lines = content.split('\n');
76+
const lines = content.split(/\r?\n/);
8277
const result: string[] = [];
8378
let pendingComments: string[] = [];
8479
let inBlockComment = false;

0 commit comments

Comments
 (0)