Skip to content

Commit 14b6dd9

Browse files
committed
fix(ipc): foundation review remediation
Addresses the full ipc-codegen/ipc-runtime review (68 findings): - runtime: NAPI lifecycle (close/join, TSFN leak, sync deadline), zero-length response deadlock, u64 timeout truncation, max-frame guards, crash cleanup, unified constants, UDS timeouts, new cpp/rust/ts test suites - codegen: schema validation (required error variant, name pairing, reserved words), unified server error wrapping, zig comptime handler injection, uniform --strip-method-prefix, skeleton/dead-code removal - wire fixes: rust Option<bytes>/[bytes;N] serde, ts u64 bigint bridging + bin32 guards, zig signed-int tolerance, cpp union ordering + PrimAlias so schema reflection of generated types round-trips aliases - tests: golden corpus extended (error variant pinned) with cpp/zig runners, error-path + boundary-value steps in every matrix combo, FFI compile checks
1 parent 6a51c53 commit 14b6dd9

101 files changed

Lines changed: 3965 additions & 2254 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

ipc-codegen/SCHEMA_SPEC.md

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,25 @@ from C++ type metadata via the `MsgpackSchemaPacker` infrastructure.
3131
```
3232

3333
- `commands` and `responses` are both **NamedUnion** types (see below).
34-
- Commands and responses are positionally paired: the Nth command corresponds to the Nth
35-
non-error response. The error response (ending in `ErrorResponse`) is shared across all commands.
34+
- Commands and responses are paired by name: command `Foo` corresponds to the
35+
response named `FooResponse`. The error response (ending in `ErrorResponse`)
36+
is shared across all commands.
37+
38+
### Validation rules
39+
40+
Schemas are validated at generation time; violations are hard errors:
41+
42+
- Exactly one response variant named `*ErrorResponse` must exist, with
43+
exactly one field `message: string`. Generated servers wrap handler
44+
failures into this variant; generated clients surface its message.
45+
- Every command `Foo` must have a matching response `FooResponse`, and the
46+
number of commands must equal the number of non-error responses.
47+
- Command names must be unique.
48+
- Response schemas must be struct definitions, not type-name strings.
49+
- Field names must not map (via the snake_case or camelCase projection) to a
50+
reserved word in any target language, and two field names in one struct
51+
must not collapse to the same projected identifier.
52+
- C++ `SERIALIZATION_FIELDS` supports at most 20 fields per struct.
3653

3754
## Type Encodings
3855

ipc-codegen/bootstrap.sh

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -46,10 +46,15 @@ function test_cmds {
4646
local prefix="$hash:CPUS=1:TIMEOUT=120s"
4747
local script="ipc-codegen/echo_example/scripts/run_cross_language_test.sh"
4848

49-
# Golden tests (Rust + TS each verify they can deserialize the goldens
50-
# baked by build()).
49+
# Generator unit tests (schema validation).
50+
echo "$prefix node --experimental-strip-types --no-warnings ipc-codegen/test/schema_visitor.test.ts"
51+
52+
# Golden tests (each language verifies it can deserialize the goldens
53+
# baked by build(), and re-encode them byte-identically).
5154
echo "$prefix $script golden rust"
5255
echo "$prefix $script golden ts"
56+
echo "$prefix $script golden cpp"
57+
echo "$prefix $script golden zig"
5358
echo "$prefix ipc-codegen/echo_example/cpp/build/bin/schema_reflection_test --schema ipc-codegen/echo_example/schema/schema.json"
5459
echo "$prefix ipc-codegen/echo_example/ts_package/test.sh uds"
5560
echo "$prefix ipc-codegen/echo_example/ts_package/test.sh shm"
@@ -71,14 +76,12 @@ function test_cmds {
7176
done
7277
done
7378

74-
# TS SHM client coverage requires the NAPI addon built by
75-
# ipc-runtime/bootstrap.sh under ts/build/<arch>-<os>/.
76-
local napi_dir="$(cd ../ipc-runtime/ts 2>/dev/null && pwd)/build"
77-
if [ -d "$napi_dir" ] && compgen -G "$napi_dir/*/ipc_runtime_napi.node" > /dev/null; then
78-
for server in "${shm_server_langs[@]}"; do
79-
echo "$prefix $script matrix $server ts shm"
80-
done
81-
fi
79+
# TS SHM client coverage. The NAPI addon is built by ipc-runtime/bootstrap.sh
80+
# during build(); a missing addon should fail these tests loudly rather than
81+
# silently dropping coverage.
82+
for server in "${shm_server_langs[@]}"; do
83+
echo "$prefix $script matrix $server ts shm"
84+
done
8285
}
8386

8487
function test {
Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,2 @@
11
build/
2-
echo_client
3-
echo_server
42
src/generated/

ipc-codegen/echo_example/cpp/CMakeLists.txt

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,3 +50,6 @@ target_link_libraries(echo_client PRIVATE echo_common ipc_runtime)
5050

5151
add_executable(schema_reflection_test src/schema_reflection_test.cpp)
5252
target_link_libraries(schema_reflection_test PRIVATE echo_common)
53+
54+
add_executable(golden_test src/golden_test.cpp)
55+
target_link_libraries(golden_test PRIVATE echo_common)

ipc-codegen/echo_example/cpp/bootstrap.sh

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,10 @@ $NODE "$CODEGEN/src/generate.ts" \
1010
--lang cpp \
1111
--server \
1212
--client \
13-
--uds \
13+
--strip-method-prefix \
1414
--out "$DIR/src/generated" \
1515
--prefix Echo \
1616
--cpp-namespace echo
1717

1818
cmake -S "$DIR" -B "$DIR/build"
19-
cmake --build "$DIR/build" --target echo_server echo_client schema_reflection_test
19+
cmake --build "$DIR/build" --target echo_server echo_client schema_reflection_test golden_test

ipc-codegen/echo_example/cpp/src/echo_client.cpp

Lines changed: 71 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,18 @@
44
#include "generated/echo_ipc_client.hpp"
55

66
#include <array>
7-
#include <cassert>
87
#include <iostream>
98
#include <string_view>
109

10+
// Explicit check (not assert): verification must survive NDEBUG builds.
11+
#define CHECK(cond, label) \
12+
do { \
13+
if (!(cond)) { \
14+
std::cerr << "echo_client(cpp): " << (label) << " FAIL\n"; \
15+
return 1; \
16+
} \
17+
} while (0)
18+
1119
namespace {
1220
echo::Fr test_hash(uint8_t base) {
1321
echo::Fr hash{};
@@ -33,24 +41,26 @@ int main(int argc, char **argv) {
3341

3442
{
3543
auto resp = client.bytes({.data = {0xDE, 0xAD, 0xBE, 0xEF, 0x42}});
36-
assert((resp.data == std::vector<uint8_t>{0xDE, 0xAD, 0xBE, 0xEF, 0x42}));
44+
CHECK((resp.data == std::vector<uint8_t>{0xDE, 0xAD, 0xBE, 0xEF, 0x42}),
45+
"EchoBytes");
3746
std::cerr << "echo_client(cpp): EchoBytes OK\n";
3847
}
3948

4049
{
4150
auto resp =
4251
client.fields({.a = 42, .b = 999999, .name = "hello wire compat"});
43-
assert(resp.a == 42 && resp.b == 999999 &&
44-
resp.name == "hello wire compat");
52+
CHECK(resp.a == 42 && resp.b == 999999 && resp.name == "hello wire compat",
53+
"EchoFields");
4554
std::cerr << "echo_client(cpp): EchoFields OK\n";
4655
}
4756

4857
{
4958
auto resp =
5059
client.nested({.inner = {.values = {{1, 2, 3}, {4, 5}}, .flag = true}});
51-
assert((resp.inner.values ==
52-
std::vector<std::vector<uint8_t>>{{1, 2, 3}, {4, 5}}));
53-
assert(resp.inner.flag == true);
60+
CHECK((resp.inner.values ==
61+
std::vector<std::vector<uint8_t>>{{1, 2, 3}, {4, 5}}),
62+
"EchoNested values");
63+
CHECK(resp.inner.flag == true, "EchoNested flag");
5464
std::cerr << "echo_client(cpp): EchoNested OK\n";
5565
}
5666

@@ -61,13 +71,63 @@ int main(int argc, char **argv) {
6171
.hash = hash,
6272
.maybeHash = second,
6373
.hashes = {hash, second}});
64-
assert(resp.treeId == 7);
65-
assert(resp.hash == hash);
66-
assert(resp.maybeHash == second);
67-
assert((resp.hashes == std::vector<echo::Fr>{hash, second}));
74+
CHECK(resp.treeId == 7, "EchoAliases treeId");
75+
CHECK(resp.hash == hash, "EchoAliases hash");
76+
CHECK(resp.maybeHash == second, "EchoAliases maybeHash");
77+
CHECK((resp.hashes == std::vector<echo::Fr>{hash, second}),
78+
"EchoAliases hashes");
6879
std::cerr << "echo_client(cpp): EchoAliases OK\n";
6980
}
7081

82+
// Optional-absent over live IPC.
83+
{
84+
auto hash = test_hash(0x10);
85+
auto resp = client.aliases({.treeId = 7,
86+
.hash = hash,
87+
.maybeHash = std::nullopt,
88+
.hashes = {hash}});
89+
CHECK(!resp.maybeHash.has_value(), "EchoAliases none");
90+
std::cerr << "echo_client(cpp): EchoAliases none OK\n";
91+
}
92+
93+
// uint64 wire encoding above 2^32 over live IPC.
94+
{
95+
const uint64_t big = (1ULL << 53) - 1;
96+
auto resp = client.fields({.a = 42, .b = big, .name = "big"});
97+
CHECK(resp.b == big, "EchoFields u64");
98+
std::cerr << "echo_client(cpp): EchoFields u64 OK\n";
99+
}
100+
101+
// Optional bytes Some/None and fixed [bytes; 2].
102+
{
103+
auto resp = client.blobs({.maybeData = std::vector<uint8_t>{0xAA, 0xBB},
104+
.parts = {{{1, 2, 3}, {4}}}});
105+
CHECK((resp.maybeData == std::vector<uint8_t>{0xAA, 0xBB}),
106+
"EchoBlobs maybeData");
107+
CHECK((resp.parts == std::array<std::vector<uint8_t>, 2>{{{1, 2, 3}, {4}}}),
108+
"EchoBlobs parts");
109+
auto resp_none =
110+
client.blobs({.maybeData = std::nullopt, .parts = {{{}, {9}}}});
111+
CHECK(!resp_none.maybeData.has_value(), "EchoBlobs none");
112+
std::cerr << "echo_client(cpp): EchoBlobs OK\n";
113+
}
114+
115+
// Server error surfaces with its message.
116+
{
117+
bool threw = false;
118+
std::string message;
119+
try {
120+
client.fail({.message = "deliberate failure"});
121+
} catch (const std::exception &e) {
122+
threw = true;
123+
message = e.what();
124+
}
125+
CHECK(threw, "EchoFail threw");
126+
CHECK(message.find("deliberate failure") != std::string::npos,
127+
"EchoFail message");
128+
std::cerr << "echo_client(cpp): EchoFail OK\n";
129+
}
130+
71131
std::cerr << "echo_client(cpp): all tests passed\n";
72132
return 0;
73133
}

ipc-codegen/echo_example/cpp/src/echo_server.cpp

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
#include "generated/echo_ipc_server.hpp"
66

77
#include <iostream>
8+
#include <stdexcept>
89
#include <string_view>
910

1011
namespace echo {
@@ -38,6 +39,17 @@ wire::EchoAliasesResponse handle_aliases(EchoCtx & /*ctx*/,
3839
.hashes = std::move(cmd.hashes)};
3940
}
4041

42+
template <>
43+
wire::EchoBlobsResponse handle_blobs(EchoCtx & /*ctx*/, wire::EchoBlobs &&cmd) {
44+
return {.maybeData = std::move(cmd.maybeData),
45+
.parts = std::move(cmd.parts)};
46+
}
47+
48+
template <>
49+
wire::EchoFailResponse handle_fail(EchoCtx & /*ctx*/, wire::EchoFail &&cmd) {
50+
throw std::runtime_error(cmd.message);
51+
}
52+
4153
} // namespace echo
4254

4355
int main(int argc, char **argv) {

0 commit comments

Comments
 (0)