Repository navigation
fix: explain dynamic app contract fallbacks without exception text - #497
veryCrunchy wants to merge 13 commits into
Conversation
Classify App Store contract acquisition failures in the contract module and translate them once in shared JVM code for Android and desktop. A runtime linkage failure, such as a static initializer the Android regex engine rejects, now becomes a client-fault fallback instead of escaping with a class name as its message. Record a typed fallback reason on metadata-only discoveries and derive the fallback card copy from it, so missing contracts, unconfirmed versions, unreachable or unavailable App Store releases, rejected packages, and unsupported contracts each explain what happened. Extend the brace regex architecture check to Android-only source sets and initialize every contract module facade in the Android instrumented test.
Obiente previewNC Native · Obiente updates this comment as the preview changes. |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d1e0f3d646
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Return cancellation unchanged from the package verification wrapper so the acquisition boundary still recognizes it as control flow. Publish a downloaded App Store catalog to the local cache on a best effort basis, matching verified-contract caching. A read-only cache directory or a full disk no longer fails an acquisition that already has the catalog, and is never reported as an unreachable App Store.
Discover androidApp source sets instead of listing only main, so debug, release, directApk, and instrumented test sources are checked too. JVM unit test sources stay excluded because they never run on ICU.
…ontract-acquisition # Conflicts: # tools/kotlin-file-size-baseline.txt
Signed-off-by: veryCrunchy <me@verycrunchy.dev>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9eb8dcf9d1
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…ontract-acquisition # Conflicts: # tools/kotlin-file-size-baseline.txt # website/public/screenshots/capture-manifest.json
A completed acquisition without a contract no longer collapses to null. The contract module reports whether no release matches the installed version, the linked source refused or rate limited the last exact-tag fallback, or the inspected releases provide no usable contract. The shared JVM boundary carries that reason to discovery, and the fallback card derives its copy and action from it.
List the new catalog cache, trust verifier, and YAML parser facades in the Android initializer test, and exercise the real catalog path with a failing cache and YAML parsing on the Android runtime. A JVM unit test loads every packaged facade and fails when one is missing from that list.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 830f783b4e
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ).filterNot { candidate -> candidate.version == exactRelease?.version } | ||
| val releases = listOfNotNull(exactRelease) + compatibleReleases | ||
| if (releases.isEmpty()) return null | ||
| if (releases.isEmpty()) return ContractAcquisitionOutcome.Unavailable(ContractUnavailableReason.NoMatchingRelease) |
There was a problem hiding this comment.
When the App Store contains this app/release but omits a required field such as certificate, download, signature, or signatureDigest, parseReleases silently drops the entry and this branch reports NoMatchingRelease. The new UI then incorrectly states that the App Store lists no signed release, although the catalog was malformed or unsupported; return a typed malformed-source failure rather than conflating that case with genuine absence.
AGENTS.md reference: AGENTS.md:L294-L298
Useful? React with 👍 / 👎.
| } | ||
| val acquired = when (acquisition) { | ||
| is AppStoreContractAcquisition.Acquired -> acquisition.contract | ||
| is AppStoreContractAcquisition.Unavailable -> return sameOrigin.withUnavailable(acquisition.reason, contractAppId, coreVersion) |
There was a problem hiding this comment.
When the same-origin OCS catalog/spec request fails with a network error or 5xx and the App Store package independently yields NoUsableContract, this maps only the App Store outcome and shows the permanent-sounding claim that neither source provides an API. The server probe was inconclusive, not absent, so its typed failure must participate in the fallback reason instead of being reduced to diagnostics that the notice ignores.
AGENTS.md reference: AGENTS.md:L482-L484
Useful? React with 👍 / 👎.
This branch was successfully deployed
1 participant
Outcome
Advances #493 and #473.
On Android, every dynamic app (Forms, Collectives, News, Tables, and others) showed "No verified native API" with a raw class name such as
AppOwnedOpenApiContractKt. The reported nightlies contained a top-level regex with an unescaped}inAppOwnedOpenApiContract.kt. Desktop Java accepts it, but Android's ICU engine rejects it, so the class initializer failed. The first attempt threwExceptionInInitializerErrorand every retry threwNoClassDefFoundError, which is why "Retry discovery" changed the message in #473. Commit e4fac17, already onmain, replaced that regex. This PR removes the remaining ways exception text reaches the user and gives each fallback a clear explanation.JvmAppStoreContractAcquisition.kt) maps acquired contracts once for Android and desktop, replacing duplicated mapping in both services. It catchesLinkageError, so class-load failures no longer escape to the app info screen. Cancellation and other errors still propagate.ui/src/androidMainand everyandroidApp/src/*source set except JVM unit tests (main,debug,release,directApk,androidTest), with self-test fixtures for each.uiAndroid target now depends on:contractAcquisition.DYNAMIC_APP_DESCRIPTOR.mdandPLATFORMS.mdare updated.Audit: all regex literals in Android-bound contract code were checked for ICU-incompatible syntax. The org.json, YAML, XML and
java.nioAPIs the module uses are available at minSdk 26. Nothing else was found.On a fixed build, Tables 1.0.10 gets its contract from its app-owned
openapi.json. News 28.7.0 has no OpenAPI file and gets verified read routes from its upstream routes and controllers. No endpoints were invented.Verification
bash tools/check-repository.shpasseschanges/unreleased/fragments record the changeRun on Windows with JDK 21:
:contractAcquisition:test: 91 tests pass, 5 skipped.:ui:desktopTest(DynamicContractFallbackDiscovery,JvmAppStoreContractAcquisition,DynamicNativeRuntime,DynamicDiscoveryDiagnostics,DynamicDiscoveryPersistence): 106 tests pass.:androidApp:testDebugUnitTest :androidApp:compileDebugAndroidTestKotlin: pass.:ui:desktopTest: 3780 tests, 1 failure inNoteEditorContentTest.shortEditorKeepsSaveVisibleAndDetailsPreserveDraftAndPreview. It does not touch changed code, passed in a later run on this branch, and passes onmain; treated as a flake but not proven.bash tools/check-kotlin-architecture.sh,bash tools/test-kotlin-architecture.sh,node tools/changelog-fragments.mjs validate: pass.New tests:
ContractAcquisitionFailureTest(12, including cancellation during verification and catalog cache write failures),JvmAppStoreContractAcquisitionTest(4),DynamicContractFallbackDiscoveryTest(6), and a new method inAppOwnedOpenApiAndroidInstrumentedTestthat initializes every top-level class in the contract module on Android.Not run: instrumented tests on an emulator or device (
AppOwnedOpenApiAndroidInstrumentedTest, andPublicBudgetContractInstrumentedTestwith-e publicBudgetProbe true),:androidApp:verifyReleaseLintGate(worth running because of the new dependency),:androidApp:assembleDebug,:ui:createDistributable.Compatibility and risk
PLATFORMS.mdkeeps its previous review date because the full inventory was not re-reviewed.Visual changes
The fallback card copy changes per reason. Screenshots for phone, tablet and desktop are not yet captured.