Report the real port status, and fix the desktop-port defects it was hiding - #5501
Open
shai-almog wants to merge 44 commits into
Open
Report the real port status, and fix the desktop-port defects it was hiding#5501shai-almog wants to merge 44 commits into
shai-almog wants to merge 44 commits into
Conversation
…hiding The Port Status table showed almost every column as partial, skipped or stale. Most of that was the reporting pipeline, not the ports. Reporting - scripts/website/sync_port_status_reports.sh re-implemented the publish rule as a jq expression that demanded a measured duration for all ten performance workloads. iOS, tvOS and watchOS legitimately skip the three GC-footprint workloads on the simulator, so every fresh Apple report was rejected, the site served the checked-in fallback, and after fourteen days those four columns rendered as stale. The rule now lives once in port_status.py (publishable_report_problems + the "accept" subcommand) next to the normalizer whose own tests already covered skipped workloads. Contract drift keeps the fallback with a warning; a malformed report fails the website build instead of quietly degrading. - port-status-publish.yml only publishes when a workflow_run event reaches it, and those events never arrive for the Linux and Windows suites: the data branch has never held a linux or windows-x64 report. The nightly now runs backfill_port_status.sh, which publishes from the newest master run of every producing workflow and fails when a port has no report inside the contract's staleness window. - Only the Android pipeline failed on a failing compliance test. iOS, macOS and JavaScript now do too; all three are at zero failures, so this is a ratchet rather than a new red. - A skip the errata account for by name renders as a pass with a marked note instead of a partial, and the page validator refuses a noted cell whose test the errata do not cover. A run that stopped early no longer withdraws the result of a feature whose every mapped test reported back. - The checked-in reports are refreshed, including the real (failing) Linux and Windows results, so the fallback states what those ports actually do. Desktop ports - java.time asked the host for the rules of a fixed offset by handing "GMT-05:00" to the platform time zone database. POSIX inverts the sign of a TZ offset and the Windows CRT cannot parse the form at all, so every OffsetDateTime formatted through a pattern came out shifted by twice its offset. Custom GMT/UTC IDs are now resolved in Java, and a ZoneOffset never reaches the host database. - Offsets now come from TimeZone.getOffset rather than from Calendar, which reconstructs local time from a raw offset plus a flat one-hour daylight guess. - Character.getType threw UnsupportedOperationException, which meant isLetter, isLetterOrDigit and isJavaIdentifierStart/Part threw for every input on these ports. ASCII now has a category table and the rest answers from the primitives this runtime implements. - openInputStream returned a stream wrapping a null file handle for a missing path, so callers could not tell a missing file from an empty one (issue #1502 on both desktop ports); openOutputStream and the storage streams silently discarded writes the same way. - The Linux port carried the Windows port's backslash path join, so its staged-resource fallback never resolved. - CameraApiTest treated the native Linux port as having a headless camera. It drives real V4L2 devices through GStreamer, which a hosted runner does not have, so it now skips with a stated reason like the other native ports. - The Linux capture harness accepts CN1_REQUIRE_SUITE, so it can demand the suite's own completion marker rather than stopping when screenshots go quiet while trailing tests are still queued. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: aad27d0718
ℹ️ 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".
Contributor
✅ Continuous Quality ReportTest & Coverage
Static Analysis
Generated automatically by the PR CI workflow. |
Contributor
✅ ByteCodeTranslator Quality ReportTest & Coverage
Benchmark Results
Static Analysis
Generated automatically by the PR CI workflow. |
Collaborator
Author
|
Compared 146 screenshots: 146 matched. Benchmark ResultsDetailed Performance Metrics
|
Collaborator
Author
|
Compared 151 screenshots: 151 matched. Native Android coverage
✅ Native Android screenshot tests passed. Native Android coverage
Benchmark ResultsDetailed Performance Metrics
|
Collaborator
Author
|
Compared 146 screenshots: 146 matched. Benchmark ResultsDetailed Performance Metrics
|
Collaborator
Author
|
Compared 148 screenshots: 148 matched. |
Collaborator
Author
|
Compared 148 screenshots: 148 matched. |
Collaborator
Author
|
Compared 144 screenshots: 144 matched. |
Collaborator
Author
|
Compared 217 screenshots: 217 matched. |
Collaborator
Author
|
Compared 146 screenshots: 146 matched. Benchmark ResultsDetailed Performance Metrics
|
Collaborator
Author
|
Compared 143 screenshots: 143 matched. Benchmark Results
Build and Run Timing
Detailed Performance Metrics
|
Collaborator
Author
|
Compared 181 screenshots: 181 matched. |
Collaborator
Author
|
Compared 149 screenshots: 149 matched. Benchmark Results
Build and Run Timing
Detailed Performance Metrics
|
…native semantics - backfill_port_status.sh published whatever the newest run produced. A report built against an older contract passes the freshness check but is rejected by the website sync, so the column would stay on its stale fallback while the sweep reported success. Each artifact now goes through "port_status.py accept" before publication, and the closing assertion re-checks the published file instead of only its timestamp -- which is how windows-arm64's drifted report now surfaces. - CN1_REQUIRE_SUITE is now set by both Linux legs. Left unset, the new branch in the capture harness was unreachable and both jobs kept the screenshot stabilization exit that kills the suite while DesktopMode, the VideoIO grid, the VR scene and the 360 panorama are still queued. - The shared offset lookup passes UTC fields. The POSIX native resolves them with timegm and the JavaScript runtime with Date.UTC, but the iOS native built its NSDate from [NSCalendar currentCalendar], reading them in the device's zone; near a transition that lands on the wrong side of it. It now builds the date in UTC, and no longer drops the hour and second components. - Character.getType collapsed every non-ASCII whitespace code point to SPACE_SEPARATOR. U+2028 and U+2029 are LINE_SEPARATOR and PARAGRAPH_SEPARATOR and U+180E is FORMAT, all of which isWhitespace already treats individually. - The page validator matched the note marker with quoted attributes, which the production build minifies away, so the check failed on CI and passed locally. - java.time/DateTimeSupport.java carries the project header; java.util.TimeZone keeps its Apache Harmony notice and is recorded in the exclusions list. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3c58dcc345
ℹ️ 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".
Contributor
Cloudflare Preview
|
The first CI run on this branch confirmed the time zone, Character.getType and openInputStream fixes -- TimeApiTest, SurfacesPublishTest and FileSystemStorageOpenInputStreamMissingTest all pass on Linux now -- and the new exceptions turned two silent write failures into named ones. - getAppHomePath() returned a bare path on both desktop ports. Android and iOS return it with the file:// scheme, and com.codename1.io.File prepends the app home to any path that lacks the scheme, so new File(fs.getAppHomePath() + "x") resolved to the home directory joined to itself: AudioMixerApiTest was asking to write ".../codenameone//home/runner/.local/share/codenameone/audio-mixer-api-test.wav". Both ports now return the scheme and implement toNativePath. - The Windows port never overrode getAppHomePath at all, so it inherited listFilesystemRoots()[0] + AppName, which is a drive root plus the literal string "null" when no app name is set. It now anchors on the same per-user storage directory the Linux port uses. - cn1StorageDir() created only the leaf directory. A home without an existing ~/.local/share -- a fresh CI runner, or a new account -- left the storage directory absent, so every write into it failed at fopen(); that is why ClipboardRoundTripTest could not create its file. The path is now created component by component. - Both ports record why the last open failed and include it in the exception, so a missing directory is distinguishable from a permission or sharing problem without another CI round trip. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a2cb05fce5
ℹ️ 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".
Collaborator
Author
|
Compared 148 screenshots: 148 matched. Benchmark Results
Detailed Performance Metrics
|
…misuse
Crypto
- The Linux port answers the whole com.codename1.security surface through
OpenSSL's EVP layer: secure random, AES in GCM/CBC/ECB, RSA with OAEP or
PKCS#1, SHA-2 signatures and RSA key generation. Keys cross the boundary in
the encodings the portable API documents -- X.509 SubjectPublicKeyInfo and
PKCS#8 PrivateKeyInfo -- so d2i_PUBKEY and d2i_PKCS8_PRIV_KEY_INFO do the
ASN.1 and nothing parses DER by hand. libcrypto comes with the libcurl the
port already links.
- The Windows port answers the same surface through CNG, with crypt32 doing
the ASN.1 between those DER encodings and BCRYPT_RSAKEY_BLOB.
- A failed operation raises rather than returning an empty array: an
authentication failure that answered "no bytes" would read as a successful
decryption of nothing. GCM keeps the tag appended to the ciphertext, which
is the convention the portable API documents.
- The OpenSSL implementation was exercised against libcrypto off-device
before landing: GCM round trip, tamper and wrong-AAD rejection, CBC with
padding, OAEP round trip, and sign/verify including tampered-data and
wrong-key rejection.
stringToUTF8 aliasing
stringToUTF8 returns one buffer per thread and overwrites it on every call,
so a native that converted a second String silently repointed the first
result at the second string. Five natives in the Linux port did exactly
that:
- fileRename renamed a file onto itself, which is why WAVWriter's rename
step left AudioMixerApiTest without its output;
- httpSetHeader sent every request header as "value: value";
- printDocument, showNotification and shareText each collapsed their
arguments onto the last one.
They now copy through cn1LinuxJStrDup, which the header documents as
mandatory for any native converting more than one String. The Windows port
was already safe -- its wide-string helper allocates.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8dd4c6a954
ℹ️ 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".
…ietly A test that blocks the event dispatch thread outright can never be timed out by the runner, because the per-test deadline is itself enforced by an EDT callback. The suite simply stopped: the log ended mid-line, every later test was published as "never run", and nothing said which test was responsible -- that is how Media360PanoramaScreenshotTest on Linux and CalendarApiTest on Windows have been going unattributed. A watchdog thread now tracks the running test and, thirty seconds past its deadline, reports it by name, emits a CN1SS:SUITE:WEDGED marker and exits. The normalized report then records that test as failed and the rest as unreached, which is what actually happened, and the harness stops burning its forty-minute cap on a thread that is not coming back. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The shared POSIX implementation sets TZ and reads tm_gmtoff back. Neither half exists on Windows: the Microsoft C runtime only parses the "EST5EDT" form of TZ, not an IANA identifier, and its struct tm carries no GMT offset at all. Every named zone therefore resolved to an offset of zero, which is why TimeApiTest read America/New_York as UTC. Windows has shipped ICU since Windows 10 1703, and its calendar speaks IANA identifiers and knows the daylight rules for the instant being asked about. The three time zone natives now go through it and fall back to the previous behaviour if it is unavailable. The POSIX path is untouched and still passes the same probe through the ParparVM clean target. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0c6c9701c8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The watchdog reached for Runtime.exit, which the bytecode compliance gate rejects along with System.exit -- both are outside the API the ports support, and exitApplication would have to run on the very thread that is stuck. That broke the suite build, and with it every job that compiles the suite. The watchdog now only reports: it names the test and emits CN1SS:SUITE:WEDGED. Both capture harnesses watch for that marker, stop waiting as soon as it appears and fail with the test name, which is ordinary JUnit code under no such restriction. It also starts through Display.startThread rather than configuring a raw Thread, and stops itself once the suite finishes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 19255ebe82
ℹ️ 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".
Crypto - The RNG now fails closed. RAND_bytes and BCryptGenRandom report failure to Java, which throws, instead of leaving a zeroed buffer that KeyGenerator would hand out as a key. - OAEP masks with SHA-1 even when the digest is SHA-256, matching the JCE providers behind the JavaSE and Android ports. Naming the digest for both halves made anything sealed on a desktop port undecryptable elsewhere. - Initialization vectors are checked before they reach the platform library: a missing GCM nonce silently repeated across messages under one key, and a short CBC IV was read as a whole block past the Java array. - Windows imports private keys through NCrypt, which takes PKCS#8 for both RSA and EC, so the ECDSA signature APIs work instead of decoding every key as RSA. Sign and verify pick their padding from the key's own algorithm. Time zones - Custom IDs split their last two digits as minutes for the three-digit form too, so GMT+012 is UTC+00:12 rather than UTC+12. - The Windows raw offset samples the current year and prefers the later standard-time reading. A zone whose base offset changed mid-year with neither sample flagged as daylight saving -- Asia/Almaty in 2024 -- would otherwise report its retired offset forever. - The UWP native reads its fields as UTC like the POSIX, JavaScript and iOS implementations, rather than as host-local time. Port status - A report whose generated_at cannot be parsed is unusable rather than publishable; it would otherwise poison the sweep and the page's own rendering. - The sweep merges artifacts across candidate runs until every port a workflow owns is covered, so one failed matrix leg no longer hides the others, and compares exact elapsed seconds rather than whole days. - A feature whose tests all passed or were documented skips keeps its noted pass even when the suite run stopped early; the completion fallback now runs after that case rather than before it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 902b535c80
ℹ️ 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".
Contributor
There was a problem hiding this comment.
🟡 Not ready to approve
There is at least one confirmed runtime bug (null TimeZone.getTimeZone() can NPE) plus a concrete error-reporting issue in Linux native I/O that should be corrected before merging.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
Suppressed comments (2)
vm/JavaAPI/src/java/util/TimeZone.java:175
getTimeZone()can be called with a null ID (JDK returns GMT in that case). After this change,customTimeZone(null)returns null but the subsequentID.equalsIgnoreCase(...)will throwNullPointerException, which would break callers that pass null defensively.
Ports/LinuxPort/nativeSources/cn1_linux_io.c:182- When
pathis null,fileOpenRead()/fileOpenWrite()callcn1RecordIoError(p)withp==0, buterrnois not defined in that case. This can makelastIoError()report an unrelated/previous errno (or even "Success"), which is misleading in the Java exception messages.
const char* p = cn1JStr(threadStateData, path);
FILE* f = p ? fopen(p, "rb") : 0;
if (f == 0) {
cn1RecordIoError(p);
}
- Files reviewed: 71/73 changed files
- Comments generated: 0 new
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 63a2a31ce3
ℹ️ 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".
…stale Every "the Linux suite hangs in X" reading in this branch has been wrong, and this is why. C streams block-buffer when they are not a tty, so an app logging into a pipe -- which is how CI captures it -- emits in 4KB chunks. When the harness kills a run mid-flight the captured log ends wherever the last chunk happened to flush, thousands of lines behind the process. Every diagnosis made from that tail named the wrong place: Base64NativePerformanceTest, then MutableImageReadbackTest, and the "missing" marker in the latest run is stage=prepared, whose implementation is an empty method that cannot block. The generated main now sets _IOLBF on stdout and stderr, so the log says where the process actually is. One flush per line, in return for diagnostics that mean what they say. This also revises the previous commit's conclusion: raising the budget to 70 minutes did not move the stopping point, so the suite is not merely slow. What it is doing will only be answerable from a log that is not lagging, which is what this provides. Verified through the clean target: the generated main compiles with the setvbuf calls and the probe still passes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Build Android has failed intermittently across JDK legs with nothing but "A failure occurred while executing PackageAndroidArtifact$IncrementalSplitterRunnable" and no cause, and it passes on a re-run, so there has been nothing to act on. --stacktrace costs nothing on a successful build and prints the actual exception the next time it happens. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🟡 Human review recommended
It spans CI/workflows, website publishing/validation, runtime Java APIs, and native implementations across multiple ports, so it needs careful human verification across platforms.
Review details
Suppressed comments (1)
vm/JavaAPI/src/java/util/TimeZone.java:176
getTimeZone()will throw a NullPointerException when called withID == nullbecause it unconditionally evaluatesID.equalsIgnoreCase(getTimezoneId())after the custom/GMT checks. Since the method already guardsequalsIgnoreCase("gmt")withID != null, it looks like null is meant to be handled gracefully here too.
- Files reviewed: 73/75 changed files
- Comments generated: 0 new
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7d0ad8cf43
ℹ️ 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".
Contributor
There was a problem hiding this comment.
🟡 Human review recommended
It touches multiple cross-platform runtimes, native layers, CI workflows, and website publication logic, so it needs careful human validation across the affected ports and pipelines.
Review details
Suppressed comments (1)
Ports/LinuxPort/src/com/codename1/impl/linux/LinuxBrowserComponent.java:41
- The header comment says the Linux BrowserComponent peer is rendered from a cached PNG captured after each navigation, but the native implementation currently stubs browserCapturePng() to return null (pending an async WebKit snapshot bridge). This makes the comment misleading and contradicts the current behavior (and the screenshot-test expectations).
/// Native Linux BrowserComponent peer backed by a WebKitGTK WebView (the native
/// lifecycle lives in cn1_linux_browser.c). The component is rendered from a
/// cached image: the native side captures the view to PNG bytes after each
/// navigation, which `generatePeerImage()` turns into the peer image that
/// `PeerComponent.paint()` draws, so it appears in the offscreen screenshot
- Files reviewed: 73/75 changed files
- Comments generated: 0 new
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
My previous commit made every Windows clean-target binary die on its first instruction. Exit code -1073740791 is 0xC0000409, the MSVC fail-fast: the CRT rejects setvbuf(stream, NULL, _IOLBF, 0) because a buffered mode demands a size of at least 2, and it answers an invalid parameter by killing the process rather than returning non-zero. That took clean-target on both architectures, the Windows capture and the cross-built exe run. _IONBF ignores the size argument, is valid on every CRT, and is what the diagnostics actually want -- unbuffered rather than merely line-buffered. Verified this time against the real thing rather than assuming: the exact two calls compile and link into a PE with clang-cl against the xwin MSVC CRT headers, and the clean target still builds and runs through the probe on this machine. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🟡 Human review recommended
It combines significant cross-platform runtime/native changes with CI/reporting pipeline and workflow modifications, which warrants full human review despite only minor actionable nits found.
Review details
Suppressed comments (2)
scripts/hellocodenameone/conformance/backfill_port_status.sh:229
- Same portability issue as above:
base64 --decodewon’t work on macOS’ BSD base64. Using a small fallback here prevents the final freshness assertion from failing purely due to platform differences when running this script locally.
scripts/hellocodenameone/conformance/backfill_port_status.sh:203 base64 --decodeis GNU-specific; this script already includes a BSDdatefallback, so it appears intended to run on macOS too. Consider decoding with a portable fallback (e.g., try--decodethen-D) sogh api ... | base64 ...works on both GNU coreutils and BSD base64.
This issue also appears on line 227 of the same file.
- Files reviewed: 73/75 changed files
- Comments generated: 0 new
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
I raised the harness cap from 40 to 70 minutes and the job timeout from 90 to 130 on the theory that the Linux suite was merely slow. The very next run used 78 minutes and stopped at the same test, which disproved it. I should have reverted then instead of leaving a 130-minute timeout in the workflow. Both are back to 40 and 90. A longer timeout does not fix anything here, it just makes every future run of this job slower to fail and hides how long the suite really takes. The suite not finishing is still unexplained. What changed for real is that the generated main is now unbuffered, so the next Linux log will show where the process actually is rather than a stale chunk -- which is the evidence the budget theory was invented in the absence of. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sampling only at the 40-minute cap is why the stacks were uninformative: by then every thread is idle and the EDT is parked in mainEDTLoop, which is what "nothing queued" looks like and says nothing about how it got there. The harness now tracks when the app last produced any output and takes a gdb thread dump after two minutes of silence, up to six times across a run, each sample separated by a timestamped header in hang-stacks.txt. Two minutes is far longer than the gap between any two tests in a healthy run, so a healthy run never triggers it; a stalled one gets photographed repeatedly while it is stuck and the samples show whether it is frozen on one call or crawling through something. Paired with the unbuffered stdout from the previous commits, this is the first setup that can actually answer the question instead of inviting another theory. Kept regardless of what it finds: it costs nothing on a healthy run and this job has a history of failing with no evidence. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🟡 Not ready to approve
vm/JavaAPI/src/java/util/TimeZone.java can still throw a NullPointerException in getTimeZone() for null IDs, which is a correctness issue in a core API surface.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
Suppressed comments (1)
vm/JavaAPI/src/java/util/TimeZone.java:175
TimeZone.getTimeZone()will throw a NullPointerException when called with a null ID because it unconditionally evaluatesID.equalsIgnoreCase(getTimezoneId()). The JDK contract is to return GMT for unknown/invalid IDs (andnullshould not crash callers), so this should be handled explicitly before anyequalsIgnoreCase()calls.
- Files reviewed: 73/75 changed files
- Comments generated: 0 new
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
Six dumps appended to one file are unreadable without knowing where each begins, and the timestamps are what show whether the process moved between samples. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🟡 Human review recommended
It touches many cross-cutting areas (CI publishing, website generation/validation, multi-platform runtime + native code paths), so it warrants final human review and validation across affected targets.
Review details
- Files reviewed: 73/75 changed files
- Comments generated: 0 new
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: af9944c375
ℹ️ 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".
…rs, MCP listeners Windows OAEP was capped at about 8456-bit keys The mask was a fixed 1024-byte array, so a larger key was refused as though the block did not fit it. KeyGenerator.rsa() accepts every byte-aligned size from 1024 bits up and callers can import larger ones, so keys that work on every other port failed here. The mask is now allocated from the modulus and freed on each exit. Checked under AddressSanitizer at 8456 and 12288 bits -- both previously refused, both now round-trip, no leak on any path. Linux accepted DER keys with trailing data d2i_* stops at the end of the first object it recognises, so a buffer holding a valid key followed by extra bytes parsed happily. JavaSE and Android reject that through KeyFactory, so the same bytes validated on one port and not another. Both parsers now require the whole input to be the key. verify() could not tell a bad signature from a bad configuration An unsupported algorithm, malformed key DER or family mismatch came back as plain false, which reads as "this signature was tampered with". JavaSE and Android throw, and Signature.verify turns that into a CryptoException. Both ports gained clearCryptoError() so the slot can be emptied before the call, which is what makes a recorded failure attributable to it, and a configuration error now raises instead of returning false. A retired MCP listener could serve a replacement transport close() released the process-wide registration before stopping the listener, so a connection already accepted could resolve `active` to a transport that took the slot afterwards and hand it a client that dialled the old port. The listener is stopped first now, and attach() refuses streams once the transport is closed rather than wiring them to a dead session. Also the no-video-encoder-on- skip reason, scoped to the Apple ports like its siblings. core-unittests: 4678 tests, no failures, SpotBugs zero findings. Windows cross-compile links. Both port sources compile. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🟡 Not ready to approve
vm/JavaAPI/src/java/util/TimeZone.java can still throw a NullPointerException in getTimeZone() when called with a null ID, and the fix is localized.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
Suppressed comments (1)
vm/JavaAPI/src/java/util/TimeZone.java:171
getTimeZone()callsID.equalsIgnoreCase(...)even whenIDmay be null, which will throw a NullPointerException (and would also lead to creating a TimeZone with a null ID). SincecustomTimeZone()already tolerates null, it’s cheap to make the public entrypoint null-safe too (matching typicalTimeZone.getTimeZone()behavior).
- Files reviewed: 73/75 changed files
- Comments generated: 0 new
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1092abd0bf
ℹ️ 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".
| # could be force-killed with trailing tests unrun and nothing failed. | ||
| # The Windows pipeline passes a real boolean, which is why its gate | ||
| # works. 'true' arms it here. | ||
| CN1_REQUIRE_SUITE: 'true' |
There was a problem hiding this comment.
When either Linux matrix leg reaches the harness deadline without CN1SS:SUITE:FINISHED, this newly armed setting makes CleanTargetLinuxIntegrationTest fail even if it produced usable logs and screenshots. The compare-comment job in linux-build-run.yml depends on build-run but lacks an always() status check, so GitHub skips the job on that failure; consequently cn1ss_process_and_report and the Upload Linux port status step never run, and the public table retains the previous report instead of publishing the current failure/not-run results. Allow the normalization job to run after failed producer legs while preserving the overall workflow failure.
Useful? React with 👍 / 👎.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
The Port Status page reported almost every port as partial, skipped or stale. Most of that was the reporting pipeline lying about healthy ports; the rest was real defects in the desktop ports that nothing was failing on.
Before / after, from a local build of the page:
Reporting pipeline
Every fresh Apple report was being thrown away.
sync_port_status_reports.shre-implemented the publish rule as a jq expression demanding a measured duration for all ten performance workloads. iOS, tvOS and watchOS legitimately skip the three GC-footprint workloads on the simulator (performance.skipped, which the normalizer supports and has a unit test for), so each night's report was rejected, the site fell back to the checked-in copy, and after fourteen days those four columns went stale.The rule now lives once, in
port_status.py(publishable_report_problems+ anacceptsubcommand the shell calls), beside the normalizer whose tests already covered skipped workloads. Contract drift — a report produced before a newly registered test — keeps the fallback with a warning; a malformed report now fails the website build instead of degrading quietly. Six new tests cover it, including one asserting every checked-in report is itself publishable.Linux and Windows have never published a report.
port-status-publish.ymlreacts toworkflow_run, and those events are not delivered for those two producers — Android/iOS/macOS/JavaScript get a publish run about three seconds after their run completes, Linux and Windows get none, and the data branch has nolinux-*orwindows-x64file in its entire history. Rather than keep guessing at that, the nightly now runsbackfill_port_status.sh: it publishes from the newest completed master run of every producing workflow and then fails if any port lacks a report inside the contract's staleness window, so a silent producer is a red workflow instead of a grey column.Nothing failed when a compliance test failed. Only the Android pipeline set
CN1SS_FAIL_ON_TEST_PROBLEMS. iOS (4 legs), macOS and JavaScript now set it too — all three are at zero failures and zero never-run tests today, so it is a ratchet, not a new red.Presentation. A skip the errata account for by name now renders as a pass with a marked note (
✓*) and its own legend entry, and the page validator refuses a noted cell whose test the errata do not cover. A run that stopped early no longer withdraws the result of a feature whose every mapped test reported back — the unreached tests still show as not-run and the port card still says the run was incomplete.Desktop port defects
Each of these was reproduced against the ParparVM clean target before and after the fix.
java.timeconverted aZoneOffsetto"GMT-05:00"and handed it to the platform. POSIX inverts the sign of aTZoffset, and the Windows CRT cannot parse the form at all, so everyOffsetDateTimeformatted through a pattern came out shifted by twice its offset —2020-03-08T01:30-05:00printed as11:30-05:00. CustomGMT/UTC/UTIDs are now resolved in Java, and aZoneOffsetnever reaches the host database. All ofTimeApiTest's zone assertions pass locally now, including both DST transitions.Calendar, which rebuilds local time from a raw offset plus a flat one-hour daylight guess. They now come fromTimeZone.getOffset, which consults the platform's real rules.Character.getTypethrewUnsupportedOperationException, soisLetter,isLetterOrDigit,isJavaIdentifierStartandisJavaIdentifierPartthrew for every input on these ports. ASCII now has a category table; above it the answer comes from the primitives this runtime actually implements.openInputStreamreturned a stream over a null file handle for a missing path, so a caller could not distinguish a missing file from an empty one — issue on iOS openInputStream for non-existent file do not throw FileNotFoundException #1502's exact defect, on both desktop ports.openOutputStreamand the storage streams discarded writes the same way.CameraApiTestassumed the native Linux port had a headless camera. It drives real V4L2 devices through GStreamer, which a hosted runner does not have, so it now skips with a stated reason like the other native ports; the existing camera erratum covers it.CN1_REQUIRE_SUITE, so it can demand the suite's own completion marker instead of stopping once screenshots go quiet — which it did while DesktopMode, the VideoIO grid, the VR scene and the 360 panorama were still queued, leaving every trailing test reported as never run.Crypto (added after the first review round)
Both desktop ports now answer the whole
com.codename1.securitysurface, whichCryptoApiTestwas failing at its first call:libcryptocomes with thelibcurlthe port already links. The implementation was exercised against reallibcryptooff-device before landing: GCM round trip, tamper and wrong-AAD rejection, CBC with padding, OAEP round trip, and sign/verify including tampered-data and wrong-key rejection.BCRYPT_RSAKEY_BLOB.The shared UTF-8 buffer
stringToUTF8returns one buffer per thread and overwrites it on every call, so a native that converted a second String silently repointed the first result at the second string. Five natives in the Linux port did exactly that:fileRenamerenamed a file onto itself — which is whyWAVWriter's rename step leftAudioMixerApiTestwith no output;httpSetHeadersent every request header as"value: value";printDocument,showNotificationandshareTexteach collapsed their arguments onto the last one.They now copy through
cn1LinuxJStrDup, documented in the header as mandatory for any native converting more than one String. The Windows port was already safe — its wide-string helper allocates.Still red after this change
Stated plainly rather than hidden:
Media360PanoramaScreenshotTeston Linux andCalendarApiTeston Windows. The runner's per-test deadline is enforced by an EDT callback, so a test that blocks the thread outright could never be timed out: the suite just stopped, mid-log-line, and every later test was published as "never run" with nothing naming the cause. A watchdog thread now reports the responsible test and exits, so the report attributes the failure instead of silently truncating. Fixing the two tests themselves needs profiling on those platforms — the Linux one is most likely the software Mesa rasterizer taking minutes over a stereo 360 sphere.java.util.Calendarstill mishandles daylight saving on these ports.java.timeno longer depends on it; the legacy class is a separate fix.Verification
port_status.py validateand the 16 normalizer tests pass.validate_port_status.mjspasses against the generated page.backfill_port_status.shwas dry-run against the live GitHub API: it selects the newest master run for all seven producing workflows and would publish the seven stale or missing reports.java.time,TimeZoneandCharacterfixes were each verified by building and running a probe through the ParparVM clean target.🤖 Generated with Claude Code