Skip to content

Device runtime: run pushed Codename One apps on a phone - #5561

Open
shai-almog wants to merge 100 commits into
masterfrom
device-runtime
Open

Device runtime: run pushed Codename One apps on a phone#5561
shai-almog wants to merge 100 commits into
masterfrom
device-runtime

Conversation

@shai-almog

@shai-almog shai-almog commented Aug 17, 2026

Copy link
Copy Markdown
Collaborator

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

# install ~/cn1-device-runtime.apk on a phone (11MB, no native libs, any arch)
cd scripts/devruntime-ide-project
mvn -Ppush-lan package

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/ the interpreter
Ports/{Android,iOSPort} per-platform linkers, iOS native bridge
vm/ByteCodeTranslator bundle writer, lambda desugaring, DevicePush tool
scripts/cn1-device-runtime/ the runtime app itself
scripts/devruntime-ide-project/ the project you open in an IDE
scripts/devruntime-probes/ 20 programs that found the defects worth knowing about
docs/developer-guide/Device-Runtime.asciidoc how and why

Decisions 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 the
class 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 the
runtime'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. InterpHostVtableSynthesisIntegrationTest
stays for the day that changes.

synchronized uses the real object monitor, not a private lock table, which
is what makes wait/notify work.

Framework fixes that fell out

  • AndroidImplementation.getHostOrIP() returned dummy0's IPv6 link-local
    instead of a usable IPv4 — affects any caller.
  • CodenameOneImplementation.getResourceAsStream gained a local-resource hook,
    so a pushed program's theme.res is found by Resources.openLayered, which
    never 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 Lifecycle rather than main.

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:

  • Pairing handed out a bearer token. The peer id travelled in the clear on
    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.
  • A failed class initializer left the class looking initialized, so later
    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.yml runs 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/) so
supply and deliver consume it directly, with store/privacy.md for both
stores' data forms and store/README.md for the secrets, the pre-submission
checklist 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

NativeLookup stubbing covers the Java half of a cn1lib; the native half
reports 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

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 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".

Comment thread .github/workflows/device-runtime-store.yml Outdated
Comment thread CodenameOne/src/com/codename1/interp/InterpRuntime.java Outdated
Comment thread vm/ByteCodeTranslator/src/com/codename1/tools/translator/InterpBundleWriter.java Outdated
shai-almog and others added 3 commits August 17, 2026 15:49
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>
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>
@chatgpt-codex-connector

Copy link
Copy Markdown

💡 Codex Review


P1 Badge Register the Android linker before checking support

On every Android launch this support check is false because no code installs the newly added InterpAndroidLinker: a repository-wide search finds InterpPlatform.register(...) only in IOSImplementation. Consequently DeviceRuntimeApp.init() returns without starting either transport and the Android runtime app cannot accept any pushed program; register an InterpAndroidLinker during Android port initialization.


final InterpRuntime rt = new InterpRuntime(bundle, InterpPlatform.getLinker(), factory);
factory.attach(rt);
runtime = rt;

P2 Badge Stop the previous program before replacing its runtime

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 Lifecycle.stop()/destroy(). Programs that registered global listeners, timers, network callbacks, or worker threads therefore continue executing alongside the replacement, and after this overwrite the service can no longer stop them.


