build(cli): fix HL_ENABLE_HTTP_SERVER=0 link + cover flavors in CI#33
Merged
Conversation
farkasmark
force-pushed
the
fix/cli-flavor-link
branch
3 times, most recently
from
c0d6355 to
c22898d
Compare
The CLI flavor (HL_ENABLE_HTTP_SERVER=0) neither linked nor ran. Two
distinct bugs, plus no CI coverage so both went unnoticed:
1. Link: always-compiled TUs referenced HTTP-server-gated symbols.
- tool_orchestration.c: gate the bindings that call HTTP-server-only
symbols under #ifdef HL_ENABLE_HTTP_SERVER (l_tool_agent_errors;
the l_tool_dev_* block) plus their registration-table entries.
- Makefile: add runtime/{lua,js}/mod_request.c (entirely streaming-
multipart, HTTP-server-only) to the HL_ENABLE_HTTP_SERVER=0 filter-out
lists, next to its already-filtered caller bindings.c.
2. Runtime SIGABRT on Linux: serve_cli.c created the async worker pool
AFTER applying the phase-2 kernel sandbox. glibc registers a per-thread
rseq area when each worker thread starts; once the pledge seccomp filter
is installed the rseq syscall is rejected and glibc aborts the whole
process ("Fatal glibc error: rseq registration failed"). serve.c creates
its pool (hl_serve_init_infra) BEFORE load + sandbox; reorder serve_cli.c
to match: async ctx + worker pool first, then phase-1 pledge, then app
load, then phase-2 sandbox, then run. Also adds the phase-1
hl_sandbox_apply_pledge() serve_cli was missing, so exec/proc/fork are
blocked during app load like serve.c. macOS has no rseq/seccomp, so
neither bug reproduced there (found via a CI smoke that prints hull's
output on failure).
CI: new "Build flavor" matrix builds each flavor + smoke-tests it (binary
starts via `hull version`; an app.main app returning 7 exits 7, which
exercises serve_cli.c's full sandbox + worker-pool + run path). Covers
HL_ENABLE_HTTP_SERVER=0 (this fix) and HL_ENABLE_DB=0.
Still not covered (separate pre-existing link breaks, NOTE in the matrix):
server-only (HL_ENABLE_HTTP_CLIENT=0) and pure-compute (HL_ENABLE_HTTP=0)
fail on release_io.c whole-file HTTP-client gating (also drops the offline
verify-self helpers); pure-compute additionally needs mbedTLS SHA for core
crypto.
Validated: default build + 50/50 tests unchanged; HL_ENABLE_HTTP_SERVER=0
and HL_ENABLE_DB=0 link, run, and exit 7 (macOS); the Linux rseq path is
validated by the new CI smoke (where the root cause surfaced).
farkasmark
force-pushed
the
fix/cli-flavor-link
branch
from
c22898d to
bfd2095
Compare
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.
The CLI flavor (
HL_ENABLE_HTTP_SERVER=0) didn't link — always-compiled TUs referenced HTTP-server-gated symbols, and CI never built a non-default flavor so it went unnoticed.Fixes:
tool_orchestration.c: gate the tool bindings that call HTTP-server-only symbols under#ifdef HL_ENABLE_HTTP_SERVER—l_tool_agent_errors(hl_agent_errors, dev-server sidecar) and thel_tool_dev_*block (hl_dev_state*, backshull dev --tui), plus their table entries.Makefile: addruntime/{lua,js}/mod_request.c(entirely streaming-multipart, HTTP-server-only) to theHL_ENABLE_HTTP_SERVER=0filter-out lists, next to its already-filtered callerbindings.c.CI: new "Build flavor" matrix job builds + smoke-tests (binary starts;
app.mainruns and its return code propagates) the flavors that build today:HL_ENABLE_HTTP_SERVER=0(this fix) andHL_ENABLE_DB=0.Not covered yet (separate pre-existing breaks, NOTE in the matrix): server-only (
HL_ENABLE_HTTP_CLIENT=0) and pure-compute (HL_ENABLE_HTTP=0) fail onrelease_io.cwhole-file HTTP-client gating (drops the offline verify-self helpers); pure-compute also needs mbedTLS SHA for core crypto.Validated locally: default build + 50/50 tests unchanged;
HL_ENABLE_HTTP_SERVER=0andHL_ENABLE_DB=0both link,hull versionruns, app.main returning 7 exits 7.