Repository navigation
feat(init): jOOQ+Raoh default, reference patterns, pre-flight checks, REPL connect fix - #172
Merged
Merged
Conversation
The /init command was producing projects that could not be built. Root causes:
1. pomTemplate emitted a <parent>enkan-parent:UNKNOWN</parent> — enkan-parent is
an internal aggregator that external projects cannot inherit from, and
enkanVersion() always returned "UNKNOWN" because the enkan-repl-client jar
does not set Implementation-Version in its manifest. Maven failed at
model-building, before any LLM fix loop could run.
2. extractFileFromError() only recognized javac errors ("/path/File.java:[r,c]").
Maven model errors (pom.xml problems) never reached the LLM fix prompt with
file content, so the fix loop could not repair a broken POM even in principle.
3. Reference files fetched from GitHub showed a Doma2-based example while the
prompt listed enkan-component-jooq as "available". The LLM invented
plausible-but-wrong Doma/JPA code instead of using jOOQ.
4. The generated ApplicationFactory was produced by the LLM, but its FQCN was
hard-coded in SystemFactory. Any mistake (missing file, wrong class name)
became an instant ClassNotFoundException at runtime.
Changes:
- pomTemplate: standalone POM with no <parent>; declares
maven-compiler-plugin/surefire explicitly; default deps are enkan-component-
HikariCP/jooq/flyway + raoh + raoh-jooq + H2 + JUnit 5 + AssertJ.
- enkanVersion: resolves version from META-INF/maven/net.unit8.enkan/
enkan-repl-client/pom.properties (present in the jar), with
-Denkan.version/ENKAN_VERSION overrides. New resolveEnkanVersionOrAbort()
fails /init with an actionable message instead of emitting "UNKNOWN".
- systemFactoryTemplate: wires HikariCP (H2 in-memory) → Flyway → JooqProvider
(SQLDialect.H2) → Jetty, following the kotowari-example relationship graph.
- applicationFactoryTemplate + jsonBodyReaderTemplate + jsonBodyWriterTemplate:
new fixed templates. The middleware stack (SerDes, jOOQ transaction, routing)
is now deterministic; the LLM only generates RoutesDef.routes(), controllers,
domain records, Raoh decoders, and Flyway migrations.
- buildPrompt / buildPlannerSystemPrompt: rewritten with hard constraints ("no
imports outside the fixed POM", "never import from kotowari.example.*"), a
default-stack declaration with a canonical Raoh+jOOQ decoder snippet, and a
### MANIFEST output requirement. A hasUserRequestedAlternative() escape hatch
re-enables jakarta.persistence / org.seasar.doma imports when the user asks.
- compileAndFix: two-phase mvn validate → mvn compile -Pdev. runMvnBuild returns
a BuildResult(phase, errors) record. buildFixUserPrompt always includes
pom.xml content on VALIDATE failures or when no file could be extracted from
errors. extractFileFromError now parses both javac format and Maven model
format ("@ /path/pom.xml, line N, column N"). MAX_FIX_ATTEMPTS bumped 3→5.
- validateGeneration: fast post-generation checks run before the fix loop —
RoutesDef existence, controller class references resolved, import allowlist
(rejects Spring/Lombok always; Doma/JPA unless user asked), Flyway migration
present, optional ### MANIFEST comparison.
- isFixedTemplateFile recognises ApplicationFactory.java, JsonBodyReader.java,
and JsonBodyWriter.java so the LLM cannot overwrite them.
- 20 new tests (58 total, all green) covering pom structure, XML validity,
version override/abort, template content, POM error extraction, prompt
defaults and JPA escape hatch, validateGeneration FQCN check, import
allowlist, and manifest parsing.
- docs/reference/repl.md: new "Default stack" and "Fixed vs. generated files"
sections explaining the jOOQ+Raoh pattern and how to opt out.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Small models (mistralai/devstral-small-2-2512 was the immediate motivator)
could not produce a working project from the init prompts alone. The
prompts listed jOOQ/Raoh/HikariCP/Flyway as the default stack, but the only
concrete reference code in the LLM context was the kotowari-example files
fetched from GitHub — which demonstrate Doma2, not jOOQ. The LLM invented
plausible-but-wrong Doma/JPA code, and a previous inline snippet of the
Raoh decoder pattern used `Result.Ok` / `Result.Err` which does not exist
(Ok and Err are top-level records in net.unit8.raoh, not nested in Result).
Ship copy-paste-ready reference files bundled with the enkan-repl-client
jar so the generator is self-sufficient and always in sync with the
templates it writes.
New classpath resources under /init-reference/:
- README.md — index + non-negotiable rules
- imports.md — exhaustive FQCN list (HTTP, jOOQ, Raoh,
Jakarta Transactional, etc.) so small
models do not guess wrong packages
- components/overview.md — component graph + middleware stack +
request lifecycle + what NOT to add
- components/jooq.md — canonical queries and controller access
- patterns/routes.md — RoutesDef class shape, Routes.define DSL,
path param syntax, verb list
- patterns/controller.md — parameter injection rules, how to obtain
DSLContext, @transactional behaviour,
full CRUD example
- patterns/raoh-decoder.md — correct combine/field/map API, pattern
match on Ok(...)/Err(...), nested
decoders for JOINs, common Err codes
- patterns/domain-record.md — record + static DECODER pairing rules
- patterns/flyway-migration.md — V1__*.sql naming, H2 type cheat sheet,
`GENERATED BY DEFAULT AS IDENTITY`
InitCommand changes:
- INIT_REFERENCE_RESOURCES constant lists the 9 classpath paths in a
narrative order (overview → imports → routes → controller → domain →
decoder → migration → jooq specifics).
- loadInitReference() reads them synchronously from the classpath; no
network, no failure modes beyond a missing resource.
- buildPlannerSystemPrompt() and buildPrompt() now inject the canonical
patterns under a prominent "== CANONICAL PATTERNS (authoritative) =="
banner, above the existing GitHub-fetched references which are now
relegated to "== LEGACY REFERENCE FILES (architectural context only) ==".
- The broken inline Raoh snippet in buildPrompt() (which contained
`case Result.Ok<Todo>(var t)` — non-existent syntax) is deleted; the
correct pattern is now in patterns/raoh-decoder.md and patterns/
controller.md, guarded by a regression test.
Tests (5 new, 63 total):
- loadInitReferenceLoadsAllDeclaredResourcesFromClasspath — every declared
resource actually loads and has non-empty content.
- loadInitReferenceContainsKeyCanonicalTerms — the combined content covers
every API shape the LLM must know (RoutesDef, jooqDslContext,
JooqRecordDecoders, combine/field/long_/string/bool, net.unit8.raoh.Ok/Err,
enkan.web.data.HttpRequest, jakarta.transaction.Transactional, V1__,
db/migration).
- loadInitReferenceDoesNotUseStaleResultDotOkSyntaxInCodeBlocks — regression
guard. Walks each reference file line-by-line, tracking fence state, and
asserts that `Result.Ok` / `Result.Err` never appears inside a code block.
Anti-example mentions in prose are allowed.
- generationPromptEmbedsCanonicalPatternContent — proves buildPrompt()
actually embeds the classpath content (not just a pointer).
- plannerPromptEmbedsCanonicalPatternContent — same for the planner phase.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…emplate
Running /init produced a project that failed to compile with:
[ERROR] DefaultCharsetMiddleware not found in package enkan.middleware
[ERROR] ContentNegotiationMiddleware not found in package enkan.web.middleware.negotiation
The generated TodoApplicationFactory used stale pre-rename package paths:
- enkan.middleware.DefaultCharsetMiddleware
→ should be enkan.web.middleware.DefaultCharsetMiddleware
- enkan.web.middleware.negotiation.ContentNegotiationMiddleware
→ should be enkan.web.middleware.ContentNegotiationMiddleware
- enkan.middleware.ServiceUnavailableMiddleware
→ dead import (not used in the body), remove
The ApplicationFactory is a fixed template so this was a bug in InitCommand
itself, not in the LLM's output.
Add applicationFactoryTemplateUsesCorrectMiddlewarePackages as a regression
guard that asserts:
1. Every enkan-web middleware is imported from enkan.web.middleware
(including ContentNegotiationMiddleware, which lives directly there
and NOT under a .negotiation sub-package).
2. None of the pre-rename stale packages (enkan.middleware.*CharsetMiddleware)
are used.
3. Every import maps to a class actually referenced in the class body —
catches dead imports like the removed ServiceUnavailableMiddleware.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
When running /init against mistralai/devstral-small-2-2512 the generated
ApplicationFactory failed to compile because the fixed template used stale
pre-rename middleware packages (now fixed in code). The reference file the
LLM sees should also surface these pitfalls prominently so that (a) the LLM
does not make the same mistakes in the files it generates, and (b) the file
is self-documenting for future edits.
Additions to imports.md:
- New "Common package pitfalls" section at the top, listing the three
common training-data mistakes:
enkan.data.HttpRequest → enkan.web.data.HttpRequest
enkan.middleware.XxxMiddleware → enkan.web.middleware.XxxMiddleware
.negotiation.ContentNegotiationMiddleware → enkan.web.middleware.ContentNegotiationMiddleware
- Explicit list of exceptions (jOOQ and ServiceUnavailableMiddleware still
use the old enkan.middleware.* path because they live in separate
component modules).
- New "Web middlewares" section listing every class that lives under
enkan.web.middleware, so the LLM can copy-paste the exact imports for
the middlewares the ApplicationFactory uses.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The /init command assumes the end user has only downloaded `enkan-repl` (the single-file executable built by maven-shade from enkan-repl-client). That works when Maven is installed and the baked-in enkan.version resolves from Maven Central — but neither assumption was being checked, so the user would discover them mid-run via confusing error messages. Three fail-fast checks now run at the top of /init, before the user answers any project-setup prompts: 1. verifyMavenAvailable() — runs `mvn -v`. If the executable is missing, prints install instructions for Homebrew, SDKMAN, apt, and the upstream download page, and aborts cleanly. 2. resolveEnkanVersionOrAbort() now also warns (but does not abort) when the detected enkan.version ends with `-SNAPSHOT` and neither the -Denkan.version system property nor the ENKAN_VERSION environment variable is set. The warning explains that Maven Central does not host SNAPSHOT artifacts and suggests either `mvn install` on the enkan tree or a -Denkan.version=<released> override. 3. pomTemplate() writes an HTML comment above <modelVersion> when the baked-in enkan.version is a SNAPSHOT, so the generated project's own pom.xml is self-documenting about why its dependencies might not resolve on a fresh machine. Test changes (69 total, 5 new): - executeAbortsWhenMavenMissing — subclass returns false from verifyMavenAvailable; assert that execute() returns false, no prompts were issued, and the error message mentions Maven. - executeAbortsWhenEnkanVersionUnresolvable — in the test classpath enkanVersion() returns "UNKNOWN", so the same abort path fires. - resolveEnkanVersionDoesNotWarnWhenSnapshotOverrideIsExplicit — -Denkan.version=x.y.z-SNAPSHOT is treated as an explicit opt-in. - pomTemplateIncludesSnapshotNoteWhenVersionIsSnapshot — regression guard for the generated NOTE comment. - pomTemplateOmitsSnapshotNoteForReleasedVersion — ensures the note does not leak into released-version generations. Existing execute() tests (firstPromptIsSingleLineWithoutContinuationNewline, cancelsImmediatelyWhenPromptInputIsInterrupted) were updated to override verifyMavenAvailable and set -Denkan.version, because they no longer reach the prompt phase otherwise. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Running /jsonRequest in the generated jOOQ+Raoh project made the REPL client
show output like:
enkan(61288)❯ /jsonRequest GET /api/todos
[]
-----------------END------------------
The `-----------------END------------------` line was a sentinel written by
SystemIoTransport.send() after every DONE response. It was introduced in the
WebSocket transport refactor (commit 667be3b) as a chunk marker so the host
could split JShell stdout into per-command frames, but no consumer was ever
implemented. JShellIoProxy's two reader threads filtered the marker out of
their own broadcast paths — making it invisible in the happy case — but the
filter was a single point of failure. Any alternate broadcast route (e.g.
application-level logs flushed on the same line, or any future IO path) that
skipped the filter would leak the marker to the client.
Remove the write entirely. Completion is already signalled by the DONE status
flag on the ReplResponse frame itself (which the host broadcasts after the
snippet returns), so the delimiter carries no information the client cannot
already see. The filters in JShellIoProxy become dead code and are deleted
together with the unused `java.util.Objects` import.
Add JShellReplIntegrationTest#transportSendFromSnippetDoesNotLeakDelimiter
as a regression guard. It sends `transport.send(withOut("LINE_...")); transport.sendOut("")`
through the JShell-scope SystemIoTransport and asserts the client never sees
the chunk delimiter on either the out or err channel.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…e REPL
Typing `/connect <port>` where nothing is listening would silently terminate
the REPL: the prompt disappeared with no error message and the user dropped
back to the shell. The same bug also took down a previously-working
connection if the user attempted to /connect to a bad port a second time.
Root cause was in ConsoleHandler.connect():
1. The new ZMQ socket / completer socket / monitor socket were assigned to
instance fields immediately, before the connection was verified to
succeed. A failure overwrote any previous good state.
2. The monitor thread treated DISCONNECTED / CLOSED events as a server
shutdown signal in *all* phases — even when the events came from a
never-completed initial handshake. It then called close() on the whole
ConsoleHandler, which sets isAvailable=false and closes the JLine
terminal, ending the REPL run loop.
3. The completer-reply poll loop had no timeout: if the monitor signalled
"give up" the loop returned silently without printing anything.
Fix:
- connect() now builds all per-attempt state on local variables (newSocket,
newCompleterSock, monitorSocket, attemptFailed flag) and only commits
them to instance fields after the /completer handshake succeeds.
- The monitor thread tracks a handshakeSeen flag. Phase 1 (before
CONNECTED is observed): treat DISCONNECTED / CLOSED / >3 retries as
"attempt failed" and signal via attemptFailed without touching instance
state. Phase 2 (after CONNECTED): preserve the original behaviour of
setting serverDisconnected=true and calling close() so a real server
shutdown still ends the REPL with the "Server disconnected." message.
- The completer-reply poll loop has a 3-second deadline. On timeout or on
monitor-signalled failure, the per-attempt sockets are torn down and a
clear error ("Failed to connect to localhost:9999 (no response within
3s — is the server running?)") is printed via printErr() so the user
sees what went wrong.
- On a successful new connection, closePreviousConnection() drops any
sockets the previous /connect left behind so re-connecting to a
different server works without leaks.
ConsoleHandler is promoted from `private static` to package-private and
gains test accessors `isAvailable()` / `getConnectedPort()` so the new
ReplClientConnectFailureTest can drive connect() directly.
Tests (71 total, 2 new):
- connectToNonListeningPortDoesNotKillRepl — single failed attempt;
asserts isAvailable stays true, connectedPort stays -1, and the
terminal output mentions "connect".
- repeatedFailedConnectAttemptsDoNotLeakStateOrKillRepl — three
consecutive failed attempts; asserts the REPL is still alive and
a connect-failure message appears at least three times in the output.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Added tests for handling server shutdown after successful connection in ReplClientConnectFailureTest. - Improved validation for hostname/IP in ReplClientLocalHelpTest, ensuring special characters are rejected. - Refactored InitCommandTest to use instance methods instead of static calls for version retrieval. - Enhanced file generation tests to handle CRLF line endings and validate against XML injection. - Added checks to reject invalid group IDs and project names with special characters in InitCommand. - Ensured that the system property for version handling is robust against potential XML injection attacks.
There was a problem hiding this comment.
Pull request overview
This PR updates the REPL /init scaffolding flow to default to a jOOQ + Raoh project stack, embeds canonical reference patterns as classpath resources for more reliable prompt guidance, adds pre-flight checks (Maven availability + Enkan version resolution), and hardens the REPL client/server interaction (notably /connect failure handling and removal of the REPL chunk delimiter sentinel).
Changes:
- Switch
/initgenerated project defaults to jOOQ+Raoh and add offline “init-reference” pattern resources used in prompts. - Add pre-flight checks + stronger generation validation and a more structured Maven validate/compile fix loop.
- Fix REPL client
/connectrobustness and remove theCHUNK_DELIMITERsentinel from JShell transport output (with regression tests).
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| enkan-repl-server/src/test/java/enkan/system/repl/JShellReplIntegrationTest.java | Adds regression test ensuring JShell transport output never leaks the old delimiter sentinel. |
| enkan-repl-server/src/main/java/enkan/system/repl/jshell/SystemIoTransport.java | Removes CHUNK_DELIMITER emission and documents the behavioral change. |
| enkan-repl-server/src/main/java/enkan/system/repl/jshell/JShellIoProxy.java | Removes filtering logic for the deleted delimiter sentinel. |
| enkan-repl-client/src/test/java/enkan/system/repl/command/InitCommandTest.java | Expands coverage for new templates, references, pre-flight checks, fix-loop behavior, and validation rules. |
| enkan-repl-client/src/test/java/enkan/system/repl/client/ReplClientLocalHelpTest.java | Adds tests for hostname validation. |
| enkan-repl-client/src/test/java/enkan/system/repl/client/ReplClientConnectFailureTest.java | Adds regression tests ensuring failed /connect attempts don’t kill the REPL and that disconnect monitoring works after a successful connect. |
| enkan-repl-client/src/main/resources/init-reference/README.md | Adds canonical init reference overview and non-negotiable rules for the default stack. |
| enkan-repl-client/src/main/resources/init-reference/patterns/routes.md | Adds authoritative RoutesDef pattern guidance for the generator. |
| enkan-repl-client/src/main/resources/init-reference/patterns/raoh-decoder.md | Adds canonical Raoh+jOOQ decoder patterns and anti-pattern warnings. |
| enkan-repl-client/src/main/resources/init-reference/patterns/flyway-migration.md | Adds canonical Flyway migration layout and H2-friendly DDL guidance. |
| enkan-repl-client/src/main/resources/init-reference/patterns/domain-record.md | Adds canonical “record + decoder” pattern guidance. |
| enkan-repl-client/src/main/resources/init-reference/patterns/controller.md | Adds canonical controller patterns including DSLContext acquisition and transaction rules. |
| enkan-repl-client/src/main/resources/init-reference/imports.md | Adds authoritative import/FQCN reference (including rename pitfalls). |
| enkan-repl-client/src/main/resources/init-reference/components/overview.md | Adds canonical component + middleware-stack overview for the default stack. |
| enkan-repl-client/src/main/resources/init-reference/components/jooq.md | Adds canonical jOOQ wiring and usage patterns. |
| enkan-repl-client/src/main/java/enkan/system/repl/command/InitCommand.java | Implements new init workflow (templates, references, validation, pre-flight checks, Maven validate/compile fix loop, version resolution). |
| enkan-repl-client/src/main/java/enkan/system/repl/client/ReplClient.java | Fixes /connect failure behavior to avoid killing the client REPL; adds host validation and connection lifecycle hardening. |
| docs/src/content/reference/repl.md | Documents /init default stack, fixed vs generated files, build phases, and Enkan version override behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+1029
to
+1036
| proc = new ProcessBuilder(cmd) | ||
| .directory(outPath.toFile()) | ||
| .redirectErrorStream(true) | ||
| .start(); | ||
| String output = new String(proc.getInputStream().readAllBytes(), StandardCharsets.UTF_8); | ||
| int exitCode = proc.waitFor(); | ||
| if (exitCode == 0) return null; | ||
| // Extract only the ERROR lines to keep the prompt compact | ||
| return output.lines() | ||
| .filter(l -> l.startsWith("[ERROR]")) | ||
| boolean finished = proc.waitFor(MVN_PHASE_TIMEOUT_MINUTES, TimeUnit.MINUTES); | ||
| if (!finished) { | ||
| return new BuildResult(phase, "mvn " + String.join(" ", mvnArgs) |
There was a problem hiding this comment.
runMvnPhase reads the entire process output via proc.getInputStream().readAllBytes() before calling waitFor(timeout, ...). Because readAllBytes() blocks until the process exits (EOF), the timeout will not be effective for long-running/stuck Maven invocations. Consider enforcing the timeout first (and destroying the process on timeout) while concurrently draining stdout/stderr (or reading output after the process ends) so the phase cannot hang indefinitely.
Comment on lines
+193
to
+197
| } | ||
| if (!SAFE_ARTIFACT_PATTERN.matcher(projectName).matches()) { | ||
| transport.sendErr("Project name '" + projectName + "' contains invalid characters. Use only alphanumerics, hyphens, and underscores (no dots)."); | ||
| return false; | ||
| } |
There was a problem hiding this comment.
The validation patterns accept Maven-valid groupId / projectName values that are not valid Java identifiers (e.g. hyphens in groupId segments like com.my-app, or projectName starting with a digit like 123). Since these values are used to form basePackage and class names, this can generate uncompilable code even though the input passed validation. Consider validating against Java package/class identifier rules (or sanitizing inputs when forming package/class names) and aborting with a user-facing error when invalid.
Suggested change
| } | |
| if (!SAFE_ARTIFACT_PATTERN.matcher(projectName).matches()) { | |
| transport.sendErr("Project name '" + projectName + "' contains invalid characters. Use only alphanumerics, hyphens, and underscores (no dots)."); | |
| return false; | |
| } | |
| } | |
| String[] groupIdSegments = groupId.split("\\."); | |
| for (String segment : groupIdSegments) { | |
| if (segment.isEmpty() | |
| || !Character.isJavaIdentifierStart(segment.charAt(0))) { | |
| transport.sendErr("Group ID '" + groupId + "' is not valid for Java package generation. Each dot-separated segment must be a valid Java identifier."); | |
| return false; | |
| } | |
| for (int i = 1; i < segment.length(); i++) { | |
| if (!Character.isJavaIdentifierPart(segment.charAt(i))) { | |
| transport.sendErr("Group ID '" + groupId + "' is not valid for Java package generation. Each dot-separated segment must be a valid Java identifier."); | |
| return false; | |
| } | |
| } | |
| } | |
| if (!SAFE_ARTIFACT_PATTERN.matcher(projectName).matches()) { | |
| transport.sendErr("Project name '" + projectName + "' contains invalid characters. Use only alphanumerics, hyphens, and underscores (no dots)."); | |
| return false; | |
| } | |
| if (projectName.isEmpty() | |
| || !Character.isJavaIdentifierStart(projectName.charAt(0))) { | |
| transport.sendErr("Project name '" + projectName + "' is not valid for Java class generation. It must be a valid Java identifier."); | |
| return false; | |
| } | |
| for (int i = 1; i < projectName.length(); i++) { | |
| if (!Character.isJavaIdentifierPart(projectName.charAt(i))) { | |
| transport.sendErr("Project name '" + projectName + "' is not valid for Java class generation. It must be a valid Java identifier."); | |
| return false; | |
| } | |
| } |
Comment on lines
+1759
to
+1784
| * <p>A SNAPSHOT version is not an abort — the REPL may be running from a locally | ||
| * built enkan-repl-client against a locally installed snapshot of every enkan | ||
| * component — but the user gets a clear warning so they can diagnose "unresolved | ||
| * dependency" errors later. A user who just downloaded a release build of | ||
| * {@code enkan-repl} never sees the warning. | ||
| */ | ||
| String resolveEnkanVersionOrAbort(Transport transport) { | ||
| String v = enkanVersion(); | ||
| if ("UNKNOWN".equals(v)) { | ||
| transport.sendErr("Cannot determine Enkan version for the generated pom.xml."); | ||
| transport.sendErr(" Run from a packaged enkan-repl-client jar, or pass -Denkan.version=<x.y.z>"); | ||
| transport.sendErr(" (or set the ENKAN_VERSION environment variable)."); | ||
| return null; | ||
| } | ||
| if (!SAFE_VERSION_PATTERN.matcher(v).matches()) { | ||
| transport.sendErr("Enkan version '" + v + "' contains characters that are not safe to embed in XML."); | ||
| transport.sendErr(" Override with -Denkan.version=<x.y.z> using only alphanumerics, dots, and hyphens."); | ||
| return null; | ||
| } | ||
| if (v.endsWith("-SNAPSHOT") && System.getProperty("enkan.version") == null | ||
| && System.getenv("ENKAN_VERSION") == null) { | ||
| transport.sendOut(YELLOW + "⚠ Using SNAPSHOT version " + v + "." + RESET + "\n"); | ||
| transport.sendOut(DIM + " Maven Central does not host SNAPSHOT artifacts. The generated project\n" | ||
| + " will only build if you have run `mvn install` on the full enkan tree,\n" | ||
| + " or if you override with -Denkan.version=<released-version>.\n" + RESET); | ||
| } |
There was a problem hiding this comment.
PR description says the pre-flight checks "reject SNAPSHOT framework versions", but resolveEnkanVersionOrAbort currently only warns-and-continues for implicit SNAPSHOTs (and returns the SNAPSHOT version). Either update the PR description/docs to match the implemented behavior, or change this method to abort when a SNAPSHOT version is detected (unless explicitly overridden).
Comment on lines
+141
to
+145
| ZThread.fork(ctx, (args, c, pipe) -> { | ||
| int retryCnt = 0; | ||
| while (!Thread.currentThread().isInterrupted()) { | ||
| // Blocking recv — waits until an event arrives or context is terminated | ||
| ZEvent event = ZEvent.recv(monitorSocket); | ||
| if (event == null) { | ||
| if (monitorSocket.errno() == ZError.ETERM) break; | ||
| continue; | ||
| } | ||
| ZMonitor.Event eventType = event.getEvent(); | ||
| if (eventType == ZMonitor.Event.DISCONNECTED || eventType == ZMonitor.Event.CLOSED) { | ||
| serverDisconnected.set(true); | ||
| isSocketClosed.compareAndSet(false, true); | ||
| close(); | ||
| break; | ||
| } else if (eventType == ZMonitor.Event.CONNECT_RETRIED) { | ||
| if (retryCnt++ > 3) { | ||
| System.err.println("Connection failed"); | ||
| isSocketClosed.compareAndSet(false, true); | ||
| break; | ||
| try { | ||
| while (!Thread.currentThread().isInterrupted()) { | ||
| ZEvent event = ZEvent.recv(monitorSocket); |
There was a problem hiding this comment.
The monitor thread is started with ZThread.fork(ctx, ...) but the returned parent-side PAIR socket is ignored. ZThread.fork allocates a socket pair; if the parent socket is never closed, repeated /connect attempts can leak ZMQ resources. Consider capturing the returned socket and closing it immediately (since it's unused), or switching to a plain Thread if no pipe is needed.
Comment on lines
+1805
to
+1812
| // Drain output to avoid the child blocking on a full pipe. | ||
| proc.getInputStream().readAllBytes(); | ||
| // Cap at 10 s: a slow Maven wrapper download should not block /init forever. | ||
| boolean finished = proc.waitFor(10, TimeUnit.SECONDS); | ||
| if (!finished) { | ||
| LOG.warn("mvn -v timed out after 10 s"); | ||
| } else if (proc.exitValue() == 0) { | ||
| return true; |
There was a problem hiding this comment.
verifyMavenAvailable drains the process output with proc.getInputStream().readAllBytes() before waitFor(10, TimeUnit.SECONDS). If mvn -v hangs (e.g. Maven Wrapper downloading, or a stuck invocation), readAllBytes() can block indefinitely and the 10s timeout won't trigger. Consider waiting with timeout first (and/or draining output asynchronously) so this pre-flight check can't hang the REPL.
Suggested change
| // Drain output to avoid the child blocking on a full pipe. | |
| proc.getInputStream().readAllBytes(); | |
| // Cap at 10 s: a slow Maven wrapper download should not block /init forever. | |
| boolean finished = proc.waitFor(10, TimeUnit.SECONDS); | |
| if (!finished) { | |
| LOG.warn("mvn -v timed out after 10 s"); | |
| } else if (proc.exitValue() == 0) { | |
| return true; | |
| CompletableFuture<byte[]> outputFuture = CompletableFuture.supplyAsync(() -> { | |
| try { | |
| return proc.getInputStream().readAllBytes(); | |
| } catch (IOException e) { | |
| throw new RuntimeException(e); | |
| } | |
| }); | |
| // Cap at 10 s: a slow Maven wrapper download should not block /init forever. | |
| boolean finished = proc.waitFor(10, TimeUnit.SECONDS); | |
| if (!finished) { | |
| LOG.warn("mvn -v timed out after 10 s"); | |
| } else { | |
| outputFuture.join(); | |
| if (proc.exitValue() == 0) { | |
| return true; | |
| } |
… and identifier validation - ReplClient: switch ZThread.fork (IAttachedRunnable) to ZThread.start (IDetachedRunnable) so the parent-side PAIR pipe socket is not allocated and then leaked; monitor thread already did not use the pipe - InitCommand.runMvnPhase: cancel outputFuture in the TimeoutException branch so the async drain thread is signalled to stop - InitCommand.verifyMavenAvailable: call proc.destroyForcibly() before drain.get(1s) so the process stream closes and readAllBytes() in the drain thread unblocks promptly - InitCommand.execute: validate each dot-separated groupId segment as a valid Java package identifier; validate that normalizeProjectName() produces a non-empty, letter-starting class name Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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.
Summary
init-reference/) soInitCommandcan embed authoritative examples (controller, routes, domain-record, Flyway migration, Raoh decoder, jOOQ component, imports, overview)ApplicationFactorymiddleware package imports in generated template/connectinReplClientso connecting to a non-listening port no longer kills the REPL processCHUNK_DELIMITERsentinel that leaked into REPL client outputTest plan
mvn test -pl enkan-repl-client -am— all unit tests pass including newReplClientConnectFailureTestand expandedInitCommandTestmvn test -pl enkan-repl-server -am—JShellReplIntegrationTestadditions pass (run without-DexcludedGroups=integration)/initin the REPL and verify it compiles withmvn package/connectto a port with no listener and confirm REPL stays alive🤖 Generated with Claude Code