feat(server): custom routes, watch toggle, embedding API across all three engines - #170
Merged
Conversation
Upstream the downstream serverplus package into pkg/server as a strict superset of the existing server: - Config.Routes: custom endpoints with Ingress (HTTP -> pine.Request) and Egress (result -> HTTP response) adapters; validated against built-in endpoint conflicts, duplicates, malformed paths, and nil adapters. Custom paths join the bounded known-path set for HTTP metrics (no cardinality explosion). - Config.Watch (*bool): nil/true keeps the config hot-reload watcher (backward compatible); false disables it. pine.Bool helper added. - Embedding API: NewServer(cfg) constructs the engine + resources + snapshot without net/http; Server.Execute runs a request against the live snapshot with in-flight ref held; Server.Acquire returns a Handle (Engine/Resources/ResourceMetrics + Release) for frameworks like Gin; Server.Close joins the watcher then drops the baseline ref. - Run() reimplemented on top of NewServer; NewServer returns errors instead of log.Fatal. Unit tests cover route validation, Watch toggle, Execute/Acquire/Close lifecycle, and the routeHandler 405/ingress-error/execute paths.
-watch=false disables the config hot-reload watcher (Config.Watch). -demo-routes registers POST /api/echo, a demo custom route used by cross-validate to exercise Route/Ingress/Egress behavior across the three engines. Smoke-verified: 200 on valid body, 400 on malformed body, 405 on wrong method, custom path appears under its own label in /stats http metrics (not _other).
Section 20 starts all three servers with -demo-routes -watch=false and verifies: (1) POST /api/echo 200 + body parity, (2) wrong method 405, (3) malformed body -> Ingress error -> Egress 400, (4) built-in endpoints unaffected, (5) unknown path still 404, (6) custom route path labeled by itself in /stats http metrics (bounded set), (7) watch=false keeps reload_count at 0 after a config touch. Docs: README core features + cross-validate list (19 -> 20 sections); llmdoc dag-engine.md Server lifecycle section rewritten for NewServer/Execute/Acquire/Close + Routes + Watch; metrics doc notes custom paths join the bounded known-path set.
Port the Go Route/Ingress/Egress extension point to pine-cpp with
byte-identical black-box behavior:
- Route{method, path, ingress, egress} with RouteRequest/RouteResponse;
Ingress converts the raw HTTP request into a pine::Request, Egress
stages the full response (status/content-type/body).
- validate_routes (new routes.cpp, socket-free so it links into the
test target) rejects built-in conflicts, duplicates, malformed paths
and empty adapters with error strings byte-aligned to Go's
validateRoutes.
- normalize_path now takes the dynamic known-path set (built-ins +
custom routes) so custom paths get their own bounded metrics label.
- ServerConfig.watch (default true); false skips the config watcher
thread. -watch=true|false and -demo-routes (POST /api/echo) flags
wired into pineapple-server main.
- Custom-route execution holds the engine shared_lock for the whole
ingress-result window (equivalent to Go's refcounted snapshot; the
existing shared_mutex lifecycle is intentionally kept, no embedding
API since the C++ server is not a public library).
tests/test_routes.cpp: 10 doctest cases pinning validation error
strings and normalize_path behavior. WERROR build clean, ctest green,
demo route output verified byte-exact vs pine-go.
Port the Go Route/Ingress/Egress extension point to PineServer:
- Static nested types Ingress (HttpExchange -> ExecRequest), Egress
(writes the full response), ExecRequest, Route. addRoute validates
before start() with error strings byte-aligned to Go's
validateRoutes (built-in conflicts, duplicates, malformed paths,
null adapters).
- setWatch(false) disables the config hot-reload watcher; load() is
the embedding entrypoint (config parse + loadConfig + optional
watcher, idempotent) and start() builds the HTTP layer on top of it.
- Embedding API: execute(common, items) runs against the refcounted
live snapshot (throws IllegalStateException("engine not loaded")
when none); acquire() returns a Handle exposing
engine()/resources()/resourceMetrics()/release().
- Custom routes register through wrapHandler with the route path as
its own bounded metrics label; routeHandler enforces method (405),
routes ingress failures to egress(null, e).
- main() wires -Dpine.watch and -Dpine.demoRoutes (POST /api/echo,
byte-exact with the Go/C++ demo route via sortMapKeys).
Fix: stop() now clears the snapshot reference (getAndSet(null),
matching Go Close()'s Swap(nil)). Previously acquireSnapshot() after
stop() would spin forever on the retired snapshot (refs=0, acquire
always false) instead of reporting engine-not-loaded.
RouteTest.java adds 21 cases mirroring pine-go routes_test.go. Full
suite: 290 tests green. Demo route smoke-verified byte-exact vs Go.
Contributor
🔍 PR 审查
代码质量高，三引擎（pine-go / pine-java / pine-cpp）的自定义路由、Watch 开关、嵌入 API 行为对齐良好，测试覆盖充分（Go/Java 单测 + C++ doctest + cross-validate section 20 + fuzz）。核心并发语义（引用计数快照 + CAS acquire）在三端一致，未发现阻塞问题。 逐项确认的关键点：
|
…iew) If HttpServer.create or context registration throws after load() succeeded (e.g. port already bound), the watcher thread and snapshot baseline used to leak unless the caller remembered to call stop(). start() now wraps the HTTP-layer setup in try/catch and calls stop() on failure, mirroring Go Run()'s defer Close. New RouteTest case pins the rollback: execute() reports engine-not-loaded and acquire() returns null after a failed start().
Contributor
🔍 PR 增量审查
增量仅改动两个 Java 文件，正是上次审查中「🟢 小问题」指出的 逐项确认：
上次审查的三引擎行为对齐、并发语义、错误文案字节对齐等结论不受本次增量影响，继续保持。至此 issue #169 的三端实现与我提出的唯一改进点均已闭环。 |
Reflection upstream-serverplus-custom-routes.md captures: the demo-routes black-box validation pattern for programmatic extension points, error-string lock-in at design time, the intentional C++ no-embedding-API decision, the accepted Java throw/return error-model asymmetry, plus process lessons (worktree patch collection, old state machines reached through new entrypoints, mvn diagnostics pitfalls). Stable-doc promotions: cross-layer-validation.md gains the demo-routes pattern; pine-cpp-runtime.md documents the behavior-parity / structure-freedom decision; dag-engine.md archives the Java error-model divergence; standard-workflow.md adds the parallel-worker change collection protocol. index.md synced.
Contributor
🔍 PR 增量审查
增量仅为文档改动，无任何代码变更。新增/修订的都是 逐项确认：
前两次审查关于三引擎行为对齐、并发语义（引用计数快照 + CAS acquire）、 |
…llback (review)
Address the from-c353c04 code-review findings on the Go side:
- Custom routes now apply MaxRequestBodySize via http.MaxBytesReader
before user Ingress code reads the body, and a tripped cap responds
413 centrally (same bytes as /execute) without invoking Egress —
custom endpoints can no longer bypass the server-wide limit. The
demo Ingress propagates read errors as-is so MaxBytesError reaches
the central handler.
- Custom route paths dispatch via exact string lookup behind the "/"
fallback instead of ServeMux pattern registration: a trailing slash
no longer becomes a subtree wildcard, "{name}" is a literal, and
malformed patterns cannot panic mux setup. Matches pine-java's
exact-path guard and pine-cpp's map lookup.
- normalizeConfig applies the :8080 default in both Run and NewServer;
previously NewServer defaulted only its own copy and Run handed the
original empty Addr to http.Server (net/http would bind :http/80).
- NewServer now rolls back on every failure path between engine
creation and snapshot publication (deferred engine.Close + rm.Stop),
and Manager.Start releases values loaded before a mid-load failure
so partially-initialized resources cannot leak open connections.
Tests: 413-on-custom-route (Egress not called), exact-path dispatch
matrix (trailing slash, braces, deep paths), normalizeConfig defaults,
and a rollback test pinning that a Closer resource loaded before a
failing one is closed when NewServer fails.
- routeHandler wraps the exchange body in LimitedBodyStream before user Ingress code reads it: byte limit+1 throws BodyLimitExceededException, which the handler maps to the same central 413 response as /execute (Egress never runs). Custom routes can no longer bypass max_request_body_size. - stop() now awaits watcher termination (shutdownNow only interrupts, and loadConfig never polls the interrupt flag), and snapshot publication happens under a stop lock with a closed flag: a reload that already passed the mtime check when stop() ran can no longer re-publish a snapshot after teardown — previously that leaked the new snapshot and let execute()/acquire() spuriously come back to life. Go's Close() (watcher join, then Swap(nil)) is the reference semantics. RouteTest gains a reload-after-stop case pinning that a late loadConfig refuses to publish and the server stays engine-not-loaded. Full suite: 291 tests green.
…e (review) Custom routes previously called engine_->execute directly, bypassing the shared server execution path: scheduler run_count and per-operator exec/skip stats never counted custom-route pipeline runs, diverging from Go/Java /stats. handle_custom_route now goes through execute_with_trace — same stats accounting, same lock discipline, and client-disconnect cancel now works on custom routes exactly like /execute. Engine-not-loaded still surfaces to Egress as an error.
…y, C++ arm gate (review) - New check [7]: POST /api/echo must bump scheduler run_count and operator exec_count on all three engines — guards the C++ regression where custom routes bypassed the shared execution path and /stats stayed flat. - New check [8]: an oversized body on the custom route returns the central 413 with byte-identical body across engines. - CPP_SERVER set but readiness failed is now a hard fail (previously the C++ arm silently vanished with zero checks and the section still passed), plus a terminal assertion that a configured C++ binary ran a non-zero number of checks. - README and make help drop hard-coded section counts per the llmdoc/must/conventions.md rule; both point at scripts/cross-validate/ as the source of truth.
Contributor
🔍 PR 增量审查
本次增量是对前一轮 code-review 发现的集中修复，四个提交分别加固了 Go/Java/C++ 三端的自定义路由，并同步收紧了 cross-validate section 20。逐项验证正确，测试到位，未发现阻塞或重要问题。 逐项确认：
前三轮审查关于三引擎行为对齐、并发语义（引用计数快照 + CAS acquire）、
|
Reflection gains a second-round section covering the 8 findings from the local deep review (body-limit bypass, ServeMux pattern semantics, by-value Config default regression, stop/reload race, C++ stats bypass, fail-open test arm, hard-coded section counts, bot-APPROVE != no-issues). Promotions: standard-workflow expands the state-machine entry to "rescan the whole machine after fixing one hole" (reachability, concurrency interleaving, publication order); cross-layer-validation adds observable-side-effects parity (stats increments, check [7]) and fail-closed optional test arms; dag-engine documents the shared body-limit contract and exact-path fallback dispatch. index synced.
Contributor
🔍 PR 增量审查
增量仅为一个纯文档提交（ 逐项确认：
前四轮审查关于三引擎行为对齐、并发语义（引用计数快照 + CAS acquire）、 |
The previous commit moved custom routes onto execute_with_trace but left a check-then-execute race: the caller checked engine_ under a short shared lock, released it, and execute_with_trace then read resource_manager_ OUTSIDE any lock before re-acquiring engine_mu_ for the execute. reload_config swaps BOTH engine_ and resource_manager_ under the exclusive lock and tears the old ones down after unlocking, so a custom-route request interleaving with a hot reload could race the unique_ptr (use-after-free) or pair an old resource snapshot with a new engine. execute_with_trace now does the engine-null check, the resource_manager_->snapshot() and the execute inside ONE engine_mu_ shared-lock window, reporting an unloaded engine via the new ExecuteResult.engine_not_loaded flag. The caller-side pre-check and relock windows are gone: handle_execute maps the flag to the same 503 as before, custom routes forward it to Egress as "engine not loaded". run_count now increments only when an engine actually executes.
FilterInputStream.skip delegates straight to the underlying stream, so an Ingress calling skip()/skipNBytes() advanced past the body cap unmetered — max_request_body_size could be bypassed by skipping the request prefix instead of reading it. skip now counts against the limit exactly like read, and both bulk read and skip are clamped to remaining+1 so the underlying stream is never consumed far past the cap. New RouteTest case: an Ingress that skipNBytes(1000) over a 16-byte cap gets the central 413 and Egress never runs.
…reload (review) Restart the three servers with watch enabled and hammer /api/echo while touching the config to force reloads: every response must stay 200 and reload_count must advance. Guards the C++ lock-window regression (resource manager snapshotted outside the engine lock) under real reload interleaving.
…(review) - The "cp: same file" explanation claimed the error means identical content; GNU cp does not compare content — it means source and target resolve to the same file identity (same path or inode). Corrected in standard-workflow.md and the reflection, with guidance to diagnose via normalized paths / stat inode and to compare content explicitly with cmp/diff. - Replace this round's hard-coded runtime-count wording (三引擎/三端) with 各运行时 or the explicit Go/Java/C++ list per llmdoc/must/conventions.md; also fixed the three-runtime comment in server.cpp.
Contributor
🔍 PR 增量审查
本次增量是对上一轮深度 review 遗留的两个并发/绕过洞的收尾修复：C++ 把引擎判空、资源快照、执行收进单一锁窗口，Java 给 逐项确认：
前五轮审查关于三引擎行为对齐、引用计数快照 + CAS acquire 并发语义、
|
Reflection gains a third-round section: the lock-window regression introduced by the second-round fix itself (re-review your own diff when changing lock structure), wrapper streams must meter every advancing method (skip, not just read), documented tool-behavior claims need verification before they land in llmdoc, and the runtime-count literal rule needs to fire at writing time. index synced.
Contributor
🔍 PR 增量审查
本轮增量仅一个纯文档 commit（ 自上次审查截止点
代码无问题，已检查增量改动。 |
…review) The previous shape (touch, then a serial curl burst, then sleep) could pass with zero overlap: the 2s watcher could reload during the sleep after the burst finished, and all three servers shared one config file so an earlier arm's touches pre-satisfied reload_count >= 1 for later arms — the check never actually pinned the lock-window race it claims to guard. Now each arm: takes its own server's reload_count baseline, runs three background workers hammering /api/echo continuously, touches a per-server config file and polls until THIS server's counter strictly increases past the baseline (20s deadline), keeps hammering briefly past the observed reload, then stops workers and asserts zero non-200 responses. At least one reload demonstrably completes while requests are in flight, and cross-arm counter pollution is impossible.
Contributor
🔍 PR 增量审查
本轮增量仅一个纯测试 commit（
|
Fourth-round reflection: a concurrency regression test must prove overlap (baseline -> background hammer -> poll until the counter strictly passes the baseline mid-hammer), not assume it — an assertion that also passes under fully serial scheduling has not pinned the race. Shared mutable config files cross-pollute per-arm counters; use per-server copies. index synced.
Contributor
🔍 PR 增量审查
本轮增量仅一个纯文档 commit（ 自上次审查截止点
前几轮审查关于三引擎行为对齐、引用计数快照 + CAS acquire 并发语义、 代码无问题，已检查增量改动。 |
Contributor
🔍 PR 增量审查
本轮增量仅一个纯版本号 commit（ 自上次审查截止点
前几轮审查关于三引擎行为对齐、引用计数快照 + CAS acquire 并发语义、 代码无问题，已检查增量改动。 |
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
1 participant
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.
Closes #169
Summary
Upstreams the downstream
serverpluspackage as a strict superset of the existing server, across all three engines (pine-go / pine-java / pine-cpp):Route{Method, Path, Ingress, Egress}: Ingress converts the raw HTTP request into an engine request, Egress owns the entire HTTP response. Startup validation rejects built-in endpoint conflicts, duplicates, malformed paths and nil adapters, with byte-identical error strings across engines.Config.Watchin Go,setWatchin Java,ServerConfig.watchin C++). Default stays enabled for backward compatibility.NewServer(cfg)+Server.Execute(ctx, req)+Server.Acquire() *Handle+Server.Close();Run()is reimplemented on top. All failure paths return errors instead oflog.Fatal.load()+execute(common, items)+acquire() → HandleonPineServer.src/, notinclude/pine/); it keeps the existingshared_mutexlifecycle and gets Route + Watch black-box parity only.pine_http_requests_total//stats.httpwhile unknown paths still collapse to_other.Bug fix (found during parity work)
PineServer.stop()released the baseline snapshot but never cleared thesnapshotreference. Any lateracquireSnapshot()would spin forever on the retired snapshot (refs=0, CAS acquire always fails) instead of reporting engine-not-loaded. NowgetAndSet(null), matching GoClose()'sSwap(nil).Testing
routes_test.go— validateRoutes matrix, Watch toggle, Execute/Acquire/Close lifecycle, routeHandler 405/ingress-error/execute paths. Fullgo test ./...green.RouteTest.java— 21 cases mirroring the Go suite. Full suite 290 tests green.test_routes.cpp— 10 doctest cases pinning validation error strings + normalize_path. WERROR build clean, ctest green.-demo-routes -watch=falseand checks: echo 200 body parity, wrong-method 405, ingress-error 400, built-ins unaffected, unknown 404, custom path metrics label, watch=false keeps reload_count at 0. Full 20-section run green (55 PASS), including byte-exact/executeand server-http sections.make fuzz(Go 30s + Jazzer) clean.Compatibility
Zero custom routes + default Watch → behavior is byte-identical to the previous server.
Run(cfg)signature unchanged; Java/C++ CLI entrypoints unchanged unless the new flags/properties are passed.