fix: DGR-033 repair native worker protocol per cross-review BLOCK
Address the Codex GPT-5.5 review of the standalone fake C++ gRPC Shard worker. Four root protocol defects fixed: - Fail closed before SessionOpen: a per-session `opened` flag gates chunk/decode so no activation bypasses lifecycle, cancellation, epoch or flow-control state (terminal ERROR_CODE_INTERNAL), even when an out-of-band Cancel created placeholder state. - Strict flow-control negotiation: NegotiateFlow takes the strictest of peer-vs-worker bounds (mirrors codec.negotiate_flow_control) and the negotiated per-session max_chunk_bytes is enforced on every bundle instead of trusting the peer proposal. - In-stream ReleaseSignal now erases session state immediately. - SessionOpen rejects incompatible schema, fingerprint, and shard-range identity and reports the worker's own served fingerprint rather than echoing the caller. Adds 9 regression tests (worker suite 18 -> 27). Real gates on the rebuilt pinned-gRPC binary: cmake build exit 0; ctest 2/2; worker pytest 27 passed; harness+protocol 63 passed; compileall 0; diff --check clean; ldd/nm show 0 llama/ggml linkage. DGR-033 passes -> true. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -1,5 +1,6 @@
|
||||
#include "shard_service.h"
|
||||
|
||||
#include <algorithm>
|
||||
#include <chrono>
|
||||
#include <utility>
|
||||
|
||||
@@ -7,6 +8,42 @@ namespace meshnet::worker {
|
||||
|
||||
namespace {
|
||||
|
||||
// The exact identity this fixture worker serves. SessionOpen is validated
|
||||
// against these — not echoed back from the caller — so an incompatible peer
|
||||
// fails closed at open rather than being silently accepted with its own claimed
|
||||
// identity. Kept in one place so GetCapability and the open handshake agree.
|
||||
constexpr const char* kModelArtifactDigest = "sha256:native-test-artifact";
|
||||
constexpr const char* kRuntimeRecipeDigest = "sha256:native-test-recipe";
|
||||
constexpr const char* kRecipeId = "native-test";
|
||||
constexpr const char* kRecipeVersion = "1";
|
||||
constexpr const char* kCatalogueVersion = "1";
|
||||
constexpr uint32_t kShardStartLayer = 0;
|
||||
constexpr uint32_t kShardEndLayer = 32;
|
||||
constexpr uint32_t kShardEffectiveStartLayer = 0;
|
||||
|
||||
void FillWorkerFingerprint(sp::Fingerprint* fp) {
|
||||
fp->set_model_artifact_digest(kModelArtifactDigest);
|
||||
fp->set_runtime_recipe_digest(kRuntimeRecipeDigest);
|
||||
fp->set_recipe_id(kRecipeId);
|
||||
fp->set_recipe_version(kRecipeVersion);
|
||||
fp->set_catalogue_version(kCatalogueVersion);
|
||||
}
|
||||
|
||||
void FillWorkerShardRange(sp::ShardRange* range) {
|
||||
range->set_start_layer(kShardStartLayer);
|
||||
range->set_end_layer(kShardEndLayer);
|
||||
range->set_effective_start_layer(kShardEffectiveStartLayer);
|
||||
}
|
||||
|
||||
// Strictest-of-both bound: the smallest positive of `a`/`b`, or `fallback` when
|
||||
// neither is set. Mirrors the `_min` helper in `native_protocol/codec.py`.
|
||||
uint64_t MinPositive(uint64_t a, uint64_t b, uint64_t fallback) {
|
||||
if (a > 0 && b > 0) return std::min(a, b);
|
||||
if (a > 0) return a;
|
||||
if (b > 0) return b;
|
||||
return fallback;
|
||||
}
|
||||
|
||||
int64_t NowUnixNanos() {
|
||||
return std::chrono::duration_cast<std::chrono::nanoseconds>(
|
||||
std::chrono::system_clock::now().time_since_epoch())
|
||||
@@ -52,16 +89,8 @@ grpc::Status ShardRuntimeServiceImpl::GetCapability(grpc::ServerContext*,
|
||||
const sp::CapabilityRequest*,
|
||||
sp::CapabilityReport* response) {
|
||||
response->set_schema_version(sp::SCHEMA_VERSION_1);
|
||||
sp::Fingerprint* fp = response->mutable_fingerprint();
|
||||
fp->set_model_artifact_digest("sha256:native-test-artifact");
|
||||
fp->set_runtime_recipe_digest("sha256:native-test-recipe");
|
||||
fp->set_recipe_id("native-test");
|
||||
fp->set_recipe_version("1");
|
||||
fp->set_catalogue_version("1");
|
||||
sp::ShardRange* range = response->mutable_shard_range();
|
||||
range->set_start_layer(0);
|
||||
range->set_end_layer(32);
|
||||
range->set_effective_start_layer(0);
|
||||
FillWorkerFingerprint(response->mutable_fingerprint());
|
||||
FillWorkerShardRange(response->mutable_shard_range());
|
||||
response->set_backend("grpc-native-cpp");
|
||||
response->set_device("cpu");
|
||||
response->set_validated(true);
|
||||
@@ -88,6 +117,22 @@ grpc::Status ShardRuntimeServiceImpl::Health(grpc::ServerContext*, const sp::Hea
|
||||
return grpc::Status::OK;
|
||||
}
|
||||
|
||||
FlowLimits ShardRuntimeServiceImpl::NegotiateFlow(const sp::FlowControl& proposed) const {
|
||||
FlowLimits out;
|
||||
out.max_inflight_chunks = static_cast<uint32_t>(MinPositive(
|
||||
proposed.max_inflight_chunks(), limits_.max_inflight_chunks, limits_.max_inflight_chunks));
|
||||
const uint64_t credits = MinPositive(proposed.credits_granted(), limits_.credits_granted,
|
||||
limits_.credits_granted);
|
||||
out.credits_granted =
|
||||
static_cast<uint32_t>(std::min<uint64_t>(credits, out.max_inflight_chunks));
|
||||
out.max_chunk_bytes =
|
||||
MinPositive(proposed.max_chunk_bytes(), limits_.max_chunk_bytes, limits_.max_chunk_bytes);
|
||||
out.max_prefill_chunk_tokens = static_cast<uint32_t>(MinPositive(
|
||||
proposed.max_prefill_chunk_tokens(), limits_.max_prefill_chunk_tokens,
|
||||
limits_.max_prefill_chunk_tokens));
|
||||
return out;
|
||||
}
|
||||
|
||||
uint32_t ShardRuntimeServiceImpl::MarkCancelled(const std::string& route_session_id,
|
||||
const std::string& work_id) {
|
||||
std::lock_guard<std::mutex> lk(sessions_mu_);
|
||||
@@ -119,20 +164,59 @@ grpc::Status ShardRuntimeServiceImpl::Session(
|
||||
case sp::SessionRequest::kOpen: {
|
||||
const sp::SessionOpen& open = request.open();
|
||||
route_session_id = open.route_session_id();
|
||||
|
||||
// Reject an incompatible peer at open rather than mid-generation. The
|
||||
// worker validates the caller's schema, artifact/recipe identity and
|
||||
// requested layer range against its own — it never adopts the caller's
|
||||
// claimed identity.
|
||||
auto reject_open = [&](sp::ErrorCode code, const std::string& detail) {
|
||||
stream->Write(MakeFail(route_session_id, /*work_id=*/"", /*step=*/0, code, detail,
|
||||
/*terminal=*/true, /*retryable=*/false));
|
||||
};
|
||||
if (open.schema_version() != sp::SCHEMA_VERSION_1) {
|
||||
reject_open(sp::ERROR_CODE_SCHEMA_UNSUPPORTED,
|
||||
"worker serves schema version 1 only");
|
||||
return grpc::Status::OK;
|
||||
}
|
||||
const sp::Fingerprint& fp = open.fingerprint();
|
||||
if ((!fp.model_artifact_digest().empty() &&
|
||||
fp.model_artifact_digest() != kModelArtifactDigest) ||
|
||||
(!fp.runtime_recipe_digest().empty() &&
|
||||
fp.runtime_recipe_digest() != kRuntimeRecipeDigest)) {
|
||||
reject_open(sp::ERROR_CODE_FINGERPRINT_MISMATCH,
|
||||
"model artifact or runtime recipe digest does not match this worker");
|
||||
return grpc::Status::OK;
|
||||
}
|
||||
if (open.has_shard_range()) {
|
||||
const sp::ShardRange& r = open.shard_range();
|
||||
const bool within = r.start_layer() >= kShardStartLayer &&
|
||||
r.end_layer() <= kShardEndLayer &&
|
||||
r.start_layer() < r.end_layer() &&
|
||||
r.effective_start_layer() >= r.start_layer() &&
|
||||
r.effective_start_layer() < r.end_layer();
|
||||
if (!within) {
|
||||
reject_open(sp::ERROR_CODE_SHARD_RANGE_MISMATCH,
|
||||
"requested layer range is not served by this worker");
|
||||
return grpc::Status::OK;
|
||||
}
|
||||
}
|
||||
|
||||
// Settle the flow-control window with strict worker bounds, then keep
|
||||
// the negotiated ceilings on the session so every later check enforces
|
||||
// exactly what was agreed — not what the peer proposed.
|
||||
const FlowLimits negotiated =
|
||||
open.has_proposed_flow_control()
|
||||
? NegotiateFlow(open.proposed_flow_control())
|
||||
: limits_;
|
||||
{
|
||||
std::lock_guard<std::mutex> lk(sessions_mu_);
|
||||
SessionState state;
|
||||
state.epoch = open.route_epoch();
|
||||
if (open.has_proposed_flow_control()) {
|
||||
const sp::FlowControl& fc = open.proposed_flow_control();
|
||||
state.credits = fc.credits_granted();
|
||||
state.max_inflight = fc.max_inflight_chunks();
|
||||
state.max_chunk_bytes = fc.max_chunk_bytes();
|
||||
} else {
|
||||
state.credits = limits_.credits_granted;
|
||||
state.max_inflight = limits_.max_inflight_chunks;
|
||||
state.max_chunk_bytes = limits_.max_chunk_bytes;
|
||||
}
|
||||
state.credits = negotiated.credits_granted;
|
||||
state.max_inflight = negotiated.max_inflight_chunks;
|
||||
state.max_chunk_bytes = negotiated.max_chunk_bytes;
|
||||
state.max_prefill_chunk_tokens = negotiated.max_prefill_chunk_tokens;
|
||||
state.opened = true;
|
||||
auto it = sessions_.find(route_session_id);
|
||||
if (it != sessions_.end()) {
|
||||
// A prior out-of-band Cancel may have marked this session cancelled
|
||||
@@ -147,11 +231,7 @@ grpc::Status ShardRuntimeServiceImpl::Session(
|
||||
accepted->set_schema_version(sp::SCHEMA_VERSION_1);
|
||||
accepted->set_route_session_id(open.route_session_id());
|
||||
accepted->set_route_epoch(open.route_epoch());
|
||||
if (open.has_proposed_flow_control()) {
|
||||
*accepted->mutable_flow_control() = open.proposed_flow_control();
|
||||
} else {
|
||||
FillDefaultFlow(accepted->mutable_flow_control(), limits_);
|
||||
}
|
||||
FillDefaultFlow(accepted->mutable_flow_control(), negotiated);
|
||||
if (open.accepted_compression_size() > 0) {
|
||||
for (int c : open.accepted_compression()) {
|
||||
accepted->add_accepted_compression(static_cast<sp::Compression>(c));
|
||||
@@ -159,7 +239,9 @@ grpc::Status ShardRuntimeServiceImpl::Session(
|
||||
} else {
|
||||
accepted->add_accepted_compression(sp::COMPRESSION_NONE);
|
||||
}
|
||||
*accepted->mutable_fingerprint() = open.fingerprint();
|
||||
// Report the fingerprint the worker actually serves, so a mismatch is
|
||||
// visible at open — never a copy of the caller's claimed identity.
|
||||
FillWorkerFingerprint(accepted->mutable_fingerprint());
|
||||
stream->Write(response);
|
||||
break;
|
||||
}
|
||||
@@ -174,28 +256,35 @@ grpc::Status ShardRuntimeServiceImpl::Session(
|
||||
// holding the lock across a (possibly blocking) Write would deadlock an
|
||||
// out-of-band Cancel RPC that needs the same lock.
|
||||
sp::SessionResponse response;
|
||||
bool terminate = false;
|
||||
{
|
||||
std::lock_guard<std::mutex> lk(sessions_mu_);
|
||||
auto it = sessions_.find(route_session_id);
|
||||
SessionState* state = it != sessions_.end() ? &it->second : nullptr;
|
||||
|
||||
if (state && (state->cancelled_session || state->cancelled_work.count(work_id))) {
|
||||
if (state == nullptr || !state->opened) {
|
||||
// Fail closed: an activation before a valid SessionOpen must never
|
||||
// bypass lifecycle, cancellation, epoch or flow-control state.
|
||||
response = MakeFail(route_session_id, work_id, step, sp::ERROR_CODE_INTERNAL,
|
||||
"activation received before SessionOpen", true, false);
|
||||
terminate = true;
|
||||
} else if (state->cancelled_session || state->cancelled_work.count(work_id)) {
|
||||
response = MakeFail(route_session_id, work_id, step, sp::ERROR_CODE_CANCELLED,
|
||||
"work was cancelled", false, false);
|
||||
} else if (state && envelope.route_epoch() < state->epoch) {
|
||||
} else if (envelope.route_epoch() < state->epoch) {
|
||||
response = MakeFail(route_session_id, work_id, step, sp::ERROR_CODE_EPOCH_STALE,
|
||||
"stale route epoch", false, false);
|
||||
} else if (envelope.deadline_unix_nanos() != 0 &&
|
||||
NowUnixNanos() > envelope.deadline_unix_nanos()) {
|
||||
response = MakeFail(route_session_id, work_id, step, sp::ERROR_CODE_DEADLINE_EXCEEDED,
|
||||
"deadline already passed", false, false);
|
||||
} else if (state && state->seen_steps.count(step)) {
|
||||
} else if (state->seen_steps.count(step)) {
|
||||
response = MakeAck(work_id, step, /*duplicate=*/true);
|
||||
} else if (state && state->credits <= 0) {
|
||||
} else if (state->credits <= 0) {
|
||||
response = MakeFail(route_session_id, work_id, step, sp::ERROR_CODE_FLOW_CONTROL_VIOLATION,
|
||||
"no flow-control credit remaining", false, true);
|
||||
} else {
|
||||
const BundleCheck check = engine_.Validate(chunk.bundle());
|
||||
const BundleCheck check = engine_.Validate(chunk.bundle(), state->max_chunk_bytes);
|
||||
if (check.oversize_detail) {
|
||||
response = MakeFail(route_session_id, work_id, step, sp::ERROR_CODE_RESOURCE_EXHAUSTED,
|
||||
*check.oversize_detail, false, false);
|
||||
@@ -203,16 +292,17 @@ grpc::Status ShardRuntimeServiceImpl::Session(
|
||||
response = MakeFail(route_session_id, work_id, step, sp::ERROR_CODE_PAYLOAD_CORRUPT,
|
||||
*check.corrupt_detail, false, false);
|
||||
} else {
|
||||
if (state) {
|
||||
state->seen_steps.insert(step);
|
||||
state->credits -= 1;
|
||||
}
|
||||
state->seen_steps.insert(step);
|
||||
state->credits -= 1;
|
||||
engine_.BoundedForward(chunk.bundle()); // real bounded forward over wire bytes
|
||||
*response.mutable_chunk() = chunk; // echo the exact bundle back
|
||||
}
|
||||
}
|
||||
}
|
||||
stream->Write(response);
|
||||
if (terminate) {
|
||||
return grpc::Status::OK;
|
||||
}
|
||||
break;
|
||||
}
|
||||
|
||||
@@ -230,25 +320,32 @@ grpc::Status ShardRuntimeServiceImpl::Session(
|
||||
}
|
||||
|
||||
sp::SessionResponse response;
|
||||
bool terminate = false;
|
||||
{
|
||||
std::lock_guard<std::mutex> lk(sessions_mu_);
|
||||
auto it = sessions_.find(route_session_id);
|
||||
SessionState* state = it != sessions_.end() ? &it->second : nullptr;
|
||||
|
||||
if (state && (state->cancelled_session || state->cancelled_work.count(work_id))) {
|
||||
if (state == nullptr || !state->opened) {
|
||||
// Fail closed: a decode step before a valid SessionOpen must never
|
||||
// bypass lifecycle, cancellation, epoch or flow-control state.
|
||||
response = MakeFail(route_session_id, work_id, step, sp::ERROR_CODE_INTERNAL,
|
||||
"activation received before SessionOpen", true, false);
|
||||
terminate = true;
|
||||
} else if (state->cancelled_session || state->cancelled_work.count(work_id)) {
|
||||
response = MakeFail(route_session_id, work_id, step, sp::ERROR_CODE_CANCELLED,
|
||||
"work was cancelled", false, false);
|
||||
} else if (step_msg.deadline_unix_nanos() != 0 &&
|
||||
NowUnixNanos() > step_msg.deadline_unix_nanos()) {
|
||||
response = MakeFail(route_session_id, work_id, step, sp::ERROR_CODE_DEADLINE_EXCEEDED,
|
||||
"deadline already passed", false, false);
|
||||
} else if (state && state->seen_steps.count(step)) {
|
||||
} else if (state->seen_steps.count(step)) {
|
||||
response = MakeAck(work_id, step, /*duplicate=*/true);
|
||||
} else if (state && state->credits <= 0) {
|
||||
} else if (state->credits <= 0) {
|
||||
response = MakeFail(route_session_id, work_id, step, sp::ERROR_CODE_FLOW_CONTROL_VIOLATION,
|
||||
"no flow-control credit remaining", false, true);
|
||||
} else {
|
||||
const BundleCheck check = engine_.Validate(bundle);
|
||||
const BundleCheck check = engine_.Validate(bundle, state->max_chunk_bytes);
|
||||
if (check.oversize_detail) {
|
||||
response = MakeFail(route_session_id, work_id, step, sp::ERROR_CODE_RESOURCE_EXHAUSTED,
|
||||
*check.oversize_detail, false, false);
|
||||
@@ -256,10 +353,8 @@ grpc::Status ShardRuntimeServiceImpl::Session(
|
||||
response = MakeFail(route_session_id, work_id, step, sp::ERROR_CODE_PAYLOAD_CORRUPT,
|
||||
*check.corrupt_detail, false, false);
|
||||
} else {
|
||||
if (state) {
|
||||
state->seen_steps.insert(step);
|
||||
state->credits -= 1;
|
||||
}
|
||||
state->seen_steps.insert(step);
|
||||
state->credits -= 1;
|
||||
engine_.BoundedForward(bundle);
|
||||
// No decode response field exists; echo the step back as a
|
||||
// chunk-bearing SessionResponse per the proto's relayed-frame design.
|
||||
@@ -277,6 +372,9 @@ grpc::Status ShardRuntimeServiceImpl::Session(
|
||||
}
|
||||
}
|
||||
stream->Write(response);
|
||||
if (terminate) {
|
||||
return grpc::Status::OK;
|
||||
}
|
||||
break;
|
||||
}
|
||||
|
||||
@@ -308,6 +406,14 @@ grpc::Status ShardRuntimeServiceImpl::Session(
|
||||
|
||||
case sp::SessionRequest::kRelease: {
|
||||
const sp::ReleaseSignal& release = request.release();
|
||||
// An explicit release drops session state immediately (KV, credits,
|
||||
// dedup) instead of holding it for the TTL — the whole point of the
|
||||
// signal. Erase the session this stream opened so its resources are
|
||||
// freed the moment the terminal status is sent.
|
||||
{
|
||||
std::lock_guard<std::mutex> lk(sessions_mu_);
|
||||
sessions_.erase(route_session_id);
|
||||
}
|
||||
sp::SessionResponse response;
|
||||
sp::ShardStatus* status = response.mutable_status();
|
||||
status->set_work_id(release.work_id());
|
||||
|
||||
Reference in New Issue
Block a user