Skip to content

ci: gate locale parity and dead keys with an i18n job - #368

Closed
RonenMars wants to merge 1 commit into
mainfrom
ci/i18n-parity-gate
Closed

ci: gate locale parity and dead keys with an i18n job#368
RonenMars wants to merge 1 commit into
mainfrom
ci/i18n-parity-gate

Conversation

@RonenMars

Copy link
Copy Markdown
Owner

The locale gates live at the root of __tests__/, so no CI job ever matched them.
test:unit runs __tests__/unit and test:integration runs __tests__/integration, which means __tests__/i18n-completeness.test.ts (and __tests__/i18n.test.ts) have never run on a pull request.

Type check does not cover this gap.
lib/i18n.types.ts types t() against the English locale JSON, so a missing key is a compile error — but a missing translation is not, and it reaches users as silent English fallback in the middle of an RTL layout.

This was not hypothetical.
Two open PRs added English-only keys and stayed green: #343 (feedback.success.viaSentry, viaEmail) and #354 (conversation.resume.takeOver, takeOverFailed), each missing all three of ar/he/ru.
Both have since been fixed on their own branches.
It also means the new dead-key gate added by #356 would land dead on arrival, since it sits at the same root level.

What this does

Adds a test:i18n script (jest --ci --testPathPattern='__tests__/i18n') and an i18n CI job alongside Lint and Type check, following the same [skip-ci] gate pattern as the existing jobs.
The pattern also picks up __tests__/i18n-unused-keys.test.ts automatically once #356 merges.

Verification

npm run test:i18n runs both suites and passes on main.
Deleting a single key from locales/ar/common.json turns the job red, confirming the gate actually catches a regression rather than merely passing.

Once this merges, i18n should be added to the required status checks in branch protection so it can block a merge.

@RonenMars RonenMars closed this Jul 31, 2026
RonenMars added a commit that referenced this pull request Aug 1, 2026
## Why

The runbook described itself as a "live procedure — edit it as PRs
land", but the chain it sequenced is gone.

Verified 2026-08-01 — every PR it orchestrated is `CLOSED` with
`mergedAt=null`, i.e. **closed, not merged**:

```
#339 #341 #343 #345 #346 #347 #353 #354 #355 #356 #357 #358 #359
#360 #361 #362 #363 #364 #368 #372 #373 #376 #385 #386 #387
```

The work reached `main` through the integration branch instead, and the
PRs were closed as redundant.
The integration branch the runbook treated as proof the set could
coexist, `integration-dev/v1.0.0-2026-07-22`, has been deleted.

That invalidates the phase order, the per-PR loop, the pre-flight sweep,
the decision to close `#355`, the conflict **A–I** resolutions, and the
definition of done.
The only PR row still accurate is `#291`, which is still open and still
excluded by standing request.

## What changed

The body becomes a tombstone: what the runbook was for, what actually
happened, and a table pointing at where each still-true trap now lives.
The file is kept rather than deleted so inbound links resolve and nobody
rebuilds the same plan from the merge report.

Two of its four traps were already in `docs/troubleshooting.md` (the
`.claude/` worktree gotcha and serial `SessionScreen` verification). The
other two are lifted there now under a new "CI signals" section, because
both are repo-wide facts rather than properties of those branches:

- a `[skip-ci]` head commit reports every required context green in
seconds having run nothing
- a stacked PR (base ≠ `main`) still gets full CI here, since
`test.yml`'s `pull_request` trigger has no `branches:` filter

Conflict **J** is obsolete for a separate reason: those
`ios/Podfile.lock` checksums encode the checkout's absolute path, so
resolving it was never meaningful. #469 added
`scripts/reset-podfile-lock-path-noise.sh`, which drops that drift.

## Two live findings, one already fixed

The runbook's `[skip-ci]` section claimed the required contexts were
`Type check`, `Unit tests`, `Integration tests`, `Lint`. The ruleset
actually required **only `Lint` and `Type check`** — unit and
integration tests could not block a merge at all, which makes the
`[skip-ci]` trap worse than documented, not better.

Its one outstanding post-merge action (add `i18n` to required checks,
never done since #368) is now complete: `Unit tests`, `Integration
tests` and `i18n` were added to the `main` ruleset on 2026-08-01.
Confirmed safe first — all three report `pass` on a real `chore/bump-**`
version-bump PR (#472), so the deploy pipeline's bumps are not
deadlocked.

Note: `.github/workflows/test.yml:25` carries the same wrong claim about
which contexts are required. Left alone here to keep this docs-only.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant