Repository navigation
fix(mobile): localize security agent sync errors - #7888
Merged
Merged
Conversation
The security operation-ledger markers the server raises for sync and dismissal failures were rendered verbatim, so a Japanese UI showed the raw English sentence above an otherwise translated screen. Map every failure onto catalog copy once at the mutation boundary, the same way the PR-review operation ledger does: known markers keep their own sentence, the in-progress duplicate keeps the per-surface copy, and any other message reads one localized generic sentence. The mapped error carries the retryability decision taken from the raw error, so the key-rotation and disabled-control contracts still work after the message is replaced. The shared persistence-failure sentence moves to common.actionNotRecorded, which the PR-review surfaces now point at too. Covered by the ledger unit tests, a mounted dashboard test asserting the Japanese ambiguous copy renders, and an Android run against the local stack with an unreachable sync worker.
iscekic
added
drain
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
Contributor
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Executive SummaryThe only change since the last review at Assumptions: Files Reviewed (104 files)
Previous Review Summaries (4 snapshots, latest commit ad46af4)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit ad46af4)Status: No Issues Found | Recommendation: Merge Executive SummaryThe follow-up commit Files Reviewed (3 files)
Previous review (commit 6a22750)Status: 2 Issues Found | Recommendation: Address before merge Executive SummaryThe incremental fix is functionally sound: the durable dismiss draft now persists a catalog key and re-translates at render, and the terminal fallback uses non-retry copy. The two findings are maintainability issues in the new default-filling path (a no-op helper and a mutating type predicate), not runtime defects. Overview
Issue Details (click to expand)SUGGESTION
Files Reviewed (10 files)
Earlier findings are resolved at HEAD: the terminal fallback now maps to Fix these issues in Kilo Cloud Previous review (commit a0f3594)Status: 1 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)SUGGESTION
Files Reviewed (4 files)
Fix these issues in Kilo Cloud Previous review (commit 3fda4e8)Status: 2 Issues Found | Recommendation: Address before merge Executive SummaryThe new localization boundary is sound and the retryability contract is preserved, but the generic fallback replaces terminal server errors with retry copy while the controls are disabled, and the durable dismiss card persists a language snapshot. Overview
Issue Details (click to expand)WARNING
SUGGESTION
Files Reviewed (100 files)
Reviewed by deepseek-flash-latest · Input: 117K · Output: 10K · Cached: 2M Review guidance: REVIEW.md from base branch |
A terminal rejection (FORBIDDEN, BAD_REQUEST, PRECONDITION_FAILED) maps to a fallback sentence that told the reader to try again while the sync or dismiss controls were disabled. Give the non-retryable fallback its own catalog sentence that names the fix-first condition instead. Also store the dismissal failure's retry class on the durable draft, so the dashboard retry card keeps the Retry affordance decision even when the stored failure sentence came from a different app language.
The durable dismiss draft stored the failure sentence in the language that recorded it, so the dashboard retry card kept the old language after an app-language change. Store the failure's catalog key beside the sentence (null when the mapper does not recognise it), and re-translate through that key wherever the draft renders. Drafts recorded before the field existed read back with the schema's default, so the retry card keeps showing their stored sentence.
isSecurityDismissDraft filled the new field's default by mutating the parsed value inside a narrowing predicate. Drop the mutation and the no-op parseKnownDraft helper: the guard only narrows, and the consumers that read the failure key default a missing field to null, which is exactly what a legacy draft looks like.
iscekic
added
merge:human
pandemicsyn
approved these changes
Oct 20, 2026
Collaborator
Author
|
bot: Re-delivered reports for the same surface (Arabic overview banner, pt-BR headline, Hindi dismissal failure and retry card) are covered by the mapping already on this branch at head
|
iscekic
enabled auto-merge
October 21, 2026 15:24
…ty-sync-errors # Conflicts: # apps/mobile/src/i18n/locales/sv.json
iscekic
disabled auto-merge
October 21, 2026 17:18
iscekic
removed
the
merge:bot
iscekic
added
the
merge:bot
This was referenced Oct 22, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Change
The security operation-ledger markers the server raises for sync and dismissal failures were rendered verbatim: under
jathe dashboard banner showed the raw English sentence above an otherwise translated screen.Mapping now happens once at the mutation boundary, the same way
pr-operation-ledger.tshandles the PR-review surface:apps/mobile/src/lib/security-agent-ledger.tsowns the raw marker constants, the retryability policy, and the two surface mappers.use-security-agent-mutations.tsanduse-security-findings.tsimport from it.securityAgent.mutations.ambiguous,.settleFailed,.replayFailed,common.actionNotRecorded), the in-progress duplicate keeps the per-surface copy, and any other message reads one localized generic sentence (securityAgent.mutations.actionFailed) — the server's English reasons cannot be translated on the device.causeand carries the retryability flag computed from the raw error, so the key-rotation and disabled-control contracts are unchanged after the message is replaced.prReview.operation.persistenceFailedtocommon.actionNotRecorded; the four PR-review call sites point at it.dashboard-screen.tsxanddismiss-finding-screen.tsxdrop the now-redundant missing-configuration special case: the mapped message is already localized.check:i18nandcheck:pluralspass.Verification
pnpm --dir apps/mobile typecheck,lint,format:check,check:unused: clean;check:i18n/check:plurals: 87 catalogs match; full mobile suite: 1190 files, 17871 tests passed.dashboard-screentest: underja, an ambiguous sync renders the JapanesesecurityAgent.mutations.ambiguouscopy, the English marker is gone, and both sync controls stay retryable.SECURITY_SYNC_WORKER_URLunreachable so the Worker submission is ambiguous (server confirmed returning the CONFLICT marker); signed in through the real device-auth flow, seeded the account an active GitHub integration and an enabled security-agent config, switched the app language to Japanese, and tapped 今すぐ同期. Baseline bundle (fix stashed) showed the English marker; the fixed bundle shows 「操作の結果を確認できませんでした。再試行する前にセキュリティレビューをご確認ください。」with the reconcile card in Japanese and both controls still enabled. Screenshots: ja-ambiguous-banner-fixed.png, ja-ambiguous-banner-fixed-repro.png (after restore), en-ambiguous-banner.png (English locale), ja-ambiguous-banner-baseline.png (fix stashed, English marker visible).Merge with main
The branch now merges
main. The only conflict wassv.json: the new security keys are kept, andsecurityAgent.mutations.dismissalInProgresstakes main's corrected Swedish wording. After the merge,check:i18nreports all 87 catalogs matchingen.json,check:pluralspasses, and the mobile typecheck is clean.Proxy answers
The owner's proxy gave these answers in the owner's place.
Answer: All sync/dismiss surfaces
Confidence: 0.85 · Precedent: Fix the root cause and the class: session 7f3470e0ba6f, "fix class of issue preventing progress"; i18n rule, session dcb3f9170164, "the mobile app always must respect i18n"
Answer: Generic localized copy
Confidence: 0.55 · Precedent: Localize every string in the mobile app: session dcb3f9170164, "the mobile app always must respect i18n". The brief also asks for a localized generic error for unknown messages.
Answer: Move it to common.*
Confidence: 0.50 · Precedent: Fix the class rather than the symptom: session 00eb2a1d3e4b, "fix the class". This follows the AGENTS.md convention for copy shared across sections.
Answer: Include it
Confidence: 0.85 · Precedent: If you see a gap, fix it: session 89824a222897, "if you saw a gap, fix it"
Answer: Could not complete this action. Please try again.
Confidence: 0.60 · Precedent: UX and design quality: make every surface pleasant and consistent (session a247f3c19b1e, owner asked to remove redundant copy); a neutral, actionable key fits both surfaces
Answer: Localize it in the mapping
Confidence: 0.80 · Precedent: Localize every string in the mobile app (session dcb3f9170164: "the mobile app always must respect i18n"); fix the class, not the symptom (session 7f3470e0ba6f: "fix class of issue")
Answer: Generic for every unmapped error
Confidence: 0.50 · Precedent: Do the least work possible with no drop in quality (session 7f3470e0ba6f: "do the least amount of work possible, with no drop in quality"); retryability stays intact, so the simpler mapper is enough
Answer: Push an empty non-merge commit to request the review
Confidence: 0.72 · Precedent: Never accept 'blocked' or 'failed' as an end state — session 2f71af3a787e: "a work item must converge"; the pr-wait-jit skill documents this remedy. Fixing the class (merge-commit heads should not need a review, or should trigger one) is a separate workflow fix.