Repository navigation
Initializr: show next steps after the download, and report build progress - #5954
Conversation
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 110fb2b1a8
ℹ️ 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".
| template: template ? String(template) : "", | ||
| ide: ide ? String(ide) : "" | ||
| }, "*"); | ||
| callback.complete(true); |
There was a problem hiding this comment.
When the Initializr is embedded on localhost or a PR preview, when the beacon script is unavailable, or when fetch/WebCrypto is unsupported, postMessage still succeeds and this immediately reports success. The parent either drops the request or sendSteps() resolves false, but its result is ignored, so createEmailStepsRow() disables the form and says “Check your inbox” even though no email was requested. The bridge needs an acknowledgment from the host, or it must verify that the host can send before returning true.
Useful? React with 👍 / 👎.
Cloudflare Preview
|
…PowerShell in build.bat Review (#5954): requestSteps answered true as soon as postMessage returned, but on localhost, a PR preview, a page without the beacon, or a browser without fetch/WebCrypto the host drops the request and the panel still said "Check your inbox". The request now carries an id and the host page answers cn1-initializr-steps-result with whether the beacon actually issued it; no answer within 6 s counts as not sent. A fetch that rejects now resolves false. The Java side asks off the EDT and re-enables the form on failure. windows-latest launcher job hung until cancelled: build.bat timed the build with a PowerShell child, which can wait on stdin forever when the launcher's input is redirected -- as in CI, and in IDE run configurations, where it would hang a real user's build. Duration now comes from %TIME% in plain cmd; a test fails if any non-comment line of the generated build.bat runs PowerShell. Copyright headers added to the four files this PR touched that had none. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…build progress A visitor who downloaded a project saw nothing happen: no hint of what to open, which command starts the first build, or that the first cloud build asks for an account -- and between "downloaded" and "first cloud build" we were blind. Next-steps panel: after Generate Project succeeds (GeneratorModel.generate() now returns whether the download was handed over), a card at the top of the column -- clear of the Crisp widget at the bottom-right -- gives three steps worded for the chosen IDE and build tool, the README's own commands for macOS/Linux and Windows, the account line, and an optional "Email me these steps" that posts cn1-initializr-steps-request to the host page through the new WebsiteThemeNative.requestSteps. The website forwards it with cn1InitializrBeacon.sendSteps to /api/v2/funnel/initializr-steps (same host allow-list, hashed package, no cookies). Launcher reporting: Initializr Maven downloads (not the archetype) get build.sh/build.bat that report step=launch/exit with target, os, java, exit, reason and secs to /api/v2/funnel/initializr-event, keyed by a SHA-256 of the lower-cased package (Sha256.java, plain Java for the JS port) -- never the name. Modeled on BuildCloud's starter launchers: CN1_TELEMETRY=0 opts out, curl -m 3 and failures ignored, Ctrl+C reports interrupted. The hooks are injected into common.zip's launchers at generation time, so archetype parity is untouched; an unrecognised launcher ships unmodified. README gains a "Build progress reporting" section and the account sentence. test-starter-launchers.py points CN1_EVENTS_URL at a closed local port so CI never posts to production. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
ideId() lower-cased the enum name, so VS_CODE became "vs_code", which BuildCloud's initializr-steps endpoint does not recognise -- a VS Code user's email would lose its IDE-specific step. Test pins every IDE's id. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…PowerShell in build.bat Review (#5954): requestSteps answered true as soon as postMessage returned, but on localhost, a PR preview, a page without the beacon, or a browser without fetch/WebCrypto the host drops the request and the panel still said "Check your inbox". The request now carries an id and the host page answers cn1-initializr-steps-result with whether the beacon actually issued it; no answer within 6 s counts as not sent. A fetch that rejects now resolves false. The Java side asks off the EDT and re-enables the form on failure. windows-latest launcher job hung until cancelled: build.bat timed the build with a PowerShell child, which can wait on stdin forever when the launcher's input is redirected -- as in CI, and in IDE run configurations, where it would hang a real user's build. Duration now comes from %TIME% in plain cmd; a test fails if any non-comment line of the generated build.bat runs PowerShell. Copyright headers added to the four files this PR touched that had none. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
eed1730 to
32bffd6
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 32bffd6ed5
ℹ️ 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".
| cn1_event --data-urlencode "step=exit" --data-urlencode "target=$target" \ | ||
| --data-urlencode "os=$os" --data-urlencode "java=$java" --data-urlencode "exit=130" \ | ||
| --data-urlencode "reason=interrupted" --data-urlencode "secs=$(( $(date +%s) - start ))"; \ | ||
| rm -f "$log" "$pidfile"; trap - INT TERM; exit 130' INT TERM |
There was a problem hiding this comment.
When CI, an IDE, or a timeout terminates the launcher with SIGTERM, this shared trap reports exit=130 and exits 130, which is the conventional status for SIGINT; a terminated build should preserve/report 143 (128 + SIGTERM). This both corrupts the new telemetry's exit-code data and prevents callers from distinguishing cancellation by TERM from Ctrl+C, so INT and TERM need separate status handling.
Useful? React with 👍 / 👎.
| email: String(email), | ||
| packageName: packageName ? String(packageName) : "", | ||
| template: template ? String(template) : "", | ||
| ide: ide ? String(ide) : "" | ||
| }, "*"); |
There was a problem hiding this comment.
When a third-party page embeds the directly reachable /initializr-app/index.html, the email form remains available and this wildcard postMessage sends the visitor's plaintext address to that untrusted parent. The Cloudflare Pages output in this repository has no framing restriction, so an arbitrary site can display the genuine Initializr UI and collect every submitted address; verify an allow-listed parent origin (and use it as targetOrigin) before showing or sending this request.
Useful? React with 👍 / 👎.
| rm -f "$log" "$pidfile"; trap - INT TERM; exit 130' INT TERM | ||
| set +e | ||
| set -o pipefail | ||
| { ./mvnw "$@" <&0 2>&1 & echo $! > "$pidfile"; wait $! 2>/dev/null; } | tee "$log" & |
There was a problem hiding this comment.
When the temporary log cannot be opened or fills during a build, tee returns nonzero (and may close the pipe while Maven is writing); because pipefail is enabled, that status becomes the launcher's exit code even if Maven succeeded. This also happens with CN1_TELEMETRY=0, so the optional reporting path can turn an otherwise successful build into a failure; disable reporting when the log is unavailable and preserve Maven's status independently of tee.
Useful? React with 👍 / 👎.
| new Thread(() -> { | ||
| boolean sent; | ||
| try { | ||
| sent = bridge.requestSteps(address, model.getPackageName(), model.templateId(), model.ideId()); |
There was a problem hiding this comment.
For Gradle and backend-only downloads, nextSteps() produces different commands and even different account requirements, but this request sends only package, template, and IDE. Those fields can be identical for a Maven app, a Gradle app, and a backend-only project, so the email endpoint cannot reconstruct the steps the panel promised to email and must return incorrect instructions for at least some selections; include the build tool and project type (or the resolved steps) in the request.
Useful? React with 👍 / 👎.
…osed port windows-latest hit its ten-minute limit: the harness pointed Initializr launchers at 127.0.0.1:9, and on Windows a refused localhost connection is retried for about two seconds, twice per build -- each fixture went from ~8 s to ~33 s. A local recorder now answers 204 at once, which also lets the test check the reports: Initializr launchers send launch and exit (the failing build's exit code included) keyed by the 64-hex package hash; the archetype's own launchers send nothing. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 57b4311ed0
ℹ️ 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".
| .append("version, the exit code, a one-word failure reason and the duration, to ") | ||
| .append(LAUNCHER_EVENTS_URL).append(". Never your code, paths, user name or build output.\n\n") | ||
| .append("To opt out, set `CN1_TELEMETRY=0` in your environment ") | ||
| .append("(`export CN1_TELEMETRY=0`, or `set CN1_TELEMETRY=0` on Windows).\n\n"); |
There was a problem hiding this comment.
When a Windows user follows this README from PowerShell—which the generated Getting Started section explicitly supports—set CN1_TELEMETRY=0 does not set an environment variable inherited by build.bat, so telemetry remains enabled despite the attempted opt-out. Microsoft documents PowerShell environment assignment as $Env:<variable-name> = "<new-value>"; list that command separately from the existing Command Prompt syntax. Microsoft PowerShell environment-variable documentation
Useful? React with 👍 / 👎.
…; launcher exit fixes Review (#5954): - P1: a third-party page could frame the public /initializr-app/ and receive every typed address via postMessage("*"). The bridge now only trusts a same-origin parent on www.codenameone.com / codenameone.com (reading a cross-origin parent's location throws): canRequestSteps() gates the email field, requests go to that exact origin, and answers from any other origin are ignored. - The request carries buildKind() (maven, gradle, maven-backend, gradle-backend) so BuildCloud emails the same commands -- and the same account line -- the panel showed. - build.sh: SIGTERM exits/reports 143, Ctrl+C 130. Maven's own status is written from inside the pipeline, so tee failing (full disk, unwritable log) can no longer fail a successful build; an uncreatable log just skips the reason, and CN1_TELEMETRY=0 runs ./mvnw with no wrapper at all. Verified end to end: ok, compile, opt-out, unwritable TMPDIR, SIGTERM, Ctrl+C, no leftover processes. - README and build.bat give the PowerShell opt-out ($Env:CN1_TELEMETRY = "0"); `set` in PowerShell never reaches build.bat. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ddb0b18723
ℹ️ 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".
| local pipe_pid tmpbase="${log:-${TMPDIR:-/tmp}/cn1-build-$$}" | ||
| local pidfile="$tmpbase.pid" codefile="$tmpbase.code" |
There was a problem hiding this comment.
When TMPDIR is nonexistent/unwritable, or its filesystem fills after mktemp, lines 101–103 disable the optional log, but this fallback still places both the PID and exit-code files under the same unusable TMPDIR. Those writes then fail and line 143 substitutes code=1, causing a successful Maven build to exit as a failure; signal handling also cannot locate Maven through the missing PID file. Fresh evidence beyond the existing comment: running the generated launcher with TMPDIR pointing to a nonexistent directory printed mvn-success but returned status 1. Store this metadata in an independently writable location or preserve Maven's status without relying on these files.
Useful? React with 👍 / 👎.
| { ./mvnw "$@" <&0 2>&1 & echo $! > "$pidfile"; wait $! 2>/dev/null; echo $? > "$codefile"; } \ | ||
| | { tee "$log" 2>/dev/null || cat; } & |
There was a problem hiding this comment.
On the normal Unix telemetry path, 2>&1 merges Maven's diagnostics into the pipe, and tee then emits everything on stdout. A caller using ./build.sh ... >build.log will therefore stop seeing Maven errors on its terminal, while IDE or CI integrations that classify or annotate stderr receive nothing; the launcher previously preserved both channels, so optional reporting now changes observable build output. Capture the diagnostic log without collapsing Maven's stderr into stdout.
Useful? React with 👍 / 👎.
Review (#5954): - A missing/unwritable/full TMPDIR still failed a successful build: the pid/exit-code files lived under it, and a failed write became code=1. Maven now runs in a background subshell that exits with ${PIPESTATUS[0]} -- Maven's own status -- so wait returns it with no files at all; the signal trap stops that subshell's process tree instead of reading a pid file. Only the reason-log uses TMPDIR, and without it the reason is just "unknown". - 2>&1 merged Maven's stderr into stdout for anyone redirecting either. Each channel now has its own tee into the log and passes through on its own fd. Verified with /bin/bash 3.2 on macOS against a fake Maven: ok, compile, opt-out, missing and read-only TMPDIR (exit 0), stdout redirected (stderr kept apart), SIGTERM 143, Ctrl+C 130, no leftover processes. The launcher harness now checks the missing-TMPDIR and stderr cases on every non-Windows run, keeps its recorder up through them and does not print tracebacks for a curl that gives up mid-request. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 71e948b692
ℹ️ 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".
| }).then(function () { | ||
| return true; |
There was a problem hiding this comment.
When the steps endpoint rejects the request with a 4xx/5xx response (for example, validation, rate limiting, or a server error), fetch still fulfills, and mode: "no-cors" makes the response opaque, so this returns true; the parent then acknowledges success and the panel displays “Check your inbox” even though no email was accepted. Fresh evidence in the current revision is that the new host acknowledgement delegates its result to this no-cors fetch rather than acknowledging server acceptance; use a CORS-readable response or another server-confirmed mechanism for sendSteps.
Useful? React with 👍 / 👎.
Review (#5954): sendSteps used mode "no-cors", so a 400/429/503 from the steps endpoint was an opaque response indistinguishable from success, and the panel said "Check your inbox" for a refused request. The steps request now uses mode "cors" (BuildCloud answers the website origin with Access-Control-Allow-Origin; a form POST stays a simple request, no preflight) and resolves true only on response.ok -- BuildCloud #166 answers 202 accepted, 400, 429 or 503. The download beacon stays fire-and-forget and opaque. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a18f5c2c88
ℹ️ 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".
| ( ./mvnw "$@" <&0 2> >(tee -a "$log" >&2) | tee -a "$log"; exit "${PIPESTATUS[0]}" ) & | ||
| else | ||
| ( ./mvnw "$@" <&0 ) & |
There was a problem hiding this comment.
When Maven or a plugin prompts for input, both telemetry branches deliver EOF instead of the launcher's stdin. The asynchronous compound command itself has no input redirection, so Bash first assigns it /dev/null; the inner ./mvnw ... <&0 merely duplicates that already-replaced descriptor. This is reproducible by piping a line into an equivalent background subshell, and matches the Bash asynchronous-command semantics. Put the input redirection on the parenthesized command before & (or otherwise preserve the original descriptor) so telemetry does not break interactive Maven operations.
Useful? React with 👍 / 👎.
| try { | ||
| if (email && parentOrigin) { | ||
| window.addEventListener("message", onAnswer); | ||
| window.setTimeout(function() { finish(false); }, STEPS_ACK_TIMEOUT_MS); |
There was a problem hiding this comment.
When the BuildCloud request takes longer than six seconds, this timer completes the bridge with false, causing the panel to say the email could not be sent and re-enable the button, but the parent page's fetch continues because it has no abort signal or cancellation path. If that in-flight request is then accepted, the user receives an email despite the failure message and may retry, issuing a duplicate request. Either cancel the parent request when this deadline expires or keep the UI pending until the fetch has a definitive result.
Useful? React with 👍 / 👎.
…le in flight Review (#5954): - bash gives an asynchronous command /dev/null as stdin unless that command carries its own redirection, so the `<&0` inside the background subshell only duplicated /dev/null and a Maven prompt read EOF. The redirection is now on the subshell itself. Verified under /bin/bash 3.2 (old form reads nothing, new form reads the input); the launcher harness now pipes an answer into a prompting stand-in for every generated Maven launcher. - The bridge gave up after 6 s while the page's fetch carried on, so a slow but accepted request ended "not sent" and a retry followed. The page now aborts its own request at 15 s (AbortController) and answers false; the bridge waits 20 s, so it only times out when no host answers at all. A request that reached BuildCloud before the abort is harmless to retry: one email per address per day, a repeat answered as accepted. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 38fd55506a
ℹ️ 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".
| var controller = confirm && typeof AbortController === "function" ? new AbortController() : null; | ||
| var timer = controller && typeof window.setTimeout === "function" | ||
| ? window.setTimeout(function () { controller.abort(); }, CONFIRM_TIMEOUT_MS) : null; |
There was a problem hiding this comment.
In environments that provide fetch and WebCrypto but not AbortController, a confirmed steps request is still started with controller and timer both null. If that fetch remains pending beyond the bridge's 20-second deadline, the panel reports failure and permits a retry even though the original request can later be accepted. Fresh evidence in this revision is that aborting is conditional while canRequestSteps() does not check abort support; either decline the request without cancellation support or ensure the parent cannot leave it active after the bridge times out.
Useful? React with 👍 / 👎.
| for child in $(pgrep -P "$1" 2>/dev/null); do | ||
| cn1_kill_tree "$child" | ||
| done | ||
| kill -TERM "$1" 2>/dev/null || true |
There was a problem hiding this comment.
On Linux installations without pgrep, such as slim containers without procps, this loop discovers no descendants and kill signals only the background subshell. Maven is then orphaned and continues running after the launcher reports cancellation and exits with 130/143; a supposedly cancelled build can therefore keep consuming resources or even submit a cloud build. Track the Maven process without relying on optional pgrep, or run it in a separately signallable process group.
Useful? React with 👍 / 👎.
…t pgrep Review (#5954): - Without AbortController a confirmed steps request could outlive the bridge's deadline and be accepted after the panel reported failure. The page now declines such a request (the panel shows the README fallback), and canRequestSteps() does not offer the email field in such a browser. - On a Linux without pgrep (slim containers without procps) cn1_kill_tree found no children and Maven was orphaned, still running -- even submitting a cloud build -- after the launcher reported cancellation. Children are now found through /proc/<pid>/stat when pgrep is missing. The launcher harness checks it on Linux with pgrep hidden from PATH: SIGTERM exits 143 and Maven is gone; the same check fails against the old pgrep-only code (verified in a python:3.12-slim container). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
@codex review |
|
Codex Review: Didn't find any major issues. Hooray! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
1 participant
Visitors download a project from the website Initializr and then do not build. After Generate Project the page showed nothing: no next step, and no word that the first cloud build asks for an account. Between "downloaded" and "first build" we had no visibility at all.
Depends on BuildCloud #166 being deployed first. It adds the two endpoints this PR posts to (
/api/v2/funnel/initializr-eventand/api/v2/funnel/initializr-steps). Merging this first would not break anything visible, but the events and "email me" requests would 404 until it ships.Next-steps panel
After a successful download, a card appears at the top of the column. It sits clear of the Crisp widget and does not block the form, and a second download replaces it. It shows:
cn1-initializr-steps-requestto the host page via a newWebsiteThemeNative.requestSteps. The website forwards it withcn1InitializrBeacon.sendSteps, using the same host allow-list, hashed package, and no cookies. Checked visually in light and dark mode.Launcher build-progress reporting
Initializr Maven downloads get a
build.sh/build.batthat reportstep=launchandstep=exitwith: target, OS family, Java major version, exit code, a one-word reason (no_jdk,java_too_old,maven_download,compile,login_timeout, …) and duration.Sha256.java, plain Java), the same value the download beacon sends. The package name is never sent.CN1_TELEMETRY=0opts out. Failures are ignored and never change build output or exit code. Ctrl+C reportsinterrupted.test-starter-launchers.pypoints the events URL at a closed local port so CI never posts to production.Tests
GeneratorModelMatrixTestadds: SHA-256 vectors; launcher content for Java 17 and 8 (hash, URL, opt-out, no plaintext package);nextSteps()per IDE; IDE ids matching the endpoint (vscode, notvs_code).NextStepsPanelTest.sendStepsand the bridge.test-starter-launchers.py,sync-initializr-launchers.pyandgenerate-initializr-fixtures.py(all IDEs, Java 8/17, every Maven layout and Gradle type) pass.Pre-existing, not from this PR:
GeneratorModelMatrixTestfails on current master: "unexpected source set: backend/src/test/java/.../ServedApiTest.java", from #5932. The checks this PR adds run before that loop and pass.Not verified:
build.baton Windows (covered by thewindows-latestjob), and the JS bundle in a real browser.🤖 Generated with Claude Code