morluto/rea

Fork 0

mirror of https://github.com/morluto/rea.git synced 2026-11-05 06:45:22 +00:00

[PR #2738] [MERGED] fix(apktool): read the resource locale grammar from its owner #5354

New issue

Closed

opened 2026-10-31 01:05:58 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 01:05:58 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/2738
Author: @XiaoBinGan
Created: 10/13/2026
Status: ✅ Merged
Merged: 10/13/2026
Merged by: @morluto

Base: main ← Head: fix/apktool-locale-owner

📝 Commits (1)

3fab001 fix(apktool): read the resource locale grammar from its owner

📊 Changes

2 files changed (+106 additions, -14 deletions)

View changed files

📝 src/cli/apktoolCommands.ts (+14 -14)
➕ tests/boundary/cli/apktoolLocaleOption.test.ts (+92 -0)

📄 Description

Summary

decode-android-resources restated the locale grammar with a pattern narrower than the apktool input owner, so it refused --locale b+zh+Hans+CN — the exact form the owner's own rejection message advertises — before the decoding lane, which accepts that locale, could run. Read the grammar and the string-projection default from the owning schema instead.

Problem and expected behavior

The command validated its --locale option with its own copy of the locale grammar:

	locale grammar
owner — `apktoolInputSchemas.decode_android_resources`	`/^(?:[a-z]{2,3}(?:-(?:r?[A-Z]{2}\|r?[0-9]{3}))?\|b\+[a-z]{2,3}(?:\+[A-Za-z0-9]{2,8})*)$/u`
CLI copy	`/^[a-z]{2,3}(-[A-Za-z0-9]{2,8})*$/u`

The CLI copy cannot match the BCP 47 extension syntax, so b+zh+Hans+CN fails option parsing with exit code 1. The owner accepts that locale, and its own rejection message names it as a supported example — "Locale must look like de, pt-BR, pt-rBR, or b+zh+Hans+CN" — so the CLI refuses a caller-selected input that the lane it dispatches to is prepared to honor. The MCP path passes the owner schema and accepts the same locale, so the two adapters disagreed about one caller choice.

Expected: the CLI admits exactly the locales the owner admits, and the caller's locale reaches the decoding lane unchanged. The string-projection default is the owner's fact too, so it is read from that schema rather than restated as true.

Change and scope

src/cli/apktoolCommands.ts now binds locale to apktoolInputSchemas.decode_android_resources.shape.locale and derives the strings default from the owner's include_strings default, following the field reuse the PE resource and web module commands already use for their shared schemas. The duplicate pattern, its narrower message, and the restated default are removed.

tests/boundary/cli/apktoolLocaleOption.test.ts drives the public CLI boundary with a stub provider: one case forwards every locale the owner admits, and a control case pins that a genuinely malformed locale is still refused. The case list uses the owner's advertised forms, so a future divergence fails the test rather than silently narrowing the CLI again.

No provider behavior changes: the option name, the --no-strings negation, and the forwarded include_strings value are unchanged.

Contract and boundary impact

The CLI option contract widens to the lane's real grammar. One previously refused input is now accepted, and its value is forwarded rather than replaced. Nothing that was accepted becomes refused: the owner's grammar is a strict superset of the copy it replaces for the inputs the copy admitted. The option's help text now carries the owner's description, which names the res/values-<locale>/strings.xml table, so the CLI no longer states a second version of the same meaning.

The default of --strings is unchanged (true), now sourced from the schema instead of a literal.

Evidence and regression coverage

npm run test:focused -- tests/boundary/cli/apktoolLocaleOption.test.ts:

State	Result
with the change	2 passed
`src/cli/apktoolCommands.ts` restored to `main`	1 failed, 1 passed — `exit code for locale b+zh+Hans+CN: expected 1 to be +0`

The failure is the reported defect itself: option parsing exits 1 for a locale the owner admits.

The sibling option coverage is unchanged — tests/boundary/cli/apktoolBooleanOptions.test.ts passes 3/3, including --no-strings dispatch and the omitted-flag default.

npm run check:fast passed on macOS 26.5.1 arm64 with the pinned Node 24.18.0.

Unavailable coverage

Decoding itself was not exercised against a real apktool installation, which is not available on this host; this change only affects which locales the CLI admits and what it forwards, and the boundary test asserts the forwarded request. The MCP path is not re-verified here beyond sharing the same owner schema, which is what makes the two adapters agree by construction.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/morluto/rea/pull/2738 **Author:** [@XiaoBinGan](https://github.com/XiaoBinGan) **Created:** 10/13/2026 **Status:** ✅ Merged **Merged:** 10/13/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `fix/apktool-locale-owner` --- ### 📝 Commits (1) - [`3fab001`](https://github.com/morluto/rea/commit/3fab001db32874f342cae1c42c0107a19edbf960) fix(apktool): read the resource locale grammar from its owner ### 📊 Changes **2 files changed** (+106 additions, -14 deletions) <details> <summary>View changed files</summary> 📝 `src/cli/apktoolCommands.ts` (+14 -14) ➕ `tests/boundary/cli/apktoolLocaleOption.test.ts` (+92 -0) </details> ### 📄 Description ## Summary `decode-android-resources` restated the locale grammar with a pattern narrower than the apktool input owner, so it refused `--locale b+zh+Hans+CN` — the exact form the owner's own rejection message advertises — before the decoding lane, which accepts that locale, could run. Read the grammar and the string-projection default from the owning schema instead. ## Problem and expected behavior The command validated its `--locale` option with its own copy of the locale grammar: | | locale grammar | | --- | --- | | owner — `apktoolInputSchemas.decode_android_resources` | `/^(?:[a-z]{2,3}(?:-(?:r?[A-Z]{2}\|r?[0-9]{3}))?\|b\+[a-z]{2,3}(?:\+[A-Za-z0-9]{2,8})*)$/u` | | CLI copy | `/^[a-z]{2,3}(-[A-Za-z0-9]{2,8})*$/u` | The CLI copy cannot match the BCP 47 extension syntax, so `b+zh+Hans+CN` fails option parsing with exit code 1. The owner accepts that locale, and its own rejection message names it as a supported example — "Locale must look like de, pt-BR, pt-rBR, or b+zh+Hans+CN" — so the CLI refuses a caller-selected input that the lane it dispatches to is prepared to honor. The MCP path passes the owner schema and accepts the same locale, so the two adapters disagreed about one caller choice. Expected: the CLI admits exactly the locales the owner admits, and the caller's locale reaches the decoding lane unchanged. The string-projection default is the owner's fact too, so it is read from that schema rather than restated as `true`. ## Change and scope `src/cli/apktoolCommands.ts` now binds `locale` to `apktoolInputSchemas.decode_android_resources.shape.locale` and derives the `strings` default from the owner's `include_strings` default, following the field reuse the PE resource and web module commands already use for their shared schemas. The duplicate pattern, its narrower message, and the restated default are removed. `tests/boundary/cli/apktoolLocaleOption.test.ts` drives the public CLI boundary with a stub provider: one case forwards every locale the owner admits, and a control case pins that a genuinely malformed locale is still refused. The case list uses the owner's advertised forms, so a future divergence fails the test rather than silently narrowing the CLI again. No provider behavior changes: the option name, the `--no-strings` negation, and the forwarded `include_strings` value are unchanged. ## Contract and boundary impact The CLI option contract widens to the lane's real grammar. One previously refused input is now accepted, and its value is forwarded rather than replaced. Nothing that was accepted becomes refused: the owner's grammar is a strict superset of the copy it replaces for the inputs the copy admitted. The option's help text now carries the owner's description, which names the `res/values-<locale>/strings.xml` table, so the CLI no longer states a second version of the same meaning. The default of `--strings` is unchanged (`true`), now sourced from the schema instead of a literal. ## Evidence and regression coverage `npm run test:focused -- tests/boundary/cli/apktoolLocaleOption.test.ts`: | State | Result | | --- | --- | | with the change | 2 passed | | `src/cli/apktoolCommands.ts` restored to `main` | 1 failed, 1 passed — `exit code for locale b+zh+Hans+CN: expected 1 to be +0` | The failure is the reported defect itself: option parsing exits 1 for a locale the owner admits. The sibling option coverage is unchanged — `tests/boundary/cli/apktoolBooleanOptions.test.ts` passes 3/3, including `--no-strings` dispatch and the omitted-flag default. `npm run check:fast` passed on macOS 26.5.1 arm64 with the pinned Node 24.18.0. ## Unavailable coverage Decoding itself was not exercised against a real apktool installation, which is not available on this host; this change only affects which locales the CLI admits and what it forwards, and the boundary test asserts the forwarded request. The MCP path is not re-verified here beyond sharing the same owner schema, which is what makes the two adapters agree by construction. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-31 01:05:58 +00:00

closed this issue
added the
pull-request
label

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

morluto/rea#5354

No description provided.

Rows
Columns