Loopback server sockets, so MCP can serve any platform - #5472
Loopback server sockets, so MCP can serve any platform#5472shai-almog wants to merge 22 commits into
Conversation
MCP's socket transport was JavaSE-only, and for a real reason: it binds a server socket, and the portable com.codename1.io.Socket API only binds the wildcard address — which would publish a channel that can read the screen and drive the UI on every network interface. So attaching an agent to an app running on a device was simply not possible. This adds the missing capability rather than working around it: - Socket.listenLoopback(port, class) and isLoopbackServerSocketSupported(), backed by a new CodenameOneImplementation.listenSocketLoopback hook. It is deliberately SEPARATE from listenSocket and defaults to unsupported: a port must implement loopback binding explicitly, because answering this with the wildcard implementation would silently widen the reach of a channel the caller asked to keep local. There is no fallback for the same reason. - MCPLoopbackSocketTransport in core, built on that API, used automatically by any port that did not register a transport of its own. JavaSE keeps its java.net implementation. - JavaSE and Android bind InetAddress.getLoopbackAddress(). Their server-socket caches key loopback and wildcard sockets separately, so a port already bound wildcard can never be handed to a caller that asked for loopback. - iOS gains server sockets at all: it had none (listenSocket returned null). The native implementation binds INADDR_LOOPBACK, listens, and wraps each accepted descriptor in a CFStream pair, yielding the VM thread across the blocking accept. Wildcard listening stays unsupported there, so an iOS app cannot accidentally publish a port. Tests cover the framing and session handling: frame delimiting independent of packet boundaries, CRLF tolerance, write-with-no-client, reconnect without ending the server, and close waking a pending read. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The loopback listener accepted connections but no data ever crossed them. The accepted descriptor was wrapped in a CFStream pair scheduled on [NSRunLoop currentRunLoop] — which is what the client-side connect path does, and correct there — but a server socket is read on a background thread whose run loop never runs, so the streams opened and then delivered nothing. Accepted sockets now use recv/send on the descriptor directly, with FIONREAD for available input and a blocking read (the caller is a reader thread that expects to wait, and a non-blocking nil return would spin). The client path is untouched. Verified end to end on the iOS simulator: an agent attaches to a running app, reads its accessibility tree, and drives it through several screens. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR adds loopback-only server socket support to Codename One so MCP can offer a safe “attach to a running app” socket transport on non-JavaSE ports (device/simulator) without exposing the channel on all network interfaces.
Changes:
- Adds core loopback server-socket API (
Socket.listenLoopback()/isLoopbackServerSocketSupported()) backed by a newCodenameOneImplementation.listenSocketLoopback()hook. - Implements loopback listening in JavaSE/Android, and introduces loopback-only server sockets on iOS via native
INADDR_LOOPBACKbinding + raw-fd IO for accepted sockets. - Adds a portable
MCPLoopbackSocketTransportfallback (used when a port doesn’t register its own socket transport) and unit tests covering framing/session behavior.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| Ports/JavaSE/src/com/codename1/impl/javase/JavaSEPort.java | Adds loopback-binding support and separates cached wildcard vs loopback server sockets. |
| Ports/Android/src/com/codename1/impl/android/AndroidImplementation.java | Adds loopback-binding support and separate server-socket caching for wildcard vs loopback. |
| Ports/iOSPort/src/com/codename1/impl/ios/IOSNative.java | Declares native loopback listen entrypoint for iOS. |
| Ports/iOSPort/src/com/codename1/impl/ios/IOSImplementation.java | Exposes loopback server socket support via listenSocketLoopback() and capability flag. |
| Ports/iOSPort/nativeSources/SocketImpl.m | Implements loopback-only listening on iOS and raw-fd read/write for accepted sockets. |
| Ports/iOSPort/nativeSources/SocketImpl.h | Adds raw-fd field and loopback listen method declaration for server-side sockets. |
| Ports/iOSPort/nativeSources/IOSNative.m | Bridges iOS native loopback listen into the Java layer. |
| CodenameOne/src/com/codename1/mcp/MCPLoopbackSocketTransport.java | New portable MCP loopback socket transport built on com.codename1.io.Socket. |
| CodenameOne/src/com/codename1/mcp/MCP.java | Registers portable socket transport fallback and updates socket support detection. |
| CodenameOne/src/com/codename1/io/Socket.java | Adds loopback listen API and routes listening through loopback vs wildcard accept hooks. |
| CodenameOne/src/com/codename1/impl/CodenameOneImplementation.java | Introduces loopback server socket capability hook with safe default (unsupported). |
| maven/core-unittests/src/test/java/com/codename1/testing/TestCodenameOneImplementation.java | Extends test implementation to support loopback server sockets for unit tests. |
| maven/core-unittests/src/test/java/com/codename1/mcp/MCPLoopbackSocketTransportTest.java | Adds tests for message framing and reconnect/close semantics. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Compared 12 screenshots: 12 matched. |
Cloudflare Preview
|
|
Compared 151 screenshots: 151 matched. Native Android coverage
✅ Native Android screenshot tests passed. Native Android coverage
Benchmark ResultsDetailed Performance Metrics
|
Four review findings plus the static-analysis and header gates. - The server-socket cache distinguished loopback from wildcard by negating the port. -0 == 0, so on an ephemeral-port listener (port 0) a loopback request could be handed the wildcard-bound socket, defeating the one guarantee this API exists to make. Loopback and wildcard sockets now live in separate maps, so no encoding can collide. Fixed in both the JavaSE and Android ports. - Socket.listenLoopback() documented that it fails when the platform has no loopback support, but it started a listener thread that hit the default implementation's exception and merely logged it — the caller believed it was listening. It now throws IllegalStateException on the calling thread. - The iOS listenSocketLoopback binding released nothing when the bind or accept failed, leaking a SocketImpl per retry. (The adjacent connectSocket has the same pre-existing shape; left alone to keep this change reviewable.) - PMD: the identity comparisons and borrowed-stream locals in the transport are deliberate — a singleton registration slot and streams owned by the connection callback — so they carry NOPMD markers explaining why, rather than relaxing CompareObjectsWithEquals or CloseResource for the whole codebase. - SocketImpl.h/.m never carried a copyright header; modifying them tripped the header gate. Added the standard header. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Compared 148 screenshots: 148 matched. Benchmark Results
Detailed Performance Metrics
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (1)
Ports/iOSPort/nativeSources/SocketImpl.m:200
- In the raw-fd server-side path,
writeToStream:marks the socket disconnected onsend()failure but does not closerawFd. This can leak the accepted file descriptor on client disconnects or write errors, and may leaverawFdpointing at an unusable socket until an explicit disconnect happens.
if(rawFd >= 0) {
const uint8_t* bytes = (const uint8_t*)[param bytes];
size_t remaining = [param length];
while(remaining > 0) {
ssize_t w = send(rawFd, bytes, remaining, 0);
if(w <= 0) {
connected = NO;
return;
}
✅ Continuous Quality ReportTest & Coverage
Static Analysis
Generated automatically by the PR CI workflow. |
The Android port compiles CodenameOne/src through ant javac with ASCII
encoding, so the em dashes in the doc comments I added were hard errors
("unmappable character for encoding ASCII"), not warnings. Replaced with
hyphens; master carried no non-ASCII in these files.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Serving MCP over a socket used to be JavaSE only, which meant simulator and desktop tooling: inherently a development environment. Now that the transport works wherever loopback can be bound, the same call can bind inside an app on a phone, and the risk profile is not the same. An attached agent reads the screen and drives the UI, and loopback is shared by everything on the device rather than being private to one app. So on a shipped build, any OTHER installed app could connect to the port and drive yours. Nothing binds unless the app calls a starter, and an unused MCP is stripped by the VM, so this cannot happen by accident -- but a starter left in the code is exactly how a debugging facility reaches production. startSocketServer now refuses on a release build. A development build is: - Android: the package carries FLAG_DEBUGGABLE, which the build sets for a debug variant and clears for a release variant. - iOS: the provisioning profile grants get-task-allow, the entitlement that lets a debugger attach. Development and ad-hoc profiles carry it, App Store and enterprise profiles do not. Unlike the Xcode DEBUG macro this does not depend on which configuration the build server compiled. - JavaSE: always, since that port is the simulator, the designer and the desktop tooling, and its MCP menu already serves agents from there. The default for a port that cannot tell is "release", so the answer errs towards withholding the facility rather than exposing it. setAllowOnReleaseBuilds(true) lifts the block for a build that ships to a controlled fleet. The underlying question is more broadly useful than MCP, so it is exposed as Display.isDebuggableBuild() and documented for gating any development-only facility. Verified in the built artifacts of all three ports, not just at the source level; the iOS native function was clang syntax checked for device and simulator targets. Guide chapter updated with a "Development builds only" section. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 17 out of 17 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (3)
CodenameOne/src/com/codename1/mcp/MCP.java:132
- The startSocketServer() JavaDoc still says it requires a port-registered socket transport factory, but the implementation now falls back to the portable loopback transport when available. Updating this line will prevent API consumers from concluding socket attach is JavaSE-only.
/// Requires a platform socket transport factory (registered by the JavaSE port).
Ports/iOSPort/nativeSources/SocketImpl.m:179
- In the rawFd (accepted-socket) path, recv() returning 0/-1 marks connected=NO but leaves the file descriptor open. Because SocketInputStream/SocketOutputStream only call disconnectSocket() when isSocketConnected() is true, this can leak accepted client fds on orderly disconnects and read errors. Close rawFd when recv() indicates EOF/error.
if(r > 0) {
return [NSData dataWithBytes:buffer length:r];
}
connected = NO;
return nil;
Ports/iOSPort/nativeSources/SocketImpl.m:304
- Accepted sockets use raw send()/recv(). On iOS/macOS, a write to a closed socket can raise SIGPIPE unless suppressed, which in this codebase is routed through a signal handler and can surface as an unexpected RuntimeException. Consider disabling SIGPIPE for the accepted clientFd (e.g., SO_NOSIGPIPE) so send() reports EPIPE instead of generating a signal.
// Serve this socket with raw descriptor I/O. An accepted connection is read on a
// background thread whose run loop never runs, so a scheduled CFStream would open
// but never deliver a byte.
rawFd = clientFd;
connected = YES;
|
Compared 217 screenshots: 217 matched. |
|
Developer Guide build artifacts are available for download from this workflow run:
Developer Guide quality checks: |
|
Compared 143 screenshots: 143 matched. Benchmark Results
Build and Run Timing
Detailed Performance Metrics
|
|
Compared 144 screenshots: 144 matched. |
|
Compared 181 screenshots: 181 matched. |
|
Compared 149 screenshots: 149 matched. Benchmark Results
Build and Run Timing
Detailed Performance Metrics
|
Seven review findings, all real. Register the portable transport only when loopback can actually be bound. registerIfPlatformHasNone installed it regardless, so socketTransportFactory became non-null on a platform that cannot bind, startSocketServer reported success, and the open failed later on the server's own thread where no caller could see it. The synchronous "no socket transport" refusal the message promises now actually happens. This is the same defect my own gate test tripped over -- it had to assert the wrong thing to pass. Discard a connection accepted after stop(). The listener re-checks the flag after accept returns, because stop() can be called while the thread is parked inside accept, and the connection that unblocks it belongs to a listener the caller has already abandoned. Previously that last connection was served. Close the accepted descriptor when the peer goes away. recv returning 0 or an error, and a failed send, marked the socket disconnected but left the fd open; the Java side only calls disconnectSocket while the socket still reports itself connected, so the descriptor was stranded for the life of the process. Both paths now go through one closeRawDescriptor. Suppress SIGPIPE on accepted sockets. This process installs a SIGPIPE handler (CodenameOne_GLAppDelegate), so a write to a departed peer -- the ordinary way a client disconnects mid-response -- would surface as a signal rather than the EPIPE the write path is written to handle. SO_NOSIGPIPE on the accepted fd makes it a plain error. Yield across send. accept and recv already hand the thread back to the VM; send blocks once the send buffer fills, and holding the thread there stalled cooperative scheduling for as long as the peer was slow to drain. Also drop an adverb the developer-guide Vale gate rejected. The iOS changes were clang syntax checked against the device and simulator SDKs and confirmed present in the packaged nativeios bundle, not just in the working tree. 32 MCP tests pass, including a new one covering the synchronous refusal. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
End of stream with no newline returned the partial buffer as if it were a message. For a newline-delimited protocol that is a truncated frame, which means the client went away mid-message -- so the server would parse a broken request, fail to answer it (the peer is already gone), and end its loop. That defeats the one thing this transport is built to do: stay bound so the next agent session can attach. readLine now reports end of stream regardless of how many bytes arrived first, which is the same path a clean disconnect already took. The new test was control-tested against the previous form to confirm it fails there rather than passing for unrelated reasons. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…, flaky tests attach() documented that the previous client is dropped but only overwrote the field. A reader parked in read() on the old stream is not watching that field, so a client that held its connection open and sent nothing kept the server serving a session that had already been replaced. The old streams are now closed, outside the lock, since closing is what wakes that reader. Loopback binds are named as 127.0.0.1 rather than taken from InetAddress.getLoopbackAddress(), which answers ::1 where the runtime prefers IPv6. A client connecting to 127.0.0.1 -- what adb forward and attaching agents do, and what the iOS port already binds -- would have found nothing listening while the server reported that it had started. recv and send retry on EINTR instead of treating it as a disconnect. This process installs signal handlers, so a signal arriving mid-call is ordinary here and dropping a healthy connection over one would be wrong. accept distinguishes the errnos that mean "you were asked to stop" (EBADF, EINVAL, EINTR, which is how closing the descriptor is delivered) from a real failure, so a deliberate shutdown no longer looks like a fault. The frame ceiling was off by one: a payload of exactly MAX_FRAME_BYTES was rejected before its delimiter could be read, making the largest permitted message unsendable. The ceiling is on the payload, so the test is now >. Tests: the timing-based sleeps are replaced with waits on observable conditions -- bytes consumed, end of stream reached, reader thread parked in WAITING -- which removes the flakiness and also cut this class from 1.25s to 0.26s. The new boundary test reads on a thread with a deadline: writing it as a direct call made the control run HANG rather than fail, because the regression parks the reader. And the gate test asserts its message is non-null before matching on it. Declined, with reasons in the PR: Class#newInstance cannot be replaced with getDeclaredConstructor().newInstance() because ParparVM's Class has no such method and its Constructor has only getName(), so that change would break the iOS build outright. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…exact Three from review, one of which corrects my own previous commit. accept now retries on EINTR rather than reporting it. I had listed EINTR among the errnos meaning "you were asked to stop", which was wrong: stopping is delivered by CLOSING the descriptor, and that surfaces as EBADF. EINTR is a spurious wakeup from a signal, and treating it as a shutdown would have dropped a healthy listener on a platform that installs signal handlers. Only EBADF and EINVAL mean stop now, and this matches the retry already used by recv and send. The listener loop pauses before retrying when an accept fails. A failure that recurs -- a port that cannot be bound -- had it spinning at full speed, burning a core and filling the log. Only the failure path waits; a served connection never reaches it. The frame ceiling is enforced where bytes are appended rather than at the top of the loop, so the limit is exact in both directions: a frame of precisely MAX_FRAME_BYTES is accepted and its delimiter read, and one byte beyond is refused before it is buffered. The previous form was correct about memory but allowed the buffer a byte or two past the stated ceiling, which is harder to reason about than it is worth. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
isSocketSupported() answers whether the platform CAN bind a loopback server
socket. startSocketServer can still refuse for an unrelated reason -- a release
build -- so anyone using the first as a preflight for the second would be
misled by an answer that was true and irrelevant.
Rather than folding the gate into isSocketSupported, which would conflate a
capability with a policy, the policy is now queryable on its own:
isSocketServerAllowedOnThisBuild(). startSocketServer checks both and refuses on
whichever says no first, and the capability javadoc says plainly that it ignores
the gate and points at the other half.
Also accept TIMED_WAITING alongside WAITING when waiting for the reader thread to
park. The wait is sound as it stands -- once parked the reader STAYS parked until
close() notifies it, so polling cannot miss the window, which is what separates
this from sleeping and hoping -- but a future timed wait inside the transport
would have turned it into a ten second stall instead of an immediate pass.
Declined, with reasoning in the PR: StandardCharsets in place of the "UTF-8"
literal. Core has 0 files using StandardCharsets and 55 using the literal, so
this would be the first such usage and inconsistent with the established
convention; ParparVM's StandardCharsets.UTF_8 is itself Charset.forName("UTF-8"),
so there is no lookup saved either. A sweep of all 55 would be a reasonable
separate change if the convention should move.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
close() forgot the output stream rather than closing it. A writer that had already captured it would go on writing into a session that had ended, and the underlying socket stayed open until the connection callback happened to unwind. Both streams now come out of their fields under the lock and are closed outside it, through the same closeQuietly helpers attach() uses. New test covers it, control-tested against the previous form. The guide anchor was [[Development builds only]] -- an anchor ID cannot contain spaces, so the cross-reference to it was malformed even though asciidoctor did not fail the build. Now [[development-builds-only]], referenced with explicit link text so the rendered wording is unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
isDebuggableBuild() re-read and re-parsed embedded.mobileprovision on every call. The profile is fixed for the life of the process, and this is consulted by the MCP gate and exposed publicly through Display.isDebuggableBuild(), so a caller is free to ask repeatedly -- file IO plus a plist parse each time is a poor answer to that. It is now computed once under dispatch_once. Verified by re-running the extracted-function harness: the four fixtures still classify identically (development, distribution-with-beta-reports-active, corrupt, missing), and a second harness with the once-token left intact shows the second call returning the first answer without re-reading, which is the cache doing its job. toUtf8 used buffer.toByteArray(), which copies the whole payload before decoding it -- megabytes of pointless duplication for a frame near the ceiling. ByteArrayOutputStream.toString(String) decodes straight out of the internal buffer, and ParparVM implements it as new String(buf, 0, count, charsetName), so the saving is real on device and not only on the JVM. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
buffer.toString("UTF-8") broke the build. Core is compiled with
-bootclasspath Ports/CLDC11/dist/CLDC11.jar, and CLDC11's ByteArrayOutputStream
declares only the no-argument toString(). The overload exists on the JDK and in
the ParparVM runtime, which is why a Maven build of core accepts it and the Ant
build then fails -- and why checking the ParparVM sources, as I did, was not a
check of anything core actually compiles against.
Restored the toByteArray() form with a note on the method so the next person is
not tempted by the same avoidable copy.
Also declare string.h explicitly for memset. It resolves today only because
Foundation drags it in, which is not something to rely on.
Verified this time by running the compile CI runs -- ant -buildfile
CodenameOne/build.xml jar against a locally built CLDC11 jar -- rather than
inferring from Maven, which cannot catch this class of error at all.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Closing the listening socket is how a stop is delivered, and it surfaces in the loop as a failed accept. The null-connection branch then reported that through connectionError, announcing a fault that never happened, and passed a null connection to getSocketErrorCode/Message on the way. It now breaks out when a failed accept coincides with a stop, before any callback object is built. The backoff after a genuine accept failure is slept in slices with the flag re-tested between them, so a stop arriving during the pause is acted on within 50ms rather than after the full 500. Tests: assert the exception message is non-null before matching on it, in the two places that were still calling getMessage() straight into contains() -- the same guard the refusal helper already had. And the wait helper's deadline is System.nanoTime() rather than currentTimeMillis, so an NTP step mid-wait cannot cut the wait short or stretch it. Verified with the Ant CLDC11 core compile FIRST this time, then the Maven build, 41 MCP tests and PMD. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
| private String readLine(InputStream stream) throws IOException { | ||
| ByteArrayOutputStream buffer = new ByteArrayOutputStream(); | ||
| // A carriage return is only part of the delimiter when a newline follows it, so it | ||
| // is held back one byte rather than dropped. Dropping every CR would silently edit | ||
| // a payload that happened to contain one. | ||
| boolean pendingCarriageReturn = false; | ||
| while (true) { | ||
| int b; | ||
| try { | ||
| b = stream.read(); | ||
| } catch (IOException ex) { | ||
| return null; // a read error on the client is a disconnect | ||
| } |
| static boolean hasPlatformSocketTransport() { | ||
| return socketTransportFactory != null; | ||
| } |
| public java.util.List<String> getStoppedListeners() { | ||
| return stoppedListeners; | ||
| } |
Loopback server sockets, so MCP can serve any platform
MCP's socket transport was JavaSE-only, and for a real reason: it binds a server
socket, and the portable
com.codename1.io.SocketAPI only binds the wildcardaddress — which would publish a channel that can read the screen and drive the UI
on every network interface. Attaching an agent to an app running on a device
was therefore not possible at all.
This adds the missing capability rather than working around it.
Core API
Socket.listenLoopback(port, class)andSocket.isLoopbackServerSocketSupported(),backed by a new
CodenameOneImplementation.listenSocketLoopbackhook.listenSocketand defaults tounsupported. A port has to implement loopback binding explicitly, because
answering this call with the wildcard implementation would silently widen the
reach of a channel the caller asked to keep local. There is no fallback, for
the same reason.
MCPLoopbackSocketTransportlives in core, built on that API, and is usedautomatically by any port that did not register a transport of its own. JavaSE
keeps its existing
java.netimplementation.Ports
JavaSE / Android bind
InetAddress.getLoopbackAddress(). Their server-socketcaches key loopback and wildcard sockets separately, so a port already bound to
the wildcard address can never be handed to a caller that asked for loopback.
iOS gains server sockets at all — it previously had none (
listenSocketreturned
null). The native implementation bindsINADDR_LOOPBACK, listens, andyields the VM thread across the blocking
accept. Wildcard listening staysunsupported there, so an iOS app cannot accidentally publish a port.
Accepted sockets use
recv/sendon the raw descriptor, withFIONREADforavailable input and a blocking read. The obvious implementation — wrapping the
accepted descriptor in a
CFStreampair, which is what the client-side connectpath does and is correct there — accepts connections but never delivers data: a
server socket is read on a background thread whose run loop never runs. The
client path is untouched.
Tests
MCPLoopbackSocketTransportTestcovers framing and session handling: framedelimiting independent of packet boundaries, CRLF tolerance, write with no client
attached, reconnect without ending the server, and close waking a pending read.
Verified end to end on the iOS simulator: an agent attaches to a running app,
reads its accessibility tree, and drives it through several screens.
No existing public API changed.
🤖 Generated with Claude Code