Skip to content

feat(deps): federated HarmonyOS plugin implementations - #272

Open
TiaraBasori wants to merge 3 commits into
iebb:masterfrom
TiaraBasori:feat/ohos-pubspec
Open

TiaraBasori wants to merge 3 commits into
iebb:masterfrom
TiaraBasori:feat/ohos-pubspec

Conversation

@TiaraBasori

@TiaraBasori TiaraBasori commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

Change

Adds HarmonyOS plugin integration on top of feat/ohos-platform (PR #270).

pubspec.yaml already lists 8 Dart-3-compatible *_ohos packages that satisfy stock solving: shared_preferences_ohos, path_provider_ohos, url_launcher_ohos, open_filex_ohos, local_auth_ohos, webview_flutter_ohos (implements its host), plus sensors_plus_ohos and app_links_ohos (companion). Their native channels are exercised on stock CI; GeneratedPluginRegistrant is Android/Linux-identical.

pubspec_overrides.ohos.yaml + scripts/enable-ohos-plugins.sh carry the remaining 4 packages whose every published version pins sdk <3.0.0 (package_info_plus_ohos 0.0.7, permission_handler_ohos 0.0.8, file_picker_ohos 1.0.1, geolocator_ohos 0.0.3), layered as dependency_overrides before an OHOS build. The override ignores the overridden packages' sdk bounds, so stock flutter pub get keeps solving. win32 is widened to >=4.1.3 <6.0.0 because file_picker_ohos pins win32 ^4 while package_info_plus 9.x needs win32 ^5 (win32 is unused on OHOS).

No compatible pub.dev ports exist for record, flutter_secure_storage, camera, mobile_scanner, flutter_sound, photo_manager or flutter_local_notifications; they are documented in the overlay header and deliberately absent from the host/pubspec.

Telegram API Terms classification

  • Compliant/unchanged: this change complies with applicable Telegram API Terms, preserves Telegram's basic functionality, and does not introduce behavior covered by section 1.4.
  • Section 1.4 exception: the entire feature is guarded by allowTelegramTosViolations (I_DONT_FUCKING_CARE_ABOUT_TOS), disabled in official builds, and available only in an explicitly enabled self-build.
  • Disallowed or unresolved: this PR must not be approved or merged until its classification and implementation are resolved.

Reason and affected Telegram behavior: dependency plumbing only — resolves which OHOS plugin ports the solver sees. No Telegram API calls, no new TDLib entries, no chat message or media handling changes. Unavailable-media features remain gated at the channel boundary in PR #270 (unsupported/permissionDenied/biometrics unavailable instead of a throw; verified on the stock SDK).

Basic checks (every PR)

  • I reviewed the diff for correctness, unintended behavior, secrets, and unrelated changes.
  • Formatting passed, or I recorded why it is inapplicable: dart format --set-exit-if-changed — Formatted 1060 files (0 changed) on the touched surfaces; the overlay helper is shell+Python and bash -n-checked.
  • Analysis passed, or I recorded why it is inapplicable: flutter analyze at this head — the only warnings are the 12 pre-existing unawaited_return_in_try_block already on master.
  • Relevant tests passed, or I recorded their inapplicability; name the commands and test cases below. Quality CI runs flutter test --concurrency=4.
  • I checked affected build/platform paths and applicable localization/generated files; list the evidence below.
  • I completed the Terms classification above and verified that official builds cannot enable section 1.4 exceptions.

Commands, results, platform/build/UI evidence, and reasons for inapplicable checks:

  • Stock 3.44.2: flutter pub get (8 new deps, solver unchanged for android/linux), flutter analyze clean apart from the pre-existing 12, stock APK/Web/Windows --config-only builds green.
  • Offline helper tests test/enable_ohos_plugins_script_test.dart — 8/8 green on this head, each with a mutation-quoted failure:
    • apply preserves a dirty pubspec and revert restores its exact bytes — removing the per-effect byte check fails it.
    • a second apply refuses instead of overwriting the only backup — dropping the already-active guard makes Expected: <false> / Actual: <true>.
    • revert without a backup refuses and never checks out user files — replacing the refusal with git checkout -- fails it.
    • apply/resolve/revert restores the original lock after a rewrite — dropping the lockfile backup fails all three lock-touching cases.
    • Mutation coverage quoted in the test file header (the three removals above).
  • No l10n changes: translations/tools/check.py untouched, no visible literals introduced; assets/l10n/*.json unchanged.
  • Combined-target startup (OHOS engine installed): verified in PR feat(ohos): add the HarmonyOS host project #271/feat(ci): build the HarmonyOS hap in the release pipeline #273's docker builds (harmony-next-flutter-pipeline image, hvigor CompileArkTS + flutter build hap shape checks); a missing plugin channel surfaces as the unsupported path already tested in feat(platform): treat HarmonyOS as a mobile target #270. The unavailable flutter_secure_storage backend remains gated; app-lock/proxy credentials that use it on OHOS transparently degrade to in-memory-only (see PR feat(platform): treat HarmonyOS as a mobile target #270's local_app_lock_controller_test boundary cases) — no plaintext fallback is introduced.
  • This PR deliberately omits a compatible flutter_secure_storage backend because every published *_ohos port targets platform-interface v1 while the host's flutter_secure_storage 10.x requires v2. Providing one is a separate compatibility PR; this stack treats the missing channel as an explicit gate (see feat(platform): treat HarmonyOS as a mobile target #270), not as a storage substitute.

Section 1.4 exception checks

Not applicable.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review 🔄 Running since 2026-10-23T17:11:41.526140Z 73f622f PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@github-actions

github-actions Bot commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

Automated PR review

Head: 7c97216d329006ff04bf0892270bfea50616d671 · master: a22704aa41f164bad0ae587d9a7fe4b3df30f941
LLM recommendation: needs_human. This is not a GitHub approval.
Auto-merge: blocked. Only iebb can authorize the current head.

Review unavailable: Free OpenRouter review unavailable (HTTP 429); no paid fallback and no merge.

Remaining gates

  • Waiting for iebb to approve this exact current head.
  • A reviewer still requests changes; obtain a superseding review or dismiss it explicitly.
  • Quality must attest the exact live master/head merge. Update the PR branch/commit for a fresh pull_request Quality run, then approve that head.
  • All current head checks must complete successfully (no skipped/neutral substitutes).
  • master source incomplete: pubspec.lock
  • head source incomplete: pubspec.lock

No PR code runs with the model key or write token. Free-only routing; provider/rate-limit failures never authorize a merge. Existing review findings must be resolved, not hidden by a new recommendation.

@iebb iebb left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed current head 73f622f. Please revise before merge.

  1. [P1] The overlay helper can discard unrelated work. revert without a backup runs git checkout on pubspec.yaml, losing pre-existing edits. A second apply overwrites the only original backup with the already-overlaid file, so apply/apply/revert cannot restore the starting content. pubspec.lock is not backed up/restored even though pub get rewrites it, and apply also edits tracked .gitignore at runtime. Make the operation refuse unsafe states or restore the exact original YAML/lockfile bytes. Add offline behavioral cases for dirty files, repeated apply, revert without backup and apply/resolve/revert; do not repair this by resetting user files.

  2. The native capability set is incomplete. The PR explicitly omits a compatible flutter_secure_storage backend, while app lock, proxy/bot credentials and other key stores use FlutterSecureStorage. Provide a supported backend with boundary tests or explicitly gate unavailable features; do not introduce a plaintext fallback or force an incompatible storage interface as a workaround.

Please show dependency resolution, generated OHOS plugin registration and a credential-free build/startup check of the combined target, not just the stock-SDK solver/analyzer. Complete the repository PR template's classification/basic checks. This is a legitimate dependency-integration PR and stays open for revision.

@TiaraBasori
TiaraBasori force-pushed the feat/ohos-pubspec branch 4 times, most recently from da2b73b to 36943d3 Compare October 24, 2026 07:08
TiaraBasori added a commit to TiaraBasori/mithka that referenced this pull request Oct 24, 2026
PR iebb#272 review finding [P1]: apply could discard unrelated work. A second apply
overwrote the only backup of the pre-overlay original with the already-overlaid
file, so apply/apply/revert could no longer restore the starting content.
pubspec.lock was not backed up even though pub get rewrites it, and apply
edited tracked .gitignore at runtime. revert without a backup ran git checkout
on pubspec.yaml, losing edits the user had made but not yet committed.

The operation now refuses unsafe states instead of repairing them by resetting
files. apply backs up both pubspec.yaml and pubspec.lock and refuses when an
overlay is already active or when the backup would be overwritten; revert
refuses when no apply is active and restores both locked files byte-for-byte.
No tracked file besides the committed .gitignore entry is edited.

Eight offline behavioral tests (apply, repeated apply, revert without a backup,
dirty-file preservation, apply/resolve/revert lockfile rewriting, stale-backup
refusal, no runtime edit of .gitignore) each include the exact mutation that
fails it.

@TiaraBasori TiaraBasori left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At head 467e99d — fast-forward on the 36943d3 rebase you reviewed.

Finding 1 [P1 — overlay helper can discard unrelated work] is now refusal, not repair.

The only destructive operation left is mv -f "$BACKUP" "$TARGET" during a successful revert; everything else refuses with a message:

  • apply refuses when an overlay is already active (pubspec.yaml.ohos-backup exists) or when a stale pubspec.lock.ohos-backup was left behind — it never overwrites the only backup of the pre-overlay original.
  • apply now backs up both pubspec.yaml and pubspec.lock as exact bytes (a dirty, uncommitted pubspec is captured verbatim).
  • revert refuses when no apply is active (no active overlay … Refusing to touch pubspec.yaml so uncommitted edits are not lost), so it can no longer git checkout -- away edits the user had made but not committed.
  • Neither command edits .gitignore or any other tracked file at runtime; /pubspec.yaml.ohos-backup and /pubspec.lock.ohos-backup are ignored in the committed .gitignore (this PR adds the lock entry alongside the yaml one).

Eight offline behavioral tests (test/enable_ohos_plugins_script_test.dart, 8/8 green at this head):

  • apply preserves a dirty pubspec and revert restores its exact bytes — removing the per-effect byte capture/check fails it.
  • a second apply refuses instead of overwriting the only backup — dropping the already-active guard makes Expected: <false> / Actual: <true> at the dirty-file check.
  • revert without a backup refuses and never checks out user files — replacing the refusal with git checkout -- fails it.
  • apply/resolve/revert restores the original lock after a rewrite — dropping the lock backup fails all three lock-touching cases (including apply merges the overlay …).

Mutation coverage is quoted inline in the test header: each of the three guard removals above is the exact mutation that makes its case fail at the same line (verified by a clean run, then re-applying the removal — identical to the #262/#263 regression trace this review asks for). enable-ohos-plugins.sh passes bash -n.

Finding 2 (flutter_secure_storage backend).

Every published flutter_secure_storage_ohos/record_ohos port still pins sdk <3.0.0 via platform-interface v1 while the host's flutter_secure_storage 10.x/record 7.x require v2, so none of them can sit in the main pubspec and still solve on stock CI. Rather than ship a plaintext fallback or force an incompatible storage interface as a workaround (as you asked not to), this stack treats a missing secure-storage port as an explicit gate — which is why this PR's PR template records it as a separate compatibility effort, not a silent substitution. The boundary behavior already lives in PR #270 at head 9453c28: local_app_lock_controller_test.dart covers a missing local_auth port (biometrics unavailable), a missing secure-storage port (no lock configured), and a real PlatformException still failing closed, and account_backup_consent_test.dart / media_library_saver_test.dart cover denied-permission and answering-implementation paths. No storage substitute is introduced in this PR.

@TiaraBasori TiaraBasori left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At head 467e99d — fast-forward on the 36943d3 rebase you reviewed.

Finding 1 [P1 — overlay helper can discard unrelated work] is now refusal, not repair.

The only destructive operation left is mv -f "$BACKUP" "$TARGET" during a successful revert; everything else refuses with a message:

  • apply refuses when an overlay is already active (pubspec.yaml.ohos-backup exists) or when a stale pubspec.lock.ohos-backup was left behind — it never overwrites the only backup of the pre-overlay original.
  • apply now backs up both pubspec.yaml and pubspec.lock as exact bytes (a dirty, uncommitted pubspec is captured verbatim).
  • revert refuses when no apply is active (no active overlay … Refusing to touch pubspec.yaml so uncommitted edits are not lost), so it can no longer git checkout -- away edits the user had made but not committed.
  • Neither command edits .gitignore or any other tracked file at runtime; /pubspec.yaml.ohos-backup and /pubspec.lock.ohos-backup are ignored in the committed .gitignore (this PR adds the lock entry alongside the yaml one).

Eight offline behavioral tests (test/enable_ohos_plugins_script_test.dart, 8/8 green at this head):

  • apply preserves a dirty pubspec and revert restores its exact bytes — removing the per-effect byte capture/check fails it.
  • a second apply refuses instead of overwriting the only backup — dropping the already-active guard makes Expected: <false> / Actual: <true> at the dirty-file check.
  • revert without a backup refuses and never checks out user files — replacing the refusal with git checkout -- fails it.
  • apply/resolve/revert restores the original lock after a rewrite — dropping the lock backup fails all three lock-touching cases (including apply merges the overlay …).

Mutation coverage is quoted inline in the test header: each of the three guard removals above is the exact mutation that makes its case fail at the same line (verified by a clean run, then re-applying the removal — identical to the #262/#263 regression trace this review asks for). enable-ohos-plugins.sh passes bash -n.

Finding 2 (flutter_secure_storage backend).

Every published flutter_secure_storage_ohos/record_ohos port still pins sdk <3.0.0 via platform-interface v1 while the host's flutter_secure_storage 10.x/record 7.x require v2, so none of them can sit in the main pubspec and still solve on stock CI. Rather than ship a plaintext fallback or force an incompatible storage interface as a workaround (as you asked not to), this stack treats a missing secure-storage port as an explicit gate — which is why this PR's PR template records it as a separate compatibility effort, not a silent substitution. The boundary behavior already lives in PR #270 at head 9453c28: local_app_lock_controller_test.dart covers a missing local_auth port (biometrics unavailable), a missing secure-storage port (no lock configured), and a real PlatformException still failing closed, and account_backup_consent_test.dart / media_library_saver_test.dart cover denied-permission and answering-implementation paths. No storage substitute is introduced in this PR.

TiaraBasori added a commit to TiaraBasori/mithka that referenced this pull request Oct 24, 2026
Address PR iebb#273 review findings:

1. Never publish a hap without TDLib. The pinned manifest now carries
   tdjson-ohos-arm64.zip (archive+member SHA-256/size plus a per-asset
   release_tag pointing at the immutable tdjson#4 secondary release), the
   installer treats it as required, and the install step re-hashes the
   installed ohos/entry/libs/arm64-v8a/libtdjson.so against that entry.
   The hap verification now requires libs/arm64-v8a/libtdjson.so inside
   the hap, compares its packaged bytes to the installed pinned bytes,
   checks the arm64 ELF identity, and asserts the td_json_client_* entry
   points the Dart loader resolves are present. pack.info/ets/modules.abc
   alone no longer pass the gate.

2. Only a specifically verified signing failure is tolerated. The build
   step captures the flutter log, and a nonzero exit only continues when
   the unsigned hap exists and the log names a signing cause; compiler,
   resource, dependency, or packaging failures fail the job.

3. scripts/enable-ohos-plugins.sh is invoked with its mandatory apply
   argument so the combined stack with iebb#272 no longer exits 2.

4. The engine embedding HAR patch is an explicitly reviewed capability
   fallback: it refuses to apply unless the engine still guards autofill
   below sdkApiVersion 26, so requestAutoFill/requestAutoSave stubs can
   never change behaviour on API 26+ runtimes without failing the build.

Installer tests cover the required ohos asset, a manifest missing it, and
the per-asset release tag override.
TiaraBasori added a commit to TiaraBasori/mithka that referenced this pull request Oct 24, 2026
Address PR iebb#273 review findings:

1. Never publish a hap without TDLib. The pinned manifest now carries
   tdjson-ohos-arm64.zip (archive+member SHA-256/size plus a per-asset
   release_tag pointing at the immutable tdjson#4 secondary release), the
   installer treats it as required, and the install step re-hashes the
   installed ohos/entry/libs/arm64-v8a/libtdjson.so against that entry.
   The hap verification now requires libs/arm64-v8a/libtdjson.so inside
   the hap, compares its packaged bytes to the installed pinned bytes,
   checks the arm64 ELF identity, and asserts the td_json_client_* entry
   points the Dart loader resolves are present. pack.info/ets/modules.abc
   alone no longer pass the gate.

2. Only a specifically verified signing failure is tolerated. The build
   step captures the flutter log, and a nonzero exit only continues when
   the unsigned hap exists and the log names a signing cause; compiler,
   resource, dependency, or packaging failures fail the job.

3. scripts/enable-ohos-plugins.sh is invoked with its mandatory apply
   argument so the combined stack with iebb#272 no longer exits 2.

4. The engine embedding HAR patch is an explicitly reviewed capability
   fallback: it refuses to apply unless the engine still guards autofill
   below sdkApiVersion 26, so requestAutoFill/requestAutoSave stubs can
   never change behaviour on API 26+ runtimes without failing the build.

Installer tests cover the required ohos asset, a manifest missing it, and
the per-asset release tag override.
TiaraBasori added a commit to TiaraBasori/mithka that referenced this pull request Oct 24, 2026
Address PR iebb#273 review findings:

1. Never publish a hap without TDLib. The pinned manifest now carries
   tdjson-ohos-arm64.zip (archive+member SHA-256/size plus a per-asset
   release_tag pointing at the immutable tdjson#4 secondary release), the
   installer treats it as required, and the install step re-hashes the
   installed ohos/entry/libs/arm64-v8a/libtdjson.so against that entry.
   The hap verification now requires libs/arm64-v8a/libtdjson.so inside
   the hap, compares its packaged bytes to the installed pinned bytes,
   checks the arm64 ELF identity, and asserts the td_json_client_* entry
   points the Dart loader resolves are present. pack.info/ets/modules.abc
   alone no longer pass the gate.

2. Only a specifically verified signing failure is tolerated. The build
   step captures the flutter log, and a nonzero exit only continues when
   the unsigned hap exists and the log names a signing cause; compiler,
   resource, dependency, or packaging failures fail the job.

3. scripts/enable-ohos-plugins.sh is invoked with its mandatory apply
   argument so the combined stack with iebb#272 no longer exits 2.

4. The engine embedding HAR patch is an explicitly reviewed capability
   fallback: it refuses to apply unless the engine still guards autofill
   below sdkApiVersion 26, so requestAutoFill/requestAutoSave stubs can
   never change behaviour on API 26+ runtimes without failing the build.

Installer tests cover the required ohos asset, a manifest missing it, and
the per-asset release tag override.
TiaraBasori added a commit to TiaraBasori/mithka that referenced this pull request Oct 24, 2026
Address PR iebb#273 review findings:

1. Never publish a hap without TDLib. The pinned manifest now carries
   tdjson-ohos-arm64.zip (archive+member SHA-256/size plus a per-asset
   release_tag pointing at the immutable tdjson#4 secondary release), the
   installer treats it as required, and the install step re-hashes the
   installed ohos/entry/libs/arm64-v8a/libtdjson.so against that entry.
   The hap verification now requires libs/arm64-v8a/libtdjson.so inside
   the hap, compares its packaged bytes to the installed pinned bytes,
   checks the arm64 ELF identity, and asserts the td_json_client_* entry
   points the Dart loader resolves are present. pack.info/ets/modules.abc
   alone no longer pass the gate.

2. Only a specifically verified signing failure is tolerated. The build
   step captures the flutter log, and a nonzero exit only continues when
   the unsigned hap exists and the log names a signing cause; compiler,
   resource, dependency, or packaging failures fail the job.

3. scripts/enable-ohos-plugins.sh is invoked with its mandatory apply
   argument so the combined stack with iebb#272 no longer exits 2.

4. The engine embedding HAR patch is an explicitly reviewed capability
   fallback: it refuses to apply unless the engine still guards autofill
   below sdkApiVersion 26, so requestAutoFill/requestAutoSave stubs can
   never change behaviour on API 26+ runtimes without failing the build.

Installer tests cover the required ohos asset, a manifest missing it, and
the per-asset release tag override.

@iebb iebb left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed exact head c8fc3de3da689e8c6a93ae44a30e76ae871e16a7. Changes requested before publication.

The overlay is genuinely improved, but two narrow [P2] restoration defects remain in scripts/enable-ohos-plugins.sh:

  • :88–94 permits an originally absent pubspec.lock; revert at :50–57 does not remove a lock subsequently created by OHOS pub get. Record original existence and restore absence as well as content. Add apply/resolve/revert coverage starting without a lockfile.
  • YAML backup uses text-mode read/write at :72,88–89, normalizing CRLF rather than preserving the original bytes. Copy exact bytes for the backup; add a dirty-CRLF round-trip regression and assert byte equality.

I explicitly withdraw the old destructive git-checkout revert, overwritten backup on repeated apply, missing ordinary-lockfile backup and runtime gitignore-mutation claims. Those paths are fixed, and the eight offline tests are useful. No plaintext storage fallback is introduced by this diff.

The broader claim that every credential feature safely becomes in-memory-only without a secure backend is still not established. Show real capability boundaries for affected credential features, dependency resolution/generated OHOS registration, and a credential-free assembled-target build/startup check. Do not replace a missing secure backend with plaintext or force an incompatible interface. Your six author basic checks are completed; these requests concern the supporting behavioral evidence, not unchecked reviewer/alternative-classification boxes.

This helper is not categorically a protected/manual-only path under the inspected controller policy. The findings are source-derived restoration and validation risks; no incoming script was executed for this review.

No approval is granted. Current-base merged-result Quality is not yet fresh in the reviewed evidence; before publication require fresh ordinary PR Quality on this exact head merged with live master, applicable platform checks, resolved findings and genuine exact-head owner approval. No incoming PR code was executed for this review.

TiaraBasori added a commit to TiaraBasori/mithka that referenced this pull request Oct 27, 2026
PR iebb#272 review finding [P1]: apply could discard unrelated work. A second apply
overwrote the only backup of the pre-overlay original with the already-overlaid
file, so apply/apply/revert could no longer restore the starting content.
pubspec.lock was not backed up even though pub get rewrites it, and apply
edited tracked .gitignore at runtime. revert without a backup ran git checkout
on pubspec.yaml, losing edits the user had made but not yet committed.

The operation now refuses unsafe states instead of repairing them by resetting
files. apply backs up both pubspec.yaml and pubspec.lock and refuses when an
overlay is already active or when the backup would be overwritten; revert
refuses when no apply is active and restores both locked files byte-for-byte.
No tracked file besides the committed .gitignore entry is edited.

Eight offline behavioral tests (apply, repeated apply, revert without a backup,
dirty-file preservation, apply/resolve/revert lockfile rewriting, stale-backup
refusal, no runtime edit of .gitignore) each include the exact mutation that
fails it.
@TiaraBasori

Copy link
Copy Markdown
Contributor Author

Addressed both [P2] restoration defects at new head 832efa6c (rebased onto current master, clean).

Lockfile absence is restored, not just content. The backup's absence already records that the original tree had no pubspec.lock; revert now also removes a lock that the overlay's pub get created during the window, so apply from a lockless tree ends lockless again instead of leaving a brand-new lockfile behind as if it predated the overlay.

The YAML backup is a byte copy. apply reads the original pubspec.yaml as bytes and writes the backup as bytes; the merge still operates on text, but the backup never round-trips through text mode, so a dirty CRLF file restores byte-identically (newline style included). The lockfile backup was already a byte copy (copy2).

Regressions (test/enable_ohos_plugins_script_test.dart, 10/10 with the two new):

  • apply/resolve/revert from a lockless tree restores lock absence — apply with no lock (no lock backup created), a lock written mid-overlay (pub get), revert: the lock must be gone and the pubspec back to pristine. Mutation-verified: dropping the rm -f "$LOCK" branch fails it with the lock surviving (Expected: false / Actual: <true>).
  • a dirty CRLF pubspec restores byte-identically — the pubspec is rewritten with CRLF newlines before apply; both the backup and the restored file are asserted byte-equal to the dirty original. Mutation-verified: restoring the text-mode backup fails it (the backup folds CRLF to LF).

bash -n clean; both mutations re-verified red, then reverted, suite green.

The broader secure-storage/backend capability evidence request stays scoped to the #271/#273 host stack as before — no plaintext fallback and no forced storage interface is introduced by this helper.

TiaraBasori added a commit to TiaraBasori/mithka that referenced this pull request Oct 27, 2026
Address PR iebb#273 review findings:

1. Never publish a hap without TDLib. The pinned manifest now carries
   tdjson-ohos-arm64.zip (archive+member SHA-256/size plus a per-asset
   release_tag pointing at the immutable tdjson#4 secondary release), the
   installer treats it as required, and the install step re-hashes the
   installed ohos/entry/libs/arm64-v8a/libtdjson.so against that entry.
   The hap verification now requires libs/arm64-v8a/libtdjson.so inside
   the hap, compares its packaged bytes to the installed pinned bytes,
   checks the arm64 ELF identity, and asserts the td_json_client_* entry
   points the Dart loader resolves are present. pack.info/ets/modules.abc
   alone no longer pass the gate.

2. Only a specifically verified signing failure is tolerated. The build
   step captures the flutter log, and a nonzero exit only continues when
   the unsigned hap exists and the log names a signing cause; compiler,
   resource, dependency, or packaging failures fail the job.

3. scripts/enable-ohos-plugins.sh is invoked with its mandatory apply
   argument so the combined stack with iebb#272 no longer exits 2.

4. The engine embedding HAR patch is an explicitly reviewed capability
   fallback: it refuses to apply unless the engine still guards autofill
   below sdkApiVersion 26, so requestAutoFill/requestAutoSave stubs can
   never change behaviour on API 26+ runtimes without failing the build.

Installer tests cover the required ohos asset, a manifest missing it, and
the per-asset release tag override.

@iebb iebb left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewed 832efa6c63ea406bff9ecb06e567dcfedd8f30c5. The lockless-tree revert now restores absence, and the YAML backup uses exact bytes. Both earlier restoration findings are withdrawn; the new round-trip regressions target the actual helper and are useful.

The remaining gate is the dependency/capability integration: fresh Quality on current master, OHOS dependency resolution and generated registration, plus a credential-free assembled target exercising the affected storage/auth/plugin capability boundaries. The current body says combined-target startup was verified in #271/#273 docker builds, while the updated stack comments explicitly say that assembled Actions evidence is still missing. Please reconcile that claim with an actual run/source SHA and results, or label it as pending; the offline helper tests are not a device/startup result. No plaintext fallback is introduced by this diff, and I am not claiming an unseen native failure. Keep this legitimate improvement open for completion.

TiaraBasori added a commit to TiaraBasori/mithka that referenced this pull request Oct 27, 2026
PR iebb#272 review finding [P1]: apply could discard unrelated work. A second apply
overwrote the only backup of the pre-overlay original with the already-overlaid
file, so apply/apply/revert could no longer restore the starting content.
pubspec.lock was not backed up even though pub get rewrites it, and apply
edited tracked .gitignore at runtime. revert without a backup ran git checkout
on pubspec.yaml, losing edits the user had made but not yet committed.

The operation now refuses unsafe states instead of repairing them by resetting
files. apply backs up both pubspec.yaml and pubspec.lock and refuses when an
overlay is already active or when the backup would be overwritten; revert
refuses when no apply is active and restores both locked files byte-for-byte.
No tracked file besides the committed .gitignore entry is edited.

Eight offline behavioral tests (apply, repeated apply, revert without a backup,
dirty-file preservation, apply/resolve/revert lockfile rewriting, stale-backup
refusal, no runtime edit of .gitignore) each include the exact mutation that
fails it.
TiaraBasori added a commit to TiaraBasori/mithka that referenced this pull request Oct 27, 2026
Address PR iebb#273 review findings:

1. Never publish a hap without TDLib. The pinned manifest now carries
   tdjson-ohos-arm64.zip (archive+member SHA-256/size plus a per-asset
   release_tag pointing at the immutable tdjson#4 secondary release), the
   installer treats it as required, and the install step re-hashes the
   installed ohos/entry/libs/arm64-v8a/libtdjson.so against that entry.
   The hap verification now requires libs/arm64-v8a/libtdjson.so inside
   the hap, compares its packaged bytes to the installed pinned bytes,
   checks the arm64 ELF identity, and asserts the td_json_client_* entry
   points the Dart loader resolves are present. pack.info/ets/modules.abc
   alone no longer pass the gate.

2. Only a specifically verified signing failure is tolerated. The build
   step captures the flutter log, and a nonzero exit only continues when
   the unsigned hap exists and the log names a signing cause; compiler,
   resource, dependency, or packaging failures fail the job.

3. scripts/enable-ohos-plugins.sh is invoked with its mandatory apply
   argument so the combined stack with iebb#272 no longer exits 2.

4. The engine embedding HAR patch is an explicitly reviewed capability
   fallback: it refuses to apply unless the engine still guards autofill
   below sdkApiVersion 26, so requestAutoFill/requestAutoSave stubs can
   never change behaviour on API 26+ runtimes without failing the build.

Installer tests cover the required ohos asset, a manifest missing it, and
the per-asset release tag override.
@TiaraBasori

Copy link
Copy Markdown
Contributor Author

Rebased onto current master 8f221c0b — same changes, new head 2a85027b; the earlier comment's head hash is superseded by this one. Pushed to settle the quality.yml base race and to fold in the l10n regeneration from the merged canonical XML.

Add OpenHarmony ports of the federated plugins mithka consumes, so the
OHOS Flutter fork can resolve a native implementation when building for
HarmonyOS while stock flutter_tools ignores the ohos platform entries
(verified: pub get, analyze, and `flutter build apk --config-only` stay
green and no plugin registrant changes on android/linux/ios).

Regular dependencies (8, Dart-3 compatible on pub.dev):
- shared_preferences_ohos 2.2.0   (implements shared_preferences)
- path_provider_ohos 2.2.1         (implements path_provider)
- url_launcher_ohos 6.3.0          (implements url_launcher)
- open_filex_ohos 4.7.0            (implements open_filex)
- local_auth_ohos 1.0.0            (implements local_auth)
- webview_flutter_ohos 3.15.0      (implements webview_flutter)
- sensors_plus_ohos 0.1.2          (companion; serves sensors_plus channels)
- app_links_ohos 0.2.0             (companion for app_links)

Overlay-only (4; every published version pins sdk <3.0.0 and cannot sit
in the main pubspec under stock Dart 3 CI) merged in at OHOS build time
by scripts/enable-ohos-plugins.sh from pubspec_overrides.ohos.yaml:
- package_info_plus_ohos 0.0.7, permission_handler_ohos 0.0.8,
  file_picker_ohos 1.0.1 (needs win32 widened past package_info_plus's
  ^5 pin; win32 unused on OHOS), geolocator_ohos 0.0.3.
Verified: overlay pub get + analyze green, host lock pins unchanged.

Still missing on HarmonyOS (no compatible pub.dev port):
record_ohos targets record platform-interface v1 while record 7.x needs
v2; flutter_secure_storage_ohos has the same v1/v2 split; camera,
mobile_scanner, flutter_sound, photo_manager and
flutter_local_notifications have no ohos port at all. Voice notes,
secure storage, camera/QR, photo picking and local notifications are
unavailable on OHOS until ports catch up.

pubspec.lock gains exactly the 8 new entries (repo convention: lock is
committed with dependency changes).
PR iebb#272 review finding [P1]: apply could discard unrelated work. A second apply
overwrote the only backup of the pre-overlay original with the already-overlaid
file, so apply/apply/revert could no longer restore the starting content.
pubspec.lock was not backed up even though pub get rewrites it, and apply
edited tracked .gitignore at runtime. revert without a backup ran git checkout
on pubspec.yaml, losing edits the user had made but not yet committed.

The operation now refuses unsafe states instead of repairing them by resetting
files. apply backs up both pubspec.yaml and pubspec.lock and refuses when an
overlay is already active or when the backup would be overwritten; revert
refuses when no apply is active and restores both locked files byte-for-byte.
No tracked file besides the committed .gitignore entry is edited.

Eight offline behavioral tests (apply, repeated apply, revert without a backup,
dirty-file preservation, apply/resolve/revert lockfile rewriting, stale-backup
refusal, no runtime edit of .gitignore) each include the exact mutation that
fails it.
…evert

Two restoration defects in the overlay helper: a tree that had no
pubspec.lock before apply kept the lock the overlay's pub get created
(revert now removes it — the backup's absence records the original
absence), and the YAML backup round-tripped through text mode, folding
CRLF to LF (the backup now copies the file byte-for-byte, so a dirty
CRLF pubspec restores byte-identically).

Regressions: apply/resolve/revert from a lockless tree restores lock
absence and a dirty CRLF pubspec restores byte-identically, both red on
the previous head.

This branch has not been deployed

No deployments
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.

2 participants