Device runtime: run pushed Codename One apps on a phone - #5561
Device runtime: run pushed Codename One apps on a phone#5561shai-almog wants to merge 100 commits into
Conversation
ddc43de to
7623680
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ddc43de0d2
ℹ️ 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".
Codename One apps run two ways today: the JavaSE simulator, which is not a device, or a cloud device build, which costs minutes per iteration. This adds a third: install one app on a phone, and from then on push a project to it from an IDE and watch it run natively in seconds. The app is not a shell around a compiled build. Pushed classes are interpreted on the device against the framework already compiled into it, so nothing is built, signed or installed between edits. How the pieces fit ------------------ com.codename1.interp is the interpreter: one interpreted frame per real frame, so Display.invokeAndBlock and every blocking idiom built on it still work. A per-thread fuel counter bounds runaway code, and the budget is per entry into the interpreter rather than per session -- measuring it per session kills every callback that arrives later than the budget, which in an application whose whole life is callbacks is every button press. Interpreted classes reach the framework through InterpLinker: invoke thunks on iOS, reflection on Android. A linker must dispatch on the receiver's class, not the call site's declared type -- list.add(x) names java.util.List, and resolving from there finds AbstractList.add, whose body throws. Extending a framework class needs an object the framework accepts, which neither platform can define at run time. Generated shims provide it: every public, non-final, constructible class and every public interface the device exposes, derived by scanning the framework jar and codenameone-java-runtime rather than curated. A hand-maintained list is a promise that applications only subclass what somebody anticipated, and its failure mode is not an error but an override that is silently never called. Lambdas and method references are rewritten into real classes when the bundle is written, since neither target has a runtime invokedynamic. Enums are answered by the interpreter, java.lang.Enum having no shim and needing none. Store compliance is built in rather than bolted on: the runtime refuses to load a bundle whose sources it cannot show, and shows them. Verified -------- 4798 core tests, 506 translator tests, 43 interpreter tests, SpotBugs at zero, and a 20-program device battery (scripts/devruntime-probes) passing on both an Android emulator and the iOS simulator -- including a four-file, three-package application entered through Lifecycle rather than main. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Listing text in fastlane's layout so supply and deliver can consume it, a privacy statement for both stores' data forms, and a scheduled workflow. The weekly job uploads to Play internal testing and TestFlight. It does not promote to production and does not submit for App Store review, which is a decision rather than an omission: a weekly automatic release would put unread builds in front of the public and queue an iOS review every week whether or not anything changed. Promotion stays one command, taken deliberately. Without publishing credentials the job reports which secrets are missing and stops, rather than publishing half a release. None of them exist yet. The review risk is written down rather than discovered later. This app runs code it did not ship with, which is squarely Guideline 2.5.2 -- permitted for tools that develop or test code, and only while the source stays viewable and editable on the device. That is why the runtime refuses a bundle it cannot show the source for. 4.7.2 is the sharper edge and the argument to make is that this is point to point developer tooling rather than a mini-app platform. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ate every push Three things the review asked for, and two defects found on the way. The interpreter moves from com.codename1.interp to com.codename1.impl.interp. It is an implementation detail of one app, not public API, and the impl hierarchy is what keeps it out of the javadoc. Note the package name is not only a Java name: ParparVM's dead-code pass recognises the runtime's own classes by their C-mangled prefix, so Parser.isLoadBearingForInterp moved with it. Missing that would have stubbed out InterpRuntime.run in an interp-host build, which fails by succeeding -- every pushed program "runs" instantly and executes nothing. The ~1000 generated shims leave git. They are a mechanical function of the framework jar, so the build generates them: a tools module builds the generator, exec-maven-plugin runs it into target/generated-sources/shims, and build-helper adds that as a source root. scripts/generate-interp-shims.sh keeps the three properties the build takes on faith -- every shim compiles, the load-bearing ones exist, generating twice is identical -- and now asserts them against a scratch tree instead of writing into src. Pairing no longer hands out a bearer token. v2 authorised a push with a peer id sent in the clear, so capturing one frame on a LAN meant pushing arbitrary code to somebody's phone forever. v3 derives a 256-bit secret on both ends from the typed code, the peer id and the device id -- never transmitted, 20k HMAC iterations so grinding six digits costs something -- and every connection answers a fresh challenge whose MAC covers the bundle. Authentication happens before the approval prompt, so nobody can raise dialogs on a stranger's phone until they tap Approve to stop them. What this still does not defeat is a passive observer of the pairing exchange itself, which the docs now say plainly. There are two implementations of the derivation, since ParparVM has no javax.crypto; InterpPairingSecretTest runs both and compares. Also fixed: - A class initializer that threw left the class marked initialized, so later reads returned whatever half of it had been assigned. Four states and an owning thread now, per JLS 12.4.2. - Sources were keyed by file name, so two Util.java in different packages collided and the runtime refused the program with "missing the source file Util.java" for a file it had been handed. Keyed by package now. - The iOS release job resolved ExportOptions.plist relative to the generated Xcode project, which is not where it lives. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
7623680 to
c6c6ada
Compare
check-copyright-headers gates every added source file, and the probes and the IDE sample are ours -- not third-party, so the exclusions file (which is for provenance, and rejects anything else) is the wrong place for them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
💡 Codex ReviewOn every Android launch this support check is false because no code installs the newly added When the normal “Push again … to replace it” workflow loads a second bundle, this assignment discards the service's reference to the previous runtime without requesting cancellation or invoking the previous For an already-paired LAN push, If any unrelated service happens to accept this port during the subnet sweep, the callback marks it as found before ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
✅ Continuous Quality ReportTest & Coverage
Static Analysis
Generated automatically by the PR CI workflow. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3118d715e7
ℹ️ 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".
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three were real: a SecureRandom built per call (worse and slower than one seeded once, and what it generates is the pairing code), two Files.createDirectories calls on a getParent() that SpotBugs cannot prove non-null, and an ExecutorService.submit whose Future was never going to be read -- execute() says what the scan actually wants. The other two are recorded in spotbugs-exclude.xml with their reasons: a command-line tool exits, and a failure while enumerating this machine's interfaces must be answered with 'no device found' rather than by killing the push. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
💡 Codex ReviewWhen a second bundle is pushed, this assignment only drops the service's reference to the previous runtime; it neither requests cancellation nor invokes any lifecycle cleanup. Peers, background threads, timers, and framework listeners retain references to the old runtime, so the supposedly replaced application can continue executing and mutate the UI or shared resources while the new application runs. Add a runtime deactivation/cleanup path and call it before publishing the replacement. CodenameOne/CodenameOne/src/com/codename1/impl/interp/InterpRuntime.java Lines 1347 to 1350 in 7e92c49 For a peerless interpreted object, calls inherited from When compiled output contains Kotlin classes, even explicitly passing a Kotlin source directory to Entry-point discovery recognizes only classes whose immediate superclass is ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d4d7ad9bda
ℹ️ 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".
Cloudflare Preview
|
Seven findings, all real. The interpreter: - A class literal for a pushed type puts an InterpClass on the stack, because there is no host class object to hand back -- and then the bytecode goes on calling java.lang.Class methods on it, which no linker can serve. The part of Class that means anything here (naming, identity, isInterface, isInstance, getSuperclass) is answered by the interpreter; anything else is refused by name rather than answered wrongly. - `new Entry[1][]` names its component `[LEntry;`, not `Entry`, so the bundle-membership test missed it and asked the host loader for a class only the bundle has. It looks through the brackets now, and multianewarray builds the nested Object[] itself rather than delegating. - JLS 12.4.1: initializing a class initializes the superinterfaces that declare a default method. Only those -- initializing all of them would run initializers Java never runs, which is as wrong as running them late. The push tool: - The Lifecycle entry point was chosen by direct superclass only, so a project whose app extends its own BaseApp entered BaseApp: an abstract class that was never meant to be instantiated. It walks the hierarchy now and takes the deepest concrete descendant. - A subnet scan treated any host that accepted TCP on the port as the device, and then failed the push against it while the real device sat unqueried. There is a PING frame now; only an answer in our own protocol wins. - cn1-push.sh still spoke v2, which nothing accepts any more. Its paired mode is gone rather than ported: it is a loopback helper, and pushing to a phone over Wi-Fi is DevicePush's job. A third copy of the derivation in a shell script would only drift from the two that have to agree. The release workflow now checks every secret the job will consume, not the two that name the store, so a half-configured store says so in preflight instead of half an hour later in the signing step. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1356d055bd
ℹ️ 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".
CLDC11 keeps AbstractMethodError's constructors package-private, so the framework cannot throw one with a message and the Ant leg would not compile. IncompatibleClassChangeError carries the message, and a message naming the method is worth more here than the exactly right type. The three inline source blocks in the device runtime chapter move into docs/demos and are included by tag, which is what the guide validator asks of every other chapter. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 47fd057b7e
ℹ️ 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".
The interpreter's depth cap throws one, and it is the right type: ParparVM's java.lang has StackOverflowError and so does every JVM the simulator runs on. It was simply missing from this compile-time stub, so the Ant leg could not compile the framework while the Maven leg could -- the two disagree because only the Ant build puts CLDC11 on the bootclasspath. Reproducing that locally needs the same -bootclasspath; compiling core and CLDC11 together against a full JDK resolves java.lang from the JDK and reports nothing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
💡 Codex ReviewCodenameOne/Ports/Android/src/com/codename1/impl/android/AndroidImplementation.java Lines 1565 to 1567 in 3155e0b On every Android runtime build, CodenameOne/CodenameOne/src/com/codename1/impl/interp/InterpRuntime.java Lines 677 to 679 in 3155e0b When the leaf type is interpreted, allocating every reference array as a plain ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
Compared 12 screenshots: 12 matched. |
|
Developer Guide build artifacts are available for download from this workflow run:
Developer Guide quality checks: |
The chapter was written before the rebase brought in the vale gate and had never been through it: 141 findings, all in this one file. The bulk is the guide's house style of contractions. The rest is adverbs that carried no weight, two sentences opening with 'So', and three quotations of literal text -- a runtime message, a device dialog, Google Play's policy -- where moving the period inside the quotes would misquote the source, so those carry a vale-skip naming the reason. Where an adverb was load-bearing, the sentence says the thing instead: 'which silently dropped Runnable' is now 'which dropped Runnable with no diagnostic'. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7a54e8f350
ℹ️ 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".
The rebase reconstruction rewrote the file with LF where master has CRLF, so a 63-line change showed up as a 14,000-line rewrite. That is not only noise: CodeQL reports alerts for code a PR changed, and a whole-file diff re-reported twelve alerts that master already has and this branch did not introduce. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f86bfb26ac
ℹ️ 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".
The chapter used British spellings in a US-English guide -- which the cross-document coherency rule catches, not just the dictionary -- and a vocabulary LanguageTool has never heard of. Spellings are now US; the vocabulary (vtable, clazz, dex, desugar, devirtualize, supertype, cmake, thebaselab) is in the accept list with a line saying what each one is. Two sentences were rephrased rather than allowlisted: LanguageTool reads 'An interpreted X has to be an object...' as a typo for 'and' once the code spans are stripped, and the rule is right that the sentence was hard to parse. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 79950c65f5
ℹ️ 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".
Three more.
InterpCancelled extends Error precisely so a user `catch (Exception)`
around a loop cannot swallow it, but `catch (Throwable)` and
`catch (Error)` still matched -- and a program surrounding its loop
with one of those could resume and defeat the Stop button and the
EDT budget. `findHandler` now takes a `finallyOnly` flag; on
cancellation only javac's synthesised catch-all handlers match, so
`finally` blocks and synchronized regions still run their cleanup
(and their trailing `athrow` re-raises the cancellation), while
typed user handlers are skipped and the unwind continues.
`Pushed.class.getResourceAsStream("/data.json")` on an InterpClass
receiver reached `classCall` and fell through to
UnsupportedOperationException, even though the bundle carries the
resource. `classCall` now answers it directly: an absolute path is
looked up as-is, a relative path is qualified against the class's
package (the standard Java semantics), and either spelling of the
leading slash is tried so a resource published without the leading
`/` is still found.
The shell packer at `scripts/cn1-push.sh` still accepted any static
`main(String[])` regardless of visibility, so a private helper of
that signature could win over a real `public static void main` or
over a valid Lifecycle subclass. Match `DevicePush.findEntryPoint`
and require ACC_PUBLIC there too.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: caa8498dd0
ℹ️ 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".
Two more.
`runMain` invoked whatever `declaredMethod("main", "(String[])V")`
returned, so a Lifecycle subclass that also declared a private or
instance `main` (a diagnostic helper, a static-initializer trampoline)
would be entered through that helper with a null receiver rather than
starting the lifecycle. The packer's finder rejects one for exactly
this reason; the runtime now applies the same rule -- `isPublic() &&
isStatic()` -- and falls through to the Lifecycle path when it does
not hold. The error message names the requirement.
The Class[] copy-back tried to preserve InterpClass tokens by skipping
slots whose materialised host value was unchanged from what we passed
in. That test could not distinguish "host was read-only" from "host
explicitly assigned the same value" (both leave `dst[k]` identical to
the snapshot when the token was materialised through `hostClassFor`,
which for a pushed-only leaf lands on the ancestor stand-in that the
host is most likely to write back). Every slot is copied
unconditionally now: a pushed-only class was never a real host Class
to begin with, so keeping a token that survives the call and equals
its ancestor stand-in adds no fidelity and hides a legitimate host
write. Callers that need identity across a host call should keep
their own reference, as Java's array-by-reference semantics require.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a3c4b3bb5a
ℹ️ 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".
The float/double conversion into an interpreter slot used `floatToIntBits`/`doubleToLongBits`, which collapse every NaN pattern into the canonical 0x7fc00000 / 0x7ff8000000000000. A program that got a noncanonical NaN from a host call (`Float.intBitsToFloat(0x7fc00001)` and the double equivalent) and then read the bits back with `floatToRawIntBits` / `doubleToRawLongBits` would observe the canonical NaN instead of the payload the JVM preserves. Use the raw-bit conversions in the return-value unbox path (`InterpValues.unbox`), in the frame push helpers (`InterpFrame.pushFloat` / `pushDouble`), and in the bundle's LDC float encoding so a `LDC #NaN-payload` from bytecode arrives with its bits intact. The inverse conversions (`intBitsToFloat`, `longBitsToDouble`) already preserve raw bits, so the whole round- trip now does. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 16c6d833d7
ℹ️ 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".
Round 64 introduced `Float.floatToRawIntBits` and
`Double.doubleToRawLongBits` calls in the interpreter, but neither
method existed on Codename One's Float/Double stubs (the CLDC11 shim
the core framework compiles against, or ParparVM's JavaAPI). The
Ports/JavaSE build failed the compile as a result -- javase-simulator
CI caught it -- and the JavaScript port had no bind for the new
symbols either.
Add the raw-bit variants across every layer that carries the
Float/Double surface:
* Ports/CLDC11's Float and Double stubs gain method scaffolding so
the core framework compiles.
* ParparVM's JavaAPI declares them native; nativeMethods.m adds
`Float.floatToRawIntBits` beside the existing `Double` one, both
doing the same union-based type-pun that already preserves every
NaN payload verbatim.
* The JavaScript port's native registry lists the new symbol names,
and parparvm_runtime.js binds them to the same underlying bit
conversion the canonicalising entry point uses (JS has no native
NaN payload distinction, so the same body is right).
Round 64's runtime uses now resolve on every target.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ba08bc0a3a
ℹ️ 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".
Four more. Volatile field access had no memory barrier. `volatile boolean ready` went through a plain `Object[]` slot in the interpreter, so a worker- thread handoff pattern could let the reader observe `ready` while the writes it published stayed stale. The bundle now serialises each field's access word alongside its name/descriptor, `InterpClass` keeps the `ACC_VOLATILE` flag per instance-field slot and per static name, and `getField`/`putField` synchronise on the receiving InterpObject when the field is volatile so the pair share happens- before. Static field access already went through Hashtable get/put which are synchronised, so no additional wrapping is needed there. Synthesised lambda names could collide with a user class literally called `Owner$$Lambda$0` (a nested class `$Lambda$0`). Both were written to the bundle and the later `classesByName.put` overwrote the user class with the lambda, silently redirecting allocations of the user class to the lambda body. `InterpLambdaDesugar` now tracks every already-known class name -- the input set plus previously generated lambdas -- and advances the per-owner counter past any collision before synthesising. The interface pass in `InterpClass.buildVtable` copied each interface's whole `vtable`, which had already merged in every default inherited from its own superinterfaces. On `class C implements J, K` where `J extends I` overrides `I.m` and `K extends I` inherits it, copying K last put the inherited `I.m` on top of `J.m` -- ordinary Java dispatch picks the maximally specific `J.m`. The pass now copies only each interface's directly-declared methods (from `methods`, not from `vtable`), so a sibling's inherited default cannot mask another sibling's override. LDC_DOUBLE constants were serialised through `Double.toString`, which reduces every NaN to "NaN" -- so a bytecode LDC of a noncanonical NaN would round-trip as the canonical one. Encoded as raw long bits now (matching what LDC_FLOAT does), with a matching `Long.parseLong` + `longBitsToDouble` on the reader side. The bundle format version is bumped to 4 to keep an older reader from calling `Double.parseDouble` on `"4607182418800017408"`; keeps the writer and reader in sync. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 01735c8154
ℹ️ 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".
Two more, and a SpotBugs sweep. Back-edge cancellation escaped run() without ever consulting findHandler: the fuel check sat *outside* the try/catch that dispatches interpreted handlers, so a Stop or budget cancellation raised on a loop back edge -- the most common path -- skipped every source-level `finally`. Wrapped the back-edge checkpoint in its own try/catch that routes an InterpThrowable through findHandler at the current insn, so cleanup runs on the way out just like a checkpoint fired mid-method does. The cancellation filter from round 62 admitted only javac's catch-all (typeExtern < 0) handlers, which skipped try-with-resources cleanup -- that lives in a typed `catch (Throwable)` handler with a synthetic close+rethrow body. Removed the filter: any handler now matches for cancellation, so the cleanup runs. Stop and the EDT budget are still honoured because `cancelRequested` stays set for the ThreadState -- the next checkpoint after the handler returns raises InterpCancelled again, and a user `catch (Throwable)` around a runaway loop cannot silence Stop for long because the back-edge checkpoint keeps firing. The `finallyOnly` parameter is retained in the signature but ignored so callers do not all need touching for the removed distinction. Round 66 added `isStaticFieldVolatile`, which SpotBugs flagged as UPM_UNCALLED_PRIVATE_METHOD -- static field access already goes through `Hashtable.get`/`put`, whose synchronised bodies establish happens-before, so no interpreter-side wrapping is needed there. Dropped the helper and the parallel `volatileStatics` / mark plumbing on InterpClass; the reader still reads the flag word past for format compatibility with the writer. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8f0e87bf29
ℹ️ 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".
Two more. InterpIOSLinker's argument marshalling still went through the canonicalising `Float.floatToIntBits` / `Double.doubleToLongBits` when handing a value to the native thunk, so a pushed program that computed a noncanonical NaN via `Float.intBitsToFloat(0x7fc00001)` and passed it to a host method would see the canonical NaN come back through `floatToRawIntBits`. Round 64 fixed this on the interpreter's own slot paths and the LDC-float bundle encoding; the iOS-side marshalling is symmetric and now uses the raw-bit calls too. `scripts/devruntime-ide-project/pom.xml` passed only `src/main/java` to `DevicePush`, so a standard Maven project with Kotlin classes under `src/main/kotlin` had their `.kt` files absent from the bundle and the runtime rejected the whole push as missing source for `MyApp.kt`. `DevicePush` now accepts repeated `--source` arguments (a single one is still the default for callers that pass none), and both push profiles pass both roots explicitly. A missing root is a no-op so passing `src/main/kotlin` on a Java-only project is fine. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e6479da82f
ℹ️ 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".
Two more. `InterpClass.copyInto` for the superclass pass filtered private methods but not package-private ones. A subclass in a different package therefore inherited a superclass's default-access method into its vtable, silently executing a method a real JVM refuses with IllegalAccessError -- and worse when a same-signature interface default should have won, because the inherited method won by declaration order. JLS 8.4.6 restricts package-private visibility to classes in the same package; the superclass pass now takes the receiver's package and skips a source entry whose owner is in a different package. `interfaceOwnedToo` is also dropped from the signature: only one caller ever remained, always with `false`, so the parameter was noise. The JavaSE simulator port had no linker registered, so `InterpPlatform.isAvailable()` returned false and every push was refused with "this build has no interpreter bindings" -- even though the device-runtime guide names the simulator as the reflection backend's home. Added `InterpJavaSELinker` alongside its Android twin (both are reflection-based; iOS binds through the translator's invoke thunks) and registered it from `JavaSEPort.init`. The registration guard test now covers the JavaSE port too so a future rebase cannot strand it silently. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9bed444328
ℹ️ 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".
One fixed, one documented. A pushed class implementing only a host interface (extending Object) still gets a shim peer, but its own `toString`/`hashCode`/`equals` belong to the interpreter -- the shim's inherited Object defaults print `Interp_Runnable@...` and disagree with the intercepted `getClass()` above them. `InterpObject` now records `hostPeerFromInterfacesOnly` when the peer was created for host interfaces only (no host superclass), and the invokevirtual path routes Object's default methods through `objectCall` in that case before falling to the shim. Interpreted overrides still win via the `resolveVirtual` result above; only unoverridden Object defaults route this way. The array-class sidecar caps rank at three -- a ParparVM constraint, not an interp bug. Every reference-array id in the AOT build is packed into three slots per class (`cn1_array_1_id_X` through `cn1_array_3_id_X`) and `cn1_globals.m`'s cast arithmetic leans on that fixed step; extending the sidecar without widening the AOT layout would produce class ids the runtime cannot consume. The comment in `Parser.writeSymbolSidecar` now spells this out so a future reviewer sees why it is not just a loop bound, and documents the current behaviour: pushed code using rank 4+ arrays (`String[][][][]`, rare in practice) resolves the outer descriptor to id -1 and falls back to untyped `Object[]` cascades -- functional, but reflection on the outer type reports Object[] rather than the source array type. Kept as a known limitation until ParparVM's array-class layout is generalised. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 271f0d368c
ℹ️ 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".
Two follow-ups on round 70's interface-only peer route. The route sent every intercepted Object method through `objectCall`, including `wait`/`notify`/`notifyAll`. MONITORENTER already locked the peer for a `synchronized (this)` block, and running the wait on the interpreter's InterpObject wrapper acquired a different monitor -- IllegalMonitorStateException from the very block the pushed program synchronised. Restrict the route to naming/identity defaults (`toString`, `hashCode`, `equals`); monitor methods stay on the peer so they wait on the monitor MONITORENTER already holds. Object's `equals` in `objectCall` used a plain identity check against `io`, but the argument came off the stack through `popBoxed`, which converts a peer-backed object back to its InterpObject wrapper -- except when the caller passed `this` for an interface-only peer and the value arrived as the peer itself, not the wrapper. `value.equals( value)` then answered false. Route the argument through `fromHost` first so both representations resolve to the same InterpObject. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d74c2b7d6f
ℹ️ 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".
`System.identityHashCode(value)` on a pushed object with an interface- only peer hashed the peer, because `popBoxed` converts an interpreter-owned InterpObject to its hostPeer before every host call. The interpreter's own `Object.hashCode` (in `objectCall`) hashes the InterpObject wrapper, so a pushed program that did `value.hashCode() == System.identityHashCode(value)` saw the two differ even for a class that inherits Object's default hashCode. Intercept `java/lang/System.identityHashCode(Object)` on the invokestatic path: unwrap through `fromHost` (which turns a peer back into its InterpObject) and hash the wrapper, matching the wrapper's own hashCode. Non-InterpObject arguments fall through to the host System.identityHashCode as before, and no other host method needs this treatment -- `objectCall`'s hashCode is the only interpreter identity that the peer's own identityHashCode would disagree with. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4fd0c9ddb5
ℹ️ 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".
Two more. `sortBySpecificity` had the topological direction inverted for the new `copyDeclaredMethods` pass. Its old anyBelow predicate picked "whatever has a subtype still in the pool" first, so a three-level chain `C -> B -> A` came out `[B, A, C]` -- B.m landed in the vtable and was then overwritten by the less-specific A.m. Flipped to "whatever has no unpicked superinterface" so supertypes come first and each subinterface's declared method overrides its parent's on copy. Two-level chains (round 66's regression case) still work; the three-level case the reviewer flagged is fixed. `resolveVirtual` bypassed an inaccessible superclass method after the round 69 vtable filter dropped it. On `q.C extends p.A` with A's package-private `m()` and C implementing an interface's public/default `m()`, an invokevirtual q/C.m must be an IllegalAccessError per JVMS 5.4.3.3, not a silent fallback to the interface default. Added `findClassMethodInSuperchain` to locate the inaccessible method directly and raise the linkage error before returning to `receiver.resolve()`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a2bcb6dea1
ℹ️ 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".
Three fixes plus a CI recovery. Round 73's IllegalAccessError broke `build-test (8)`: the CLDC11 subset the core compiles against does not carry that class. Switched to IncompatibleClassChangeError with the same message, matching the existing precedent for AbstractMethodError -- IllegalAccessError is a subtype of IncompatibleClassChangeError on the JVM, so pushed catch clauses see the same shape either way. `enumCall.equals`/`compareTo` compared identity against `io` directly, but the argument came off the stack through `popBoxed` and arrived as its peer whenever the enum implements a host interface (interface-only peer). A `value.equals(value)` or `value.compareTo(value)` therefore returned false / threw ClassCastException. Normalise through `fromHost` so both representations resolve to the same InterpObject, matching the earlier `objectCall.equals` fix. An interpreted enum's inherited `Comparable` and `Serializable` came in through `java.lang.Enum`, which the peer builder skips (Enum has no shim and cannot be subclassed). The peer therefore did not advertise `Comparable`, so `Collections.sort` on a list of pushed constants cast to `Comparable` and threw ClassCastException. Attach Comparable and Serializable to an enum's peer explicitly, and route Enum-inherited method calls through `enumCall` from `dispatch` when the pushed class did not override them -- so a host sort actually finds the ordering. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a92580bb51
ℹ️ 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".
A LAN peer that knows a paired peer id -- transmitted in cleartext on
an ordinary push -- could open many FRAME_PUSH connections in parallel,
advertise the 64 MiB MAX_BUNDLE on each, and force the device to
allocate a 64 MiB body per connection before proving possession of the
pairing secret. `handleAccepted` serves each connection on its own
thread and the watchdog holds a connection that keeps sending occasional
chunks, so an unauthenticated peer could sit on enough 64 MiB arrays to
exhaust the device heap.
Cap aggregate pre-authentication push memory at 2 * MAX_BUNDLE across
all in-flight connections. A push whose declared length would take the
running total past the cap is refused early with a message the desktop
can show ("this device is busy with another push"), before any
allocation. The reservation is released as soon as authentication
succeeds -- so a legitimate second push during the approval dialog is
not queued behind the first -- and always in a `finally` on the reject
path. A double release is clamped at zero so a later legitimate push
cannot silently reserve past the cap.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e8c945d4c6
ℹ️ 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".
The maximally-specific interface tiebreak silently picked the first
non-dominated candidate when two truly unrelated defaults remained --
a shape binary-compatible interface evolution can produce (an interface
adds a default that a sibling interface also declares) -- rather than
raising the JVMS 5.4.3.3 IncompatibleClassChangeError. Pushed code
therefore ran an arbitrary body the JVM refuses.
All three resolvers now collect the maximally-specific set (deduped by
declaring interface), and:
* exactly one -> return it;
* more than one -> throw IncompatibleClassChangeError naming both
declaring interfaces;
* none dominant -> fall through to the pooled first (unreachable
in practice; there for defence).
Abstract methods are dropped at collection time in the reflection
resolvers -- they contribute no body, so they cannot compete as
defaults and would confuse the tie-break. Reflection linker on JavaSE
and Android, the test-only ReflectionInterpLinker used by
core-unittests, and the iOS symbol-table resolver all get the fix in
lockstep so the three ports agree.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 050c80fbb4
ℹ️ 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".
| // arg-conversion path materialises a real Class[] | ||
| // when handing one to a host method that wants it. | ||
| f.pushRef(isInterpretedLeaf(comp) || isClassLeaf(comp) | ||
| ? new Object[count] |
There was a problem hiding this comment.
Retain component types for interpreted arrays
When both types are pushed, Base[] a = new Sub[1]; a[0] = new Base(); must throw ArrayStoreException, but this allocation represents Sub[] as an untyped Object[], so the later AASTORE accepts the incompatible value. The same erasure makes checks such as new Sub[0] instanceof Other[] incorrectly succeed because the element-by-element test has no runtime component metadata to inspect; retain the interpreted component descriptor with each array.
Useful? React with 👍 / 👎.
| } finally { | ||
| if (arrays != null) { | ||
| for (int i = 0; i < arrays.size(); i++) { | ||
| fromHostElements(arrays.elementAt(i), new Vector()); |
There was a problem hiding this comment.
Preserve peer elements in arrays retained by host calls
When a host method retains the passed array, this unconditional conversion back leaves the host's alias containing InterpObject wrappers rather than the peers it was given. For example, Arrays.asList(items) retains its input array; after this finally, a later host-side Collections.sort(list) sees wrappers that do not implement the host Comparable interface even when their peers do, and throws ClassCastException. Escaped host aliases need to remain host-compatible rather than having their contents reverted in place.
Useful? React with 👍 / 👎.
Adds a device runtime: install one app on a phone, then push a project to it
from your IDE and watch it run natively in seconds. A third way to run a
Codename One app, alongside the simulator and a cloud device build.
Pushed classes are interpreted on the device against the framework already
compiled into it. Nothing is built, signed or installed between edits — the
edit-run loop measured 2.8 seconds end to end.
Try it
The desktop finds the phone on the local network, shows a six-digit pairing
code you type once, and from then on it is edit-and-run.
--device <address>is there for networks that block a scan.
What is here
CodenameOne/src/com/codename1/interp/Ports/{Android,iOSPort}vm/ByteCodeTranslatorDevicePushtoolscripts/cn1-device-runtime/scripts/devruntime-ide-project/scripts/devruntime-probes/docs/developer-guide/Device-Runtime.asciidocDecisions worth reviewing
Shims are generated over the whole API, never curated. A hand-maintained
list is a promise that applications only subclass what somebody anticipated, and
its failure mode is not an error message but an override that is silently never
called. The generator fails the build rather than pruning what will not compile
— a compile-and-drop loop once silently ate
Interp_ui_Form.Native-heavy subsystems are excluded from the shim set (
ai,ar,camera,surfaces,car,health, …). A shim is a compiled reference to theclass it extends, which is exactly what the build scans to decide what to link,
so generating the full API pulled 300MB of ML Kit, ARCore and CameraX natives
into an app that calls none of them. Cost: those types cannot be subclassed by
pushed code; calling them degrades to
isSupported() == false, which is theruntime's existing contract for a cn1lib without its native half.
iOS keeps shims rather than runtime vtable synthesis. Synthesis would make
14 more types extensible on iOS only, and Android cannot follow — so the usable
capability, the intersection, does not move.
InterpHostVtableSynthesisIntegrationTeststays for the day that changes.
synchronizeduses the real object monitor, not a private lock table, whichis what makes
wait/notifywork.Framework fixes that fell out
AndroidImplementation.getHostOrIP()returneddummy0's IPv6 link-localinstead of a usable IPv4 — affects any caller.
CodenameOneImplementation.getResourceAsStreamgained a local-resource hook,so a pushed program's
theme.resis found byResources.openLayered, whichnever passes through
Display.Verification
4798 core · 506 translator · 52 interpreter · SpotBugs 0 · 20-program device
battery green on an Android emulator and the iOS simulator, including a
four-file, three-package app entered through
Lifecyclerather thanmain.Every probe exists because something plausible turned out not to work; the
README records which defect each was written for.
Review rounds
Eleven findings from codex, all real, all fixed and each answered on its thread.
The two that mattered most:
every push and never rotated, so one captured frame authorised arbitrary code
on that phone forever. v2 is gone rather than deprecated. v3 derives a 256-bit
secret on both ends from
(typed code, peerId, deviceId)— never transmitted —and every connection answers a fresh challenge whose MAC covers the bundle.
Authentication happens before the approval prompt, so nobody can raise dialogs
on a stranger's phone until they tap Approve to stop them. What it still does
not defeat is a passive observer of the pairing exchange itself, and the docs
say so.
reads returned whatever half of it had been assigned. Four states and an owning
thread now, per JLS 12.4.2.
Shipping it
.github/workflows/device-runtime-store.ymlruns Mondays and on demand,uploading to Play internal testing and TestFlight. It does not promote to
production and does not submit for review — a weekly automatic release would
put unread builds in front of the public and queue an iOS review every week
whether anything changed or not. Promotion stays one deliberate command.
Without credentials the job names the missing secrets and stops rather than
publishing half a release; none exist yet, so today it is a no-op that says so.
Listing text is in fastlane's layout (
scripts/cn1-device-runtime/fastlane/) sosupplyanddeliverconsume it directly, withstore/privacy.mdfor bothstores' data forms and
store/README.mdfor the secrets, the pre-submissionchecklist and the review-risk assessment.
The compliance point that matters: this app runs code it did not ship with,
which is Guideline 2.5.2 — permitted for tools that develop or test code, and
only while the source is "completely viewable and editable by the user". The
runtime refuses to load a bundle whose sources it lacks, and shows them under
View source. Removing that screen makes the app unsubmittable, which is why
the code says so where the screen is defined.
Not done
NativeLookupstubbing covers the Java half of a cn1lib; the native halfreports unsupported. Resource push covers
theme.res, CSS and images.Screenshots for both stores, the Play content rating questionnaire, Apple's
privacy manifest and the console listings themselves are human steps, listed in
store/README.md.🤖 Generated with Claude Code