if (!send(payload, port, peerId, false) && rejectedAsUnpaired()) {

P2 Badge Propagate failed LAN pushes as process failures

For an already-paired LAN push, send() returns false when the user denies approval, authentication fails, or the device rejects/runs the bundle unsuccessfully; unless the message contains “not paired,” this condition falls through and main() exits with status 0. The documented Maven push-lan profile therefore reports BUILD SUCCESS for a failed deployment, which also prevents scripts and IDE integrations from detecting the failure.


synchronized (found) {
if (found[0]) {
return;
}
found[0] = true;
foundAt[0] = candidate;
}
handle(is, os, false);

P2 Badge Validate a discovered peer before remembering its address

If any unrelated service happens to accept this port during the subnet sweep, the callback marks it as found before handle() validates the protocol magic. The sweep then persists that address, and subsequent dial attempts likewise treat a successful TCP connection as served even when the peer never sends a runtime frame, so discovery can remain stuck on the wrong machine; only publish found/foundAt after a valid handshake.

ℹ️ 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".

@github-actions

github-actions Bot commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

✅ Continuous Quality Report

Test & Coverage

Static Analysis

  • SpotBugs [Report archive]
    • ByteCodeTranslator: 0 findings (no issues)
    • android: 0 findings (no issues)
    • codenameone-maven-plugin: 0 findings (no issues)
    • core-unittests: 0 findings (no issues)
    • ios: 0 findings (no issues)
  • PMD: 0 findings (no issues) [Report archive]
  • Checkstyle: 0 findings (no issues) [Report archive]

Generated automatically by the PR CI workflow.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 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".

Comment thread scripts/cn1-push.sh Outdated
Comment thread CodenameOne/src/com/codename1/impl/interp/InterpRuntime.java Outdated
Comment thread .github/workflows/device-runtime-store.yml Outdated
shai-almog and others added 2 commits August 17, 2026 21:15
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>
@chatgpt-codex-connector

Copy link
Copy Markdown

💡 Codex Review

ShimObjectFactory factory = new ShimObjectFactory();
final InterpRuntime rt = new InterpRuntime(bundle, InterpPlatform.getLinker(), factory);
factory.attach(rt);
runtime = rt;

P1 Badge Tear down the previous runtime before replacing it

When 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.


if ("toString".equals(name) && args.length == 0) {
return io.toString();
}
return NOT_OBJECT_METHOD;

P2 Badge Route Object monitor methods to interpreted-object monitors

For a peerless interpreted object, calls inherited from Object are handled here, but wait, notify, and notifyAll fall through to NOT_OBJECT_METHOD and ultimately raise AbstractMethodError. Consequently ordinary code such as synchronized (lock) { lock.wait(); }, where lock is a pushed POJO, cannot use Java monitor coordination even though MONITORENTER successfully acquired that same InterpObject; dispatch these methods against the monitor used by the interpreter.


for (File f : kids) {
if (f.isDirectory()) {
addSourceTree(f);
} else if (f.getName().endsWith(".java")) {
String text = new String(Files.readAllBytes(f.toPath()), StandardCharsets.UTF_8);
addSource(sourceKey(packageOf(text), f.getName()), text);

P2 Badge Include Kotlin files in pushed source bundles

When compiled output contains Kotlin classes, even explicitly passing a Kotlin source directory to --source cannot produce a valid bundle because this traversal ignores every .kt file. The reader later requires the SourceFile entry (for example Foo.kt) for each carried class and rejects the bundle as missing source, so Kotlin Codename One applications cannot be pushed; collect Kotlin sources and ensure the default project discovery also includes src/main/kotlin.


if ("com/codename1/system/Lifecycle".equals(cn.superName)) {
lifecycle = cn.name;
}

P2 Badge Discover Lifecycle subclasses through the class hierarchy

Entry-point discovery recognizes only classes whose immediate superclass is Lifecycle. If an application class extends a project-defined base lifecycle, this either reports no entry point or selects the base class itself (often abstract) instead of the concrete application, even though InterpRuntime.extendsHost() can execute an indirect subclass once selected. Resolve the collected superclass graph and choose the concrete transitive Lifecycle subclass.

ℹ️ 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".

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 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".

Comment thread vm/ByteCodeTranslator/src/com/codename1/tools/translator/DevicePush.java Outdated
Comment thread vm/ByteCodeTranslator/src/com/codename1/tools/translator/DevicePush.java Outdated
Comment thread CodenameOne/src/com/codename1/impl/interp/InterpRuntime.java
Comment thread CodenameOne/src/com/codename1/impl/interp/InterpRuntime.java Outdated
@github-actions

Copy link
Copy Markdown
Contributor

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>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 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".

Comment thread CodenameOne/src/com/codename1/impl/interp/InterpRuntime.java Outdated
Comment thread CodenameOne/src/com/codename1/impl/interp/InterpRuntime.java
Comment thread CodenameOne/src/com/codename1/impl/interp/InterpRuntime.java Outdated
Comment thread vm/ByteCodeTranslator/src/com/codename1/tools/translator/DevicePush.java Outdated
Comment thread CodenameOne/src/com/codename1/impl/CodenameOneImplementation.java Fixed
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>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 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".

Comment thread CodenameOne/src/com/codename1/impl/interp/InterpObject.java
Comment thread CodenameOne/src/com/codename1/impl/interp/InterpRuntime.java
Comment thread CodenameOne/src/com/codename1/impl/interp/InterpRuntime.java Outdated
Comment thread CodenameOne/src/com/codename1/impl/interp/InterpRuntime.java Outdated
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>
@chatgpt-codex-connector

Copy link
Copy Markdown

💡 Codex Review

@Override
public void init(Object m) {
// NOTE: Do not explicitly set the PlayServices instance to anything other than

P1 Badge Register the Android linker during port initialization

On every Android runtime build, DeviceRuntimeService.isSupported() requires InterpPlatform.isAvailable(), but the Android initialization path never calls InterpPlatform.register(new InterpAndroidLinker()); a repository-wide search finds no other construction of InterpAndroidLinker, while IOSImplementation.init() performs the corresponding registration. Consequently startDialer() always returns false on Android with “no interpreter bindings,” so the newly added Android device runtime cannot accept any pushes.


f.pushRef(isInterpretedLeaf(comp)
? new Object[count]
: linker.newArray(comp.startsWith("[") ? comp : "L" + comp + ";", count));

P2 Badge Preserve interpreted array component types

When the leaf type is interpreted, allocating every reference array as a plain Object[] discards its runtime component type. For example, after Sub[] a = new Sub[1]; Base[] b = a;, storing new Base() through b must throw ArrayStoreException, but AASTORE later writes unconditionally into this Object[], so the invalid value is accepted and the array is silently corrupted. Retain component metadata or otherwise validate each store against the allocated array type.

ℹ️ 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".

@shai-almog

shai-almog commented Aug 17, 2026

Copy link
Copy Markdown
Collaborator Author

Compared 12 screenshots: 12 matched.
✅ JavaSE simulator integration screenshots matched stored baselines.

@github-actions

github-actions Bot commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

Developer Guide build artifacts are available for download from this workflow run:

Developer Guide quality checks:

  • AsciiDoc linter: No issues found (report)
  • Vale: No alerts found (report)
  • Paragraph capitalization: No paragraph capitalization issues (report)
  • LanguageTool: No grammar matches (report)
  • Image references: No unused images detected (report)

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>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 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".

Comment thread Ports/iOSPort/src/com/codename1/impl/ios/InterpIOSSymbols.java Outdated
Comment thread CodenameOne/src/com/codename1/impl/interp/InterpRuntime.java Outdated
Comment thread CodenameOne/src/com/codename1/impl/interp/InterpRuntime.java Outdated
Comment thread CodenameOne/src/com/codename1/impl/interp/InterpRuntime.java
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>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 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".

Comment thread scripts/cn1-push.sh Outdated
Comment thread scripts/cn1-push.sh Outdated
Comment thread CodenameOne/src/com/codename1/impl/interp/InterpRuntime.java
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>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 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".

Comment thread CodenameOne/src/com/codename1/impl/interp/InterpRuntime.java
Comment thread CodenameOne/src/com/codename1/impl/interp/InterpRuntime.java
Comment thread CodenameOne/src/com/codename1/impl/interp/InterpRuntime.java
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>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 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".

Comment thread CodenameOne/src/com/codename1/impl/interp/InterpRuntime.java Outdated
Comment thread CodenameOne/src/com/codename1/impl/interp/InterpRuntime.java Outdated
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>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 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".

Comment thread CodenameOne/src/com/codename1/impl/interp/InterpValues.java Outdated
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>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 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>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 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".

Comment thread CodenameOne/src/com/codename1/impl/interp/InterpClass.java
Comment thread vm/ByteCodeTranslator/src/com/codename1/tools/translator/InterpBundleWriter.java Outdated
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>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 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".

Comment thread CodenameOne/src/com/codename1/impl/interp/InterpRuntime.java Outdated
Comment thread CodenameOne/src/com/codename1/impl/interp/InterpRuntime.java Outdated
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>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 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".

Comment thread Ports/iOSPort/src/com/codename1/impl/ios/InterpIOSLinker.java Outdated
Comment thread scripts/devruntime-ide-project/pom.xml
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>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 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".

Comment thread CodenameOne/src/com/codename1/impl/interp/InterpClass.java
Comment thread CodenameOne/src/com/codename1/impl/interp/InterpPlatform.java
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>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 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".

Comment thread CodenameOne/src/com/codename1/impl/interp/InterpRuntime.java
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>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 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".

Comment thread CodenameOne/src/com/codename1/impl/interp/InterpRuntime.java
Comment thread CodenameOne/src/com/codename1/impl/interp/InterpRuntime.java Outdated
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>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 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".

Comment thread CodenameOne/src/com/codename1/impl/interp/InterpRuntime.java
`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>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 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".

Comment thread CodenameOne/src/com/codename1/impl/interp/InterpClass.java Outdated
Comment thread CodenameOne/src/com/codename1/impl/interp/InterpRuntime.java
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>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 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".

Comment thread CodenameOne/src/com/codename1/impl/interp/InterpRuntime.java Outdated
Comment thread CodenameOne/src/com/codename1/impl/interp/InterpRuntime.java
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>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 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>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 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".

Comment thread Ports/Android/src/com/codename1/impl/android/InterpAndroidLinker.java Outdated
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>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 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]

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge 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());

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge 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 👍 / 👎.

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