feat: expose per-role residency over /hive/native-telemetry (HV-3 prerequisite) - #92
Conversation
…requisite) HV-3 (docs/NATIVE_RUNTIME_HIVE_VALIDATION_PLAN.md) asserts that a role's residency returns to baseline BETWEEN jobs while its VRAM reservation does not -- the documented decoupling between the two. On a headless worker that assertion was unobservable: the daemon wired NativeTelemetryProvider to GetReservationSnapshot() alone, so ActiveCount/ConversationsCreated/Status existed in-process and nowhere else. Same shape of gap HV-2 found and closed for admission counters, one level down. The response stays additive. Reservation fields keep their exact top-level position because Tools/Hv2SchedulingRunner binds them there, and HV-6 re-runs that driver unattended 3x -- nesting them to make the payload tidier would break a driver the campaign depends on for no gain. AdapterRoleResidency.Binding is projected to display names rather than serialized whole: it carries absolute GGUF/adapter paths and this endpoint is unauthenticated (same posture as /hive/info). The phase asserts on role, counts and status; the worker's local filesystem layout is not part of that and does not need publishing to any caller that can reach the port. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Same submit -> poll -> evidence-JSON shape as the HV-1 and HV-2 drivers.
--phase sequential is the phase's core assertion and the reason residency had to
become remotely observable at all: N jobs dispatched to one worker ONE AT A TIME,
with a /hive/native-telemetry sample between each, checking three things the plan
states in prose and nothing so far has actually measured:
residency-returns-to-baseline ActiveCount back to 0 after each job
reservation-persists-between-jobs reservedBytes stays above the pre-run
baseline across the gaps -- the half of the documented decoupling that
would be invisible if we only checked that things return to zero
fresh-conversation-per-job ConversationsCreated strictly increases, which
is what separates "a fresh conversation each job" from "one conversation
silently reused and never re-counted"
One work unit per campaign, awaited to terminal before the next is submitted:
submitting all N up front would let the Warchief overlap them and destroy the
very thing the phase measures.
--phase concurrent folds in the deferred Phase D "second concurrent role"
increment -- two roles dispatched to the SAME worker at once, with telemetry
polled throughout. It records the PEAK distinct resident roles rather than a
final sample, because by the time both jobs are terminal the second role is
already disposed and an after-the-fact sample can never witness the overlap.
HV-3's third item (forced role recycle across machines) is NOT covered and is
recorded in every evidence file's uncoveredItems so a green run cannot be
mistaken for full HV-3 coverage. MarkRoleDegraded is reachable only from the
runtime's own NoKvSlot handling; a remote trigger is a MUTATION needing an
authenticated control endpoint and its own security review, not something to
smuggle in alongside a read-only campaign driver.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…he live probe Found by HV-3's first real run on HardcorePC (RTX 3050, 6 GB). The published telemetry read: totalBytes 6442450944 (6.44 GB) reservedBytes 11039541760 (11.04 GB) availableBytes 0 Reserved exceeded the card's total by 4597090816 bytes, matching that job's est_vram=4.6GB exactly. GetReservationSnapshot computed `baseline.ReservedBytes + ledger.Sum()`. Those two measure overlapping things: the daemon and the app both supply a live whole-GPU probe (NativeVramProbe / nvidia-smi), whose ReservedBytes ALREADY counts every resident model, and the ledger entries are this orchestrator's own accounting of those same models. This is the sibling of the HV-1 bug that EnsureAdmitted documents at length -- same double-count, in the reporting path rather than the admission path. Admission itself was never affected (EnsureAdmitted does its own accounting and already credits the overlap out), which is why sequential jobs kept being admitted normally while the telemetry published an impossible number. That makes this purely a diagnosability defect -- and §6's entry criteria include consistent telemetry and diagnosability across machines, so it has to be right before HV-5 sweeps these values fleet-wide. Fix: take the MAX of the two rather than the sum. With a live probe the probe wins, since it is authoritative for what is physically in use (including non-TheOrc consumers on the same card). With the static fallback budget -- VramBudget(total, ReservedBytes: 0), used when nvidia-smi is unavailable -- the ledger wins, which is the only signal available there. Accepted imprecision, documented at the call site: a role that is RESERVED but whose model is not yet resident no longer adds on top of the probe, so this can under-report during the window between reservation and load. Under-reporting a telemetry read is strictly safer than publishing a number the hardware cannot produce. Regression test reproduces the exact shape with the same stateful stand-in for the live probe the HV-1 test uses (idle -> resident); confirmed red before the fix on the "reserved must never exceed the card's total" assertion, green after. Full RuntimeOrchestrator suite 13/13 with THEORC_TEST_GGUF set. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Same discipline as the HV-1/HV-2 entries: what ran, what passed, what did not, and what was found along the way. HV-3 is recorded as NOT CLOSED - the sequential lifecycle passes on HardcorePC, but §6's criterion is lifecycle behavior ACROSS machines and the laptop is not yet in a state that can produce valid evidence, so a one-box pass is not promoted to a closure. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… warm worker The check asserted after-cycle reservedBytes > baseline. That only holds when the run starts against a COLD worker; against one still warm from a previous run the baseline sample already includes the resident model, so equality is the correct observation and the strict comparison fails on correct behavior. Caught on the first two-machine run, where the same behavior produced opposite verdicts purely from starting state: HARDCOREPC (warm, baseline 5589043712) FAILED while HardcoreLaptopMSI (freshly started, baseline 45088768) PASSED. "Persists" means the reservation never DROPS across the gaps between jobs, so that is what it now checks -- non-decreasing across cycles and at least the baseline, which holds from either starting state. A worker that never loaded anything would satisfy non-decreasing trivially, so it additionally requires a loaded model to have actually been observed (ConversationsCreated > 0) rather than passing on an idle card. The detail string now records which starting state the run saw, since that changes what the numbers mean to a later reader. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…mission defect Records the laptop's silent wrong-model binding (recursive ModelDepot scan picking a 270MB onboarding GGUF over the 4.68GB coder), the two-machine sequential pass, the driver check that was wrong for warm workers, and the cross-role admission double-count the concurrent phase exposed. HV-3 stays NOT CLOSED. The concurrent-role item is now blocked on a real product defect rather than on missing tooling, which is a more useful thing to know. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
HV-3's concurrent-role phase was denied on both fleet workers with a physically impossible budget: "Budget total=6.0 GB, reserved=10.3 GB, available=0.0 GB" on a 6 GB card. Two compounding errors, both here. 1. EnsureAdmitted summed the live whole-GPU probe with the ledger. The probe already counts EVERY resident model -- this role's and every other role's -- so adding otherRolesReserved on top charged the others a second time. Same overlap as the reporting-snapshot defect, in the admission path. Now takes the MAX of (probe minus this role's own prior footprint) and (other roles' ledger). The subtraction stays: that is the HV-1 fix, and it is why the two operands are not symmetric. The ledger arm remains the floor for the static VramBudget(total, ReservedBytes: 0) fallback, where no probe exists. 2. Both roles resolve to the SAME GGUF and SessionManager keeps ONE shared base load, yet the second role was charged a full fresh-load estimate -- billing an entire extra model for something that only costs its own context. Admission now asks SessionManager whether the load will actually reuse resident weights, via the same predicate LoadBindingAsync itself uses so the estimate cannot drift from what happens next, and drops the base weights and the per-process CUDA overhead when it will. The discount applies ONLY on the context-aware estimate path. On the two legacy file-size-only returns there is no term representing the context, so dropping the base there would charge a reusing role nothing at all and let roles over-commit without bound. Caught by EnsureAdmitted_TracksReservationsAcrossRoles_DeniesSecondRoleWhenBudgetExhausted going red mid-change -- that protection is exactly what must not regress, and the first cut of this fix broke it. EnsureAdmitted now RETURNS the admitted footprint and the caller records that verbatim, instead of recomputing at commit time. The estimate depends on residency, and the load in between is precisely what changes residency, so a recompute would file a first-ever load as though it had reused weights it actually paid for -- under-reporting it to every later role. New regression test holds two roles on one resident base concurrently and asserts the second is admitted, reserved stays under total, and the second role's ledger entry is smaller than a whole model. Native/HIVE suite 247/247 with THEORC_TEST_GGUF set. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…multaneous execution
Established by reading HiveWorkerAgent rather than inferred from a red run: the
worker's main loop is `while { PollLease -> await ClaimAndExecute }` -- strictly
one task at a time. Two work units dispatched to the same worker therefore
execute SERIALLY, and no amount of polling will ever observe two roles with
ActiveCount > 0 simultaneously. That is an architectural property of the dispatch
layer, not a scheduling or admission defect -- the same class of finding as HV-2's
"a Daemon-hosted Warchief cannot approve pairing".
The old check asserted peak simultaneous RESIDENCY >= 2, which that architecture
can never satisfy, so it was testing something the system does not claim to do.
What HV-3 actually asks for here is cross-role admission accounting, and that IS
observable: a reservation persists while the model stays loaded, outliving the
conversation that created it -- the documented decoupling this entire phase is
built on. Two roles genuinely hold reservations concurrently against one live
budget. Confirmed live on HardcorePC after the admission fix:
reservations [Worker 5589043712, Researcher 637534208], reservedBytes 6226577920
against totalBytes 6442450944, rejectedAdmissionCount 0 -- the second role
charged its real incremental context cost rather than a whole extra model.
Adds a second check that watches reservedBytes <= totalBytes across EVERY sample
rather than only at the end, so the cross-role double-count cannot silently
return between polls. The reported detail also records peak simultaneous
residency and states that 1 is expected, so a later reader is not left wondering
whether the phase quietly regressed.
start-warchief.bat: the Warchief must be launched detached. Running swarmcli from
an interactive shell does not survive -- the process is reaped when the launching
shell's tree ends, which killed it mid-campaign twice and produced a false
"heartbeat timeout" failure that looked like a concurrency defect but was just
the worker having nothing to heartbeat to.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
--timeout on `swarmcli --warchief` is a SELF-TERMINATE, not an idle timeout: the process exits the moment it elapses, mid-campaign, printing a clean "Shutting down..." that looks nothing like a failure. start-warchief.bat pinned it at 7200s, so a session ended silently at exactly the two-hour mark and the resulting symptom -- workers no longer leasing -- was misdiagnosed twice as worker or pairing trouble before the timestamps made it obvious (started 13:52:07, exited 15:52:08). Raised to 86400 and documented at the call site. Also gitignores warchief.log/worker_*.log. These are written into the repo root by the start scripts AND are fully buffered while the process runs, so they show only the startup banner until exit. That cost a wrong call too: an apparently empty log was read as "the worker never leased" and a running campaign was killed on the strength of it. The comment says so, so the next reader checks /hive/native-telemetry instead. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
HiveIdentity.LeaveHive existed but was reachable only from the GUI's HivePanel. That strands a headless worker permanently the moment its HiveId diverges from the Warchief's: §4.3 refuses to bridge two hives, so pairing can never recover it and there is no other operator surface. Both fleet workers hit exactly that during HV-3 -- re-pair returned "already belongs to a different hive" on machines with no screen to click. This does not weaken §4.3's "no silent bridge" guarantee. Leaving stays an explicit, deliberate operator action; it just becomes one performable on a machine that has no display. Pairing still never leaves a hive on its own, and --leave-hive alone is refused -- it prints what would be abandoned and requires --yes, because the spec leans on a human dialog for intent and a headless box has none. Membership only: NodeId, signing/exchange keys and existing peer secrets all survive; the own-membership cert is cleared since the hive that issued it is the one being left. Separately, the HV-3 driver's residency check could report PASS on a run where nothing executed. "ActiveCount is 0" is trivially true on a worker that never ran anything, and it went green on both machines during a run where every job sat unclaimed and telemetry was unreachable. A green tick on an empty run is worse than no check because it reads as evidence. It now requires ConversationsCreated > 0 as liveness proof and reports VACUOUS otherwise. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR updates native VRAM admission for resident base weights, expands Hive telemetry and diagnostics, adds HV-3–HV-6 validation drivers, introduces recovery and repeatability tooling, and adds hive departure, warchief storage, detached startup, and runtime log exclusions. ChangesNative runtime admission and validation
Hive diagnostics and failure handling
Validation drivers
Hive operations
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant ValidationRunner
participant Warchief
participant Worker
participant NativeTelemetry
ValidationRunner->>Warchief: Submit validation campaign
Warchief->>Worker: Assign native work units
Worker->>Warchief: Send heartbeats and task results
ValidationRunner->>Warchief: Poll task status
ValidationRunner->>NativeTelemetry: Fetch native telemetry
NativeTelemetry-->>ValidationRunner: Return reservations and residency
ValidationRunner-->>ValidationRunner: Evaluate checks and write evidence
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (1)
OrchestratorIDE/Core/Runtime/RuntimeOrchestrator.cs (1)
317-352: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueEstimate is computed twice per admission.
requiredBytesis computed directly viaOrcScheduler.EstimateRequiredBytes(...), then_scheduler.TryAdmit(binding, budget, options, reusesBaseWeights)recomputes the identical estimate internally to reach its decision — including re-parsing the GGUF header (GgufMetadataReader.TryRead) and redoing the KV/compute-buffer math, on every single conversation admission. Correctness is unaffected (both calls use the same inputs), but it's avoidable per-request work on what can be a hot path.Consider having
TryAdmit(or an overload) return/accept the pre-computed byte estimate soEnsureAdmitteddoesn't pay for the GGUF header parse twice.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@OrchestratorIDE/Core/Runtime/RuntimeOrchestrator.cs` around lines 317 - 352, Avoid calculating the admission estimate twice in EnsureAdmitted: reuse the requiredBytes value produced by OrcScheduler.EstimateRequiredBytes when calling _scheduler.TryAdmit, either by adding an estimate parameter or returning the estimate from TryAdmit. Preserve the existing admission decision and returned-byte behavior while ensuring GGUF parsing and KV/compute-buffer calculations occur only once per admission.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/NATIVE_RUNTIME_HIVE_VALIDATION_PLAN.md`:
- Around line 512-522: The HV-3 verdict’s Outstanding list skips item (b);
update the enumeration in the section around the HV-3 verdict to use the correct
consecutive item labels, preserving the existing descriptions and scope of the
concurrent second-role and forced role recycle items.
In `@OrchestratorIDE.Daemon/HiveService.cs`:
- Around line 232-253: Update the NativeTelemetryProvider reservation projection
in HiveService to default TotalBytes, ReservedBytes, and AvailableBytes to 0
when the reservation snapshot is null, while leaving Reservations nullable.
Preserve the existing telemetry structure and other field mappings.
In `@OrchestratorIDE.Daemon/Program.cs`:
- Line 40: Update the --leave-hive handling in the argument dispatch to use a
strict HiveIdentity load path that does not regenerate or persist an identity
when decryption or deserialization fails. Catch protector/load failures, report
the error, and exit with a non-zero status; preserve the existing behavior for
--show-identity and --pair.
- Around line 66-93: Update the --leave-hive branch to validate arguments before
any confirmation or mutation: require it to be the sole operational mode, allow
only its explicitly supported flags (including --yes), and reject unknown or
mixed-mode arguments with the existing validation behavior. Ensure invalid
invocations return before identity loading or LeaveHive(), while valid
--leave-hive [--yes] flows retain their current confirmation and success
behavior.
- Around line 88-90: Update the --leave-hive flow around HiveIdentity.LeaveHive
and HiveService coordination so hive membership removal cannot occur while the
daemon is using a cached identity. Require the daemon service to be stopped
before persisting the removal, or implement an explicit coordinated IPC/reload
path that refreshes the daemon’s identity before completion.
In `@start-warchief.bat`:
- Around line 4-8: Update the Warchief launch command in start-warchief.bat to
invoke swarmcli.exe through a detached start with a child cmd /c, rather than
directly from the foreground shell. Keep the existing arguments and ensure log
redirection remains inside the detached child command.
In `@Tools/Hv3LifecycleRunner/Program.cs`:
- Around line 179-259: Scope lifecycle telemetry to the roles under test instead
of aggregating all roles. Update EvaluateSequential and its RunSequentialAsync
call site to accept the tested role and derive MaxConversationsCreated and
related liveness values from matching Residency entries. In RunConcurrentAsync,
project ReservationEntry.Role to display-name strings server-side, then filter
peakReserved and peakReservedRoles to primaryRole and secondaryRole before
evaluating admission. Preserve the existing lifecycle checks while ensuring
stale reservations or conversations from other roles cannot affect their
results.
---
Nitpick comments:
In `@OrchestratorIDE/Core/Runtime/RuntimeOrchestrator.cs`:
- Around line 317-352: Avoid calculating the admission estimate twice in
EnsureAdmitted: reuse the requiredBytes value produced by
OrcScheduler.EstimateRequiredBytes when calling _scheduler.TryAdmit, either by
adding an estimate parameter or returning the estimate from TryAdmit. Preserve
the existing admission decision and returned-byte behavior while ensuring GGUF
parsing and KV/compute-buffer calculations occur only once per admission.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: f60cbb46-a5c9-459f-9dc9-1bb6e3823be2
📒 Files selected for processing (12)
.gitignoreOrchestratorIDE.Daemon/HiveService.csOrchestratorIDE.Daemon/Program.csOrchestratorIDE.UnitTests/RuntimeOrchestratorTests.csOrchestratorIDE/Core/Runtime/OrcScheduler.csOrchestratorIDE/Core/Runtime/RuntimeOrchestrator.csOrchestratorIDE/Core/Runtime/SessionManager.csOrchestratorIDE/Services/Hive/HiveNodeServer.csTools/Hv3LifecycleRunner/Hv3LifecycleRunner.csprojTools/Hv3LifecycleRunner/Program.csdocs/NATIVE_RUNTIME_HIVE_VALIDATION_PLAN.mdstart-warchief.bat
| _nodeServer.NativeTelemetryProvider = () => | ||
| { | ||
| var reservation = nativeRuntime.GetReservationSnapshot(); | ||
| return new | ||
| { | ||
| reservation?.Reservations, | ||
| TotalBytes = reservation?.TotalBytes, | ||
| ReservedBytes = reservation?.ReservedBytes, | ||
| AvailableBytes = reservation?.AvailableBytes, | ||
| RejectedAdmissionCount = reservation?.RejectedAdmissionCount, | ||
| LastRejectionReason = reservation?.LastRejectionReason, | ||
| Residency = nativeRuntime.GetResidencySnapshot().Select(r => new | ||
| { | ||
| Role = r.Role.ToString(), | ||
| BaseModel = r.Binding.BaseModel.DisplayName, | ||
| Adapter = r.Binding.Adapter?.DisplayName, | ||
| r.ActiveCount, | ||
| r.ConversationsCreated, | ||
| Status = r.Status.ToString(), | ||
| }).ToList(), | ||
| }; | ||
| }; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n "GetReservationSnapshot" -B2 -A15 --type=csRepository: hardcoreerik/TheOrc
Length of output: 157
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== repo files count =="
git ls-files | wc -l
echo "== locate HiveService.cs =="
fd -a 'HiveService.cs|Program.cs' . | sed 's#^\./##'
echo "== search NativeTelemetry/Reservation/TelemetryProvider =="
rg -n "NativeTelemetry|NativeTelemetryProvider|Reservation|Reservations|AvailableBytes|TotalBytes|RejectedAdmissionCount" -S . --glob '!packages/**' --glob '!bin/**' --glob '!obj/**' | head -200Repository: hardcoreerik/TheOrc
Length of output: 24931
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== HiveService relevant section =="
cat -n OrchestratorIDE.Daemon/HiveService.cs | sed -n '210,260p'
echo "== RuntimeOrchestrator GetReservationSnapshot implementation =="
cat -n OrchestratorIDE/Core/Runtime/RuntimeOrchestrator.cs | sed -n '400,490p'
echo "== Hv3LifecycleRunner NativeTelemetry and TryFetchTelemetryAsync =="
cat -n Tools/Hv3LifecycleRunner/Program.cs | sed -n '300,500p'
echo "== System.Text.Json behavior probe for long null =="
cat >/tmp/json_probe.cs <<'CS'
using System;
using System.Text.Json;
var jsonOk = "{\"TotalBytes\":0}";
var jsonNull = "{\"TotalBytes\":null}";
var modelType = typeof(MyModel);
try
{
var ok = JsonSerializer.Deserialize<MyModel>(jsonOk);
Console.WriteLine($"OK: {ok.TotalBytes}");
}
catch (Exception ex)
{
Console.WriteLine($"OK failed: {ex.GetType().Name}: {ex.Message}");
}
try
{
var nil = JsonSerializer.Deserialize<MyModel>(jsonNull);
Console.WriteLine($"NULL: {nil.TotalBytes}");
}
catch (Exception ex)
{
Console.WriteLine($"NULL failed: {ex.GetType().Name}: {ex.Message}");
}
public sealed class MyModel { public long TotalBytes { get; set; } }
CS
if command -v dotnet >/dev/null 2>&1; then
dotnet run --project /tmp/json_probe.cs 2>&1 || true
else
echo "dotnet not available"
fiRepository: hardcoreerik/TheOrc
Length of output: 20164
🌐 Web query:
System.Text.Json Deserialize long null NullableObjectConverter default JSON null long Error JsonException
💡 Result:
In System.Text.Json, attempting to deserialize a JSON null value into a non-nullable value type (such as long) will throw a JsonException because the internal converter for the type cannot map null to a value type [1][2]. To handle this behavior, you have several primary options: 1. Use Nullable Types: The most idiomatic approach is to change your property type from long to long? (Nullable). System.Text.Json automatically handles nulls for nullable types by assigning them a null value [1][2]. This is the recommended solution for most scenarios [3]. 2. Use a Custom Converter: If you must use a non-nullable type but need to support JSON nulls (e.g., to assign a specific default value), you can implement a custom JsonConverter. By default, the serializer skips calling the converter for null values [1][2]. To change this, you must override the HandleNull property to return true in your custom converter [1][2]. Example of a custom converter that handles null: public class LongConverter: JsonConverter { public override bool HandleNull => true; public override long Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) { if (reader.TokenType == JsonTokenType.Null) { return 0; // Return your desired default value } return reader.GetInt64; } public override void Write(Utf8JsonWriter writer, long value, JsonSerializerOptions options) { writer.WriteNumberValue(value); } } Important Considerations: - Performance: System.Text.Json optimizes performance by skipping calls to custom converters for null values [1][2]. Overriding HandleNull removes this optimization, so it should only be used when necessary [1][2]. - JsonIgnoreCondition: While JsonIgnoreCondition.WhenWritingNull exists to control serialization, there is no built-in JsonIgnoreCondition.WhenReadingNull to ignore nulls during deserialization; the property will either be set to null (if nullable) or throw an exception (if non-nullable) [4][3]. - Migrating from Newtonsoft: Newtonsoft.Json often handles nulls more permissively depending on configuration. If you are migrating, check your existing attributes and settings, as System.Text.Json enforces stricter type safety [5].
Citations:
- 1: https://github.com/dotnet/docs/blob/main/docs/standard/serialization/system-text-json/converters-how-to.md
- 2: https://learn.microsoft.com/en-us/dotnet/standard/serialization/system-text-json/converters-how-to
- 3: System.Text.Json throws when deserializing JSON
nullinto struct properties. dotnet/runtime#83706 - 4: System.Text.Json -> Deserialize Null to Integer Property dotnet/runtime#74008
- 5: https://learn.microsoft.com/en-us/dotnet/standard/serialization/system-text-json/deserialization
Default reserved-byte fields before serializing the telemetry snapshot.
RuntimeOrchestrator.GetReservationSnapshot() can return null when there is no scheduler/budget provider or when the provider throws/returns null. In that case, reservation?.TotalBytes, reservation?.ReservedBytes, and reservation?.AvailableBytes become nullable and are written as JSON null; Tools/Hv3LifecycleRunner/Program.cs’s NativeTelemetry has these non-nullable long properties, so TryFetchTelemetryAsync treats the read as Reachable=false and later maps values to -1.
Default these fields to 0 here so the existing non-nullable client DTO remains parseable while preserving the null Reservations shape.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@OrchestratorIDE.Daemon/HiveService.cs` around lines 232 - 253, Update the
NativeTelemetryProvider reservation projection in HiveService to default
TotalBytes, ReservedBytes, and AvailableBytes to 0 when the reservation snapshot
is null, while leaving Reservations nullable. Preserve the existing telemetry
structure and other field mappings.
| // not data loss, but avoidable: don't run these CLI modes against a machine that already | ||
| // has a live GUI-owned HIVE identity. Headless-only boxes (this Pi) have no such collision. | ||
| if (args.Contains("--show-identity") || args.Contains("--pair")) | ||
| if (args.Contains("--show-identity") || args.Contains("--pair") || args.Contains("--leave-hive")) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Fail closed when loading an identity for --leave-hive.
HiveIdentity.Load() silently regenerates and persists a new identity when decryption or deserialization fails. With this new --leave-hive path using AesGcmSecretProtector, a GUI-owned DPAPI identity can therefore be overwritten and reported as “not currently in a hive” instead of failing safely. Use a strict load path for this command and return a non-zero error on protector/load failure.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@OrchestratorIDE.Daemon/Program.cs` at line 40, Update the --leave-hive
handling in the argument dispatch to use a strict HiveIdentity load path that
does not regenerate or persist an identity when decryption or deserialization
fails. Catch protector/load failures, report the error, and exit with a non-zero
status; preserve the existing behavior for --show-identity and --pair.
| if (args.Contains("--leave-hive")) | ||
| { | ||
| var identity = HiveIdentity.Load(); | ||
| if (string.IsNullOrEmpty(identity.HiveId)) | ||
| { | ||
| Console.WriteLine("This node is not currently in a hive — nothing to leave."); | ||
| return 0; | ||
| } | ||
|
|
||
| // Deliberately requires an explicit confirmation flag rather than acting on --leave-hive | ||
| // alone. The spec frames leaving as a decision a human makes; on a headless box there is no | ||
| // dialog to serve that role, so the second flag is what makes the intent unambiguous. | ||
| if (!args.Contains("--yes")) | ||
| { | ||
| Console.Error.WriteLine( | ||
| $"--leave-hive would abandon hive {identity.HiveId} (role {identity.HiveRole}) on " + | ||
| $"{Environment.MachineName}, resetting membership so this node can pair into a " + | ||
| "different hive. NodeId, keys and existing peer secrets are kept."); | ||
| Console.Error.WriteLine("Re-run with --leave-hive --yes to confirm."); | ||
| return 1; | ||
| } | ||
|
|
||
| var leftHiveId = identity.HiveId; | ||
| identity.LeaveHive(); | ||
| Console.WriteLine($"Left hive {leftHiveId}. NodeId {identity.NodeId} unchanged."); | ||
| Console.WriteLine("Pair again with: --pair --target <host> --expect-fingerprint \"<phrase>\""); | ||
| return 0; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Reject mixed modes and unknown arguments before leaving the hive.
This branch checks only Contains("--leave-hive") and Contains("--yes"), then returns before the unknown-argument validation. For example, --leave-hive --yes --pair ... or a typo appended to the command still performs the destructive operation and ignores the extra arguments. Validate that --leave-hive is the sole mode and that only its allowed flags are present before calling LeaveHive().
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@OrchestratorIDE.Daemon/Program.cs` around lines 66 - 93, Update the
--leave-hive branch to validate arguments before any confirmation or mutation:
require it to be the sole operational mode, allow only its explicitly supported
flags (including --yes), and reject unknown or mixed-mode arguments with the
existing validation behavior. Ensure invalid invocations return before identity
loading or LeaveHive(), while valid --leave-hive [--yes] flows retain their
current confirmation and success behavior.
| var leftHiveId = identity.HiveId; | ||
| identity.LeaveHive(); | ||
| Console.WriteLine($"Left hive {leftHiveId}. NodeId {identity.NodeId} unchanged."); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n -C 4 'HiveIdentity\.Load|LeaveHive|HiveId' \
OrchestratorIDE.Daemon OrchestratorIDERepository: hardcoreerik/TheOrc
Length of output: 50375
🏁 Script executed:
#!/bin/bash
# Locate and inspect the one-shot command and daemon startup/HiveService lifecycle.
set -euo pipefail
printf '--- Program.cs around leave command ---\n'
if [ -f OrchestratorIDE.Daemon/Program.cs ]; then
nl -ba OrchestratorIDE.Daemon/Program.cs | sed -n '1,140p'
fi
printf '\n--- Find HiveService/startup references ---\n'
rg -n -C 3 'class HiveService|HiveService|StartAsync|StopAsync|BackgroundService|IHostApplicationLifetime|LeaveHive|HiveIdentity\.Load|HiveIdentity\.CreateEphemeral' \
OrchestratorIDE.Daemon/Program.cs OrchestratorIDE.Daemon OrchestratorIDE/Services 2>/dev/null || true
printf '\n--- Show candidate outlines for service/program files ---\n'
for f in $(git ls-files 'OrchestratorIDE.Daemon/Program.cs' 'OrchestratorIDE.Daemon/**/*.cs' 'OrchestratorIDE/Services/**/*.cs' 2>/dev/null); do
if rg -q 'HiveService|HiveBeacon|HiveMeshHeartbeat|HiveNodeServer|LeaveHive' "$f"; then
echo "### $f"
ast-grep outline "$f" --view compact 2>/dev/null | head -80 || true
fi
doneRepository: hardcoreerik/TheOrc
Length of output: 248
🏁 Script executed:
#!/bin/bash
# Locate and inspect the one-shot command and daemon startup/HiveService lifecycle.
set -euo pipefail
printf '--- Program.cs around leave command ---\n'
if [ -f OrchestratorIDE.Daemon/Program.cs ]; then
nl -ba OrchestratorIDE.Daemon/Program.cs | sed -n '1,140p'
fi
printf '\n--- Find HiveService/startup references ---\n'
rg -n -C 3 'class HiveService|HiveService|StartAsync|StopAsync|BackgroundService|IHostApplicationLifetime|LeaveHive|HiveIdentity\.Load|HiveIdentity\.CreateEphemeral' \
OrchestratorIDE.Daemon/Program.cs OrchestratorIDE.Daemon OrchestratorIDE/Services 2>/dev/null || true
printf '\n--- Show candidate outlines for service/program files ---\n'
for f in $(git ls-files 'OrchestratorIDE.Daemon/Program.cs' 'OrchestratorIDE/Services/Hive/HiveBeacon.cs' 'OrchestratorIDE/Services/Hive/HiveNodeServer.cs' 2>/dev/null); do
echo "### $f"
ast-grep outline "$f" --view compact 2>/dev/null | head -120 || true
doneRepository: hardcoreerik/TheOrc
Length of output: 248
🏁 Script executed:
#!/bin/bash
# Locate and inspect the one-shot command and daemon startup/HiveService lifecycle without dash-starting print format strings.
set -euo pipefail
echo '--- Program.cs around leave command ---'
if [ -f OrchestratorIDE.Daemon/Program.cs ]; then
nl -ba OrchestratorIDE.Daemon/Program.cs | sed -n '1,160p'
fi
echo
echo '--- Find HiveService/startup references ---'
rg -n -C 3 'class HiveService|HiveService|StartAsync|StopAsync|BackgroundService|IHostApplicationLifetime|LeaveHive|HiveIdentity\.Load|HiveIdentity\.CreateEphemeral' \
OrchestratorIDE.Daemon/Program.cs OrchestratorIDE.Daemon OrchestratorIDE/Services 2>/dev/null || true
echo
echo '--- Show candidate file summaries ---'
for f in OrchestratorIDE.Daemon/Program.cs OrchestratorIDE/Services/Hive/HiveBeacon.cs OrchestratorIDE/Services/Hive/HiveNodeServer.cs; do
if [ -f "$f" ]; then
echo "### $f"
nl -ba "$f" | sed -n '1,220p'
fi
doneRepository: hardcoreerik/TheOrc
Length of output: 237
🏁 Script executed:
#!/bin/bash
# Locate and inspect the one-shot command and daemon startup/HiveService lifecycle using portable tools.
set -euo pipefail
echo '--- Program.cs around leave command ---'
if [ -f OrchestratorIDE.Daemon/Program.cs ]; then
awk '{printf "%6d\t%s\n", NR, $0}' OrchestratorIDE.Daemon/Program.cs | sed -n '1,160p'
fi
echo
echo '--- Find HiveService/startup references ---'
rg -n -C 3 'class HiveService|HiveService|StartAsync|StopAsync|BackgroundService|IHostApplicationLifetime|LeaveHive|HiveIdentity\.Load|HiveIdentity\.CreateEphemeral' \
OrchestratorIDE.Daemon/Program.cs OrchestratorIDE.Daemon OrchestratorIDE/Services 2>/dev/null || true
echo
echo '--- Show candidate file summaries ---'
for f in OrchestratorIDE.Daemon/Program.cs OrchestratorIDE/Services/Hive/HiveBeacon.cs OrchestratorIDE/Services/Hive/HiveNodeServer.cs; do
if [ -f "$f" ]; then
echo "### $f"
awk '{printf "%6d\t%s\n", NR, $0}' "$f" | sed -n '1,220p'
fi
doneRepository: hardcoreerik/TheOrc
Length of output: 50375
🏁 Script executed:
#!/bin/bash
# Inspect HiveService lifecycle and the singleton lock implementation without large output.
set -euo pipefail
echo '--- HiveService lifecycle around service startup/stop ---'
awk '{printf "%6d\t%s\n", NR, $0}' OrchestratorIDE.Daemon/HiveService.cs | sed -n '100,220p'
awk '{printf "%6d\t%s\n", NR, $0}' OrchestratorIDE.Daemon/HiveService.cs | sed -n '280,320p'
echo
echo '--- HiveIdentity locks/load implementation ---'
awk '{printf "%6d\t%s\n", NR, $0}' OrchestratorIDE/Services/Hive/HiveIdentity.cs | sed -n '90,280p'
echo
echo '--- Daemon args handling sequence ---'
awk '{printf "%6d\t%s\n", NR, $0}' OrchestratorIDE.Daemon/Program.cs | sed -n '1,210p'Repository: hardcoreerik/TheOrc
Length of output: 32833
Coordinate with the running daemon before persisting hive membership removal.
--leave-hive resets persistence and the current process’s HiveIdentity instance, but HiveService is started by the same binary, and the daemon can cache that loaded identity for its node server/heartbeat lifecycle. If this runs while the long-running daemon is active, beacons/API pairing checks can still use the old HiveId. Require the daemon service to be stopped, or add an explicit IPC/reload path with cross-process coordination.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@OrchestratorIDE.Daemon/Program.cs` around lines 88 - 90, Update the
--leave-hive flow around HiveIdentity.LeaveHive and HiveService coordination so
hive membership removal cannot occur while the daemon is using a cached
identity. Require the daemon service to be stopped before persisting the
removal, or implement an explicit coordinated IPC/reload path that refreshes the
daemon’s identity before completion.
| private static void EvaluateSequential( | ||
| string workerId, List<Hv3TelemetrySample> samples, int cycles, Hv3Report report) | ||
| { | ||
| var afterCycles = samples.Where(s => s.Stage.StartsWith("after-cycle", StringComparison.Ordinal)).ToList(); | ||
|
|
||
| // 1. Residency returns to baseline between jobs. Sampled AFTER each job completes, so a | ||
| // nonzero ActiveCount here means a conversation outlived the job that created it. | ||
| // | ||
| // Requires evidence that work actually HAPPENED before it can pass. "ActiveCount is 0" | ||
| // is trivially true on a worker that never ran anything, and this check reported PASS | ||
| // on both machines during a run where every job sat unclaimed and the telemetry | ||
| // endpoint was unreachable -- a green tick on an empty run is worse than no check, | ||
| // because it reads as evidence. ConversationsCreated > 0 is the liveness proof. | ||
| var workDone = afterCycles.Any(s => s.Reachable && s.MaxConversationsCreated > 0); | ||
| var leaked = afterCycles.Where(s => s.TotalActiveCount > 0).Select(s => s.Stage).ToList(); | ||
| report.LifecycleChecks.Add(new Hv3LifecycleCheck | ||
| { | ||
| WorkerId = workerId, | ||
| Name = "residency-returns-to-baseline", | ||
| Passed = afterCycles.Count > 0 && workDone && leaked.Count == 0, | ||
| Detail = afterCycles.Count == 0 | ||
| ? "no post-cycle samples captured" | ||
| : !workDone | ||
| ? "VACUOUS — no conversation was ever created on this worker (unreachable " + | ||
| "telemetry or unclaimed jobs); residency being zero proves nothing here" | ||
| : leaked.Count == 0 | ||
| ? $"ActiveCount back to 0 after all {afterCycles.Count} cycle(s)" | ||
| : $"ActiveCount still nonzero after: {string.Join(", ", leaked)}", | ||
| }); | ||
|
|
||
| // 2. Reservation persists across the gap. The model stays loaded between jobs, so the | ||
| // reservation must NOT drop back to the pre-run baseline the way residency does -- | ||
| // this is the half of the decoupling that would be invisible if we only checked | ||
| // that things return to zero. | ||
| var beforeAll = samples.FirstOrDefault(s => s.Stage == "before-all"); | ||
| var reservedAfter = afterCycles.Select(s => s.ReservedBytes).ToList(); | ||
|
|
||
| // "Persists" means the reservation never DROPS across the gaps between jobs -- it must | ||
| // not be tied to `> baseline`. That stricter form only holds when the run starts against | ||
| // a cold worker; against a worker still warm from a previous run the baseline sample | ||
| // ALREADY includes the resident model, so the correct observation is equality and the | ||
| // strict comparison reports a false failure. Seen exactly that way on the first | ||
| // two-machine run: HardcorePC (warm, baseline 5589043712) failed while | ||
| // HardcoreLaptopMSI (freshly started, baseline 45088768) passed on identical behavior. | ||
| // | ||
| // Non-decreasing across cycles plus at-least-baseline is what the decoupling actually | ||
| // claims, and it holds from either starting state. | ||
| var reservationHeld = beforeAll is not null | ||
| && reservedAfter.Count > 0 | ||
| && reservedAfter.All(r => r >= beforeAll.ReservedBytes) | ||
| && reservedAfter.Zip(reservedAfter.Skip(1), (a, b) => b >= a).All(x => x); | ||
|
|
||
| // A worker that never loaded anything would also satisfy "non-decreasing" trivially, so | ||
| // require the reservation to actually reflect a loaded model rather than an idle card. | ||
| var residentFootprintSeen = afterCycles.Any(s => s.MaxConversationsCreated > 0); | ||
|
|
||
| report.LifecycleChecks.Add(new Hv3LifecycleCheck | ||
| { | ||
| WorkerId = workerId, | ||
| Name = "reservation-persists-between-jobs", | ||
| Passed = reservationHeld && residentFootprintSeen, | ||
| Detail = beforeAll is null | ||
| ? "no baseline sample captured" | ||
| : $"baseline reservedBytes={beforeAll.ReservedBytes} " + | ||
| $"({(beforeAll.MaxConversationsCreated > 0 ? "warm worker" : "cold worker")}), " + | ||
| $"after-cycle values=[{string.Join(", ", reservedAfter)}]", | ||
| }); | ||
|
|
||
| // 3. A fresh conversation per job. Without this, "residency returned to zero" could also | ||
| // be explained by a single conversation being reused and never counted again. | ||
| var created = afterCycles.Select(s => s.MaxConversationsCreated).ToList(); | ||
| var strictlyIncreasing = created.Count == cycles | ||
| && created.Zip(created.Skip(1), (a, b) => b > a).All(x => x); | ||
| report.LifecycleChecks.Add(new Hv3LifecycleCheck | ||
| { | ||
| WorkerId = workerId, | ||
| Name = "fresh-conversation-per-job", | ||
| Passed = created.Count > 0 && (cycles == 1 ? created[0] > 0 : strictlyIncreasing), | ||
| Detail = $"ConversationsCreated across cycles=[{string.Join(", ", created)}]", | ||
| }); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Sequential/concurrent lifecycle checks aggregate residency/reservation data across ALL roles instead of the role under test.
Hv3TelemetrySample.MaxConversationsCreated (line 444) takes the Max across every residency entry, not just the role being tested (role in RunSequentialAsync). Since HiveService.cs always pre-binds a second role (Researcher) into residency, any prior run on the same long-lived daemon that already bumped that other role's ConversationsCreated will pin Max to a stale constant — breaking fresh-conversation-per-job's strict-increase check with a false FAIL, or satisfying the workDone/residentFootprintSeen liveness gates without the tested role having done anything (the exact vacuous-pass shape this PR explicitly guards against elsewhere).
The same root cause shows up in RunConcurrentAsync's peakReserved/peakReservedRoles (lines 314-338): it counts every currently-reserved role fleet-wide, not just primaryRole/secondaryRole. A stale reservation from an earlier run could make peakReserved >= 2 trivially true even when this run's secondary role was actually denied — a false PASS on precisely the cross-role admission defect HV-3 exists to catch.
The raw per-sample Residency list (role names, already captured) makes the sequential-side fix straightforward:
🛠️ Proposed fix (sequential phase)
- private static void EvaluateSequential(
- string workerId, List<Hv3TelemetrySample> samples, int cycles, Hv3Report report)
+ private static void EvaluateSequential(
+ string workerId, List<Hv3TelemetrySample> samples, int cycles, string role, Hv3Report report)
{
var afterCycles = samples.Where(s => s.Stage.StartsWith("after-cycle", StringComparison.Ordinal)).ToList();
+ int RoleConversations(Hv3TelemetrySample s) =>
+ s.Residency.FirstOrDefault(r => string.Equals(r.Role, role, StringComparison.OrdinalIgnoreCase))
+ ?.ConversationsCreated ?? 0;
@@
- var workDone = afterCycles.Any(s => s.Reachable && s.MaxConversationsCreated > 0);
+ var workDone = afterCycles.Any(s => s.Reachable && RoleConversations(s) > 0);
@@
- var residentFootprintSeen = afterCycles.Any(s => s.MaxConversationsCreated > 0);
+ var residentFootprintSeen = afterCycles.Any(RoleConversations(s) => s > 0);
@@
- var created = afterCycles.Select(s => s.MaxConversationsCreated).ToList();
+ var created = afterCycles.Select(RoleConversations).ToList();And update the call site: EvaluateSequential(w.Id, samples, cycles, role, report);
For the concurrent phase, ReservationEntry.Role is a raw numeric RuntimeRole value while primaryRole/secondaryRole are name strings, so a direct filter isn't possible without a name↔int mapping — consider projecting reservation roles as display-name strings server-side (mirroring what Residency.Role already does) so both sides can filter consistently.
Also applies to: 314-338, 350-359, 443-447
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Tools/Hv3LifecycleRunner/Program.cs` around lines 179 - 259, Scope lifecycle
telemetry to the roles under test instead of aggregating all roles. Update
EvaluateSequential and its RunSequentialAsync call site to accept the tested
role and derive MaxConversationsCreated and related liveness values from
matching Residency entries. In RunConcurrentAsync, project ReservationEntry.Role
to display-name strings server-side, then filter peakReserved and
peakReservedRoles to primaryRole and secondaryRole before evaluating admission.
Preserve the existing lifecycle checks while ensuring stale reservations or
conversations from other roles cannot affect their results.
HiveAuthMiddleware.ValidateCore already distinguishes "unknown or revoked node",
"no shared secret for peer", "clock skew too large (Ns)", "HMAC mismatch" and
"nonce already seen (replay)", and HiveTaskQueue returns that verdict to the
caller as {"error": "<reason>"}. The worker read the status code and threw the
body away, so an operator facing a 401 saw only "HTTP 401" and could not tell a
stale secret from clock skew from a replay.
That gap cost two full sessions of black-box guessing during the HV-3 campaign:
peer stores were cleared, hive membership was reset, and pairing was redone
several times while the responder had been naming the actual reason in every
response the whole time.
Both rejection sites now append it. The reader is deliberately defensive -- this
is an untrusted remote body on a diagnostic path, so it truncates at 400 chars,
falls back to the raw text when the body is not the expected JSON shape, and
swallows its own failures rather than turning a handled HTTP error into a crash.
Directly serves §6's "consistent telemetry and diagnosability across machines"
criterion, which HV-5 has to demonstrate: a failure whose cause is invisible on
both sides is not diagnosable regardless of how much telemetry surrounds it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… misdiagnoses Writes down what a dead Warchief looks like from every other vantage point (self-terminating --timeout, death on the launching console's Ctrl+C, fully buffered logs), because each was mistaken for a HIVE fault first. Then records the real blocker now that rejection reasons are visible: both sides finish a mutually-successful pairing ceremony holding different shared-secret bytes, which is a crypto/pairing defect and needs a unit test over the two derivation call sites rather than more live poking. Also records that --leave-hive as shipped leaves a headless worker in a NEW hive on next daemon start, which is worse than the state it was meant to repair. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The HV-3 campaign stalled with every worker request returning "HTTP 401 - HMAC mismatch", which can only mean the two sides hold different shared-secret bytes. The derivation call sites are asymmetric in shape, so that was the first thing worth eliminating rather than assuming ECDH works both ways: responder HiveNodeServer.ApprovePairing: XorNodeIds(self, initiator) initiator HivePairingClient.CompletePairing: XorNodeIds(self, responder) The node ids go in in OPPOSITE order, so the salt is equal only if XorNodeIds is genuinely commutative -- including on its padding/truncation branch for ids that are not exactly 64 hex chars, which is where an asymmetry would have hidden while both machines still reported pairing success. It is symmetric, and the derivation is correct: 4/4 pass. A negative case is included so the equality assertion cannot pass vacuously by returning a constant. This narrows the open defect rather than closing it. Now eliminated by evidence: derivation and salting (here), peer enrolment (both stores hold the other side, role Worker, not revoked, secrets written by the SAME ceremony a second apart), identity confusion (both sides' stored nodeId AND fingerprint match the other's live identity, and NewcorePC's hive-identity.json is untouched since 07-22), clock skew (within 2s), and MachineKey.Load() non-determinism (env -> file -> generate-once). Uses CreateEphemeral so it exercises the real key types without touching hive-identity.json. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…t shapes Second half of the "HMAC mismatch" elimination. HivePairingSecretDerivationTests already showed both sides of a pairing derive identical secrets; this covers the other candidate, that signer and verifier disagree on the canonical string. They build it from different sources, which is why it was worth pinning rather than assuming: the signer uses HttpRequestMessage.RequestUri.AbsolutePath plus the body it was handed, the verifier uses HttpListenerRequest.Url.AbsolutePath plus the bytes read off the wire. Covers both fleet calls that failed -- GET /hive/models with an empty body, and the POST heartbeat with a JSON body -- plus a URL carrying a query string, since AbsolutePath drops the query on both sides and an asymmetry there would surface as an unexplained "HMAC mismatch" the moment any endpoint grew a parameter. All four pass, so the signing path is correct. The negative control asserts the exact reason string "HMAC mismatch" for a mismatched secret, which both proves the positive cases are not passing through lenient validation and pins the string the campaign log's diagnosis refers to. Secrets go in through the store's own encrypt path rather than being assigned to SharedSecretEnc directly, so the protector round-trip -- itself a suspect -- is exercised too. Needs SecretProtection.Initialize, which production does at startup and a test host never does; done in OneTimeSetUp. Net effect on the open blocker: derivation and signing are both eliminated, so the two machines genuinely hold different secret BYTES despite a ceremony both reported as successful. Next step is comparing a hash of each side's decrypted secret (never the secret itself) to confirm divergence, then auditing what rewrites a peer's secret after a successful pairing. Full Hive suite 120/120. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ronment artifacts Three of the things this campaign treated as HIVE defects were artifacts of the debugging environment, and recording that is worth more than another round of guessing: - NewcorePC has TWO AppData views (%APPDATA%\TheOrc and the packaged app's LocalCache\Roaming\TheOrc), each with its own hive-peers.json, hive-identity.json and machine.key. Tooling launched different ways writes different files, so every 'cleared the peer store, still already_paired' observation here is suspect - including, very plausibly, the long-standing note in HiveNodeServer that the on-disk file could not explain a stuck already_paired. - A running Warchief rewrites the peer store wholesale from memory, so it can only be edited while stopped. - The apparent attached-vs-detached Warchief split was a port conflict: an older process still held 7078/7079 and answered, while the new one failed to bind. What stays: the two test suites (8 tests) proving derivation, salting and the sign->validate round-trip are correct, the worker now logging WHY it was rejected, and the fact that auth is validated before endpoint routing. The persist/reload suspicion is explicitly marked unproven - it was observed while the two-AppData confound was active and has not been re-checked under controlled conditions. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The controlled re-test (both AppData views cleared to byte-identical state, worker peer store deleted, exactly one swarmcli bound to 7078) gave opposite results from the same on-disk trust state depending only on how the Warchief was started: WMI/Win32_Process.Create -> already_paired against a node absent from every store found on disk; direct child process -> clean pairing with the fingerprint verified and the secret stored. So the HMAC mismatch, the 'cleared the store and it still says already_paired', and the phantom in-memory peer were all one thing: a WMI-spawned Warchief resolving its HIVE state somewhere other than the file the rest of the tooling reads. No system-profile copy exists, so the exact redirection is unidentified, but the operational rule is clear and is now written down. With that fixed, HV-3 sequential PASSES on HardcorePC from a COLD worker -- the stronger form, where the reservation is seen appearing on first load and then holding across every gap. 3/3 native, zero fallback. This doubles as the regression check for the cross-role admission fix, which altered the admission path and had never been re-verified against the previously-passing phase. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
ExcludedWorkerIds is the only pinning mechanism these phases have, and it is built by excluding the OTHER configured workers -- so a run configured with a single --worker-a excludes nobody and any live worker in the fleet can claim its jobs. A phase aimed at HardcoreLaptopMSI had its second work unit claimed by HardcorePC, whose 6 GB card produced the very timeout the run was trying to attribute to the laptop. The evidence file still recorded the intended worker, so the result read as a finding rather than a mis-targeted job, and it briefly supported a wrong conclusion about VRAM headroom. Two changes: ClaimedByExpected is now part of the verdict instead of a field that was captured and then ignored. A phase that cannot prove WHICH machine ran the work proves nothing about "across machines", which is the entire §6 criterion this campaign exists to evidence -- a job completed natively on the wrong box must not read as a pass. A single-worker run now warns that nothing pins its jobs and tells the operator to either stop the other workers or pass them via --worker-b/-c so they are excluded explicitly. Warn rather than fail: single-worker runs are legitimate once the other workers really are stopped. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
HiveTaskQueue re-queues a claimed task after 45s of silence. A legitimately long job survives only if the beat loop keeps landing — HeadlessAgentLoop allows MaxSteps 12 at 4096 tokens each, which on a 7B is minutes of continuous native inference. During HV-3's concurrent phase the Warchief declared "exhausted 3 attempts after heartbeat loss" against a worker that was demonstrably still working (its residency showed conversationsCreated climbing to exactly 2 attempts x 12 steps). The loop made that impossible to diagnose: it never inspected the response and swallowed every exception, so a STARVED loop, a REJECTED beat and a healthy one all produced identical output — nothing. Three signals now, all non-fatal because a missed beat must never take down a running job: - Non-success responses are logged with the responder's own reason, so a stale claim token or a signature problem is named rather than inferred. - Send exceptions are logged with type and message. - An oversized GAP between beats is logged even when the send SUCCEEDS. That is the only signal separating starvation from rejection: a starved loop's sends still succeed, just too late to matter. Threshold 30s — above the 10s cadence so ordinary jitter stays quiet, below the queue's 45s cutoff so it fires BEFORE the re-queue rather than explaining it afterwards. Also stops treating shutdown cancellation as a failure to report. Deliberately diagnostic-only: no change to cadence, timeout, or threading. Which of those is the right fix depends on whether the next run shows a stall gap or a rejection, and that is now answerable from one log. Same approach that turned the opaque 401 into "HMAC mismatch" in one run. Hive suite 120/120. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Root cause of HV-3's concurrent-phase failure, named by the diagnostics added in the previous commit on their first run: Heartbeat for 'HV-3 concurrent Researcher on HardcoreLaptopMSI' failed to send: TaskCanceledException: The request was canceled due to the configured HttpClient.Timeout of 5 seconds elapsing. Not a rejection and not listener starvation — the worker's own inbound telemetry endpoint answered in ~0.09s while these beats were timing out. A worker mid-inference is CPU-saturated (HeadlessAgentLoop runs up to 12 steps of 4096 tokens back-to-back), which delays the OUTBOUND request's async continuations past 5s. Every beat died the same way, three in a row reached HiveTaskQueue's 45s cutoff, and a worker that was demonstrably still working was declared dead and its task re-queued until it exhausted all attempts. Two changes: Heartbeat HTTP timeout 5s -> 20s. Generous enough to ride out that contention, still under the 45s cutoff so a genuinely unreachable Warchief is detected well within the window rather than masked. Deliberately bounded — a hung send must not stall the loop past the point where the re-queue it exists to prevent has already happened. Retry after 3s instead of the full 10s cadence once a beat has failed. At 10s a worker only gets four attempts inside the 45s window, so two slow beats are already most of the budget and backing off the full interval spends what is left. Resets to the normal cadence on the first success. This is an HV-4 finding as much as HV-3: a healthy worker being declared dead is exactly the false positive that phase exists to catch, and it would misfire on any legitimately long job, not just this role. Hive suite 120/120. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The 5s -> 20s timeout change helped measurably but could not fix this. Measured against a recorded baseline of 4 "heartbeat timeout" lines, a full uninterrupted post-fix run still produced a new one, and the Researcher task still ended "failed" after heartbeat loss. That ruled the remaining cause in. A longer send timeout only matters once the send has STARTED; the beat was late to be ATTEMPTED. HeartbeatLoopAsync used Task.Delay plus async continuations on the managed thread pool, and this loop's entire job is to prove liveness while the worker is CPU-saturated by construction — HeadlessAgentLoop runs up to 12 steps of 4096 tokens back-to-back. Both the interval and the send queued behind that work. The worker's own inbound HttpListener stayed responsive throughout (~0.09s) precisely because it does not share the pool, which is the same reason this belongs on its own thread. Also eliminated, by reading rather than guessing: queue-side bookkeeping. HandleHeartbeatAsync rejects a stale claim token with HTTP 409, not a silent 200, and the worker now logs non-success responses — so a rotated-token beat would appear as "Heartbeat rejected: HTTP 409" rather than as silence. Now a dedicated IsBackground thread using Thread.Sleep (sliced at 250ms so shutdown cancellation is still observed promptly) and a blocking HttpClient.Send, so neither the interval nor the response can be stranded behind inference work. The method keeps returning a Task that completes when the thread exits, so the existing await in ClaimAndExecuteAsync is unchanged. Verify the same way this was measured: record the Warchief's "heartbeat timeout" line count, run --phase concurrent to a verdict, and confirm the count is unchanged. Hive suite 120/120. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…eipt logging Two changes to the responder side of the open heartbeat-timeout investigation. HandleHeartbeatAsync's not-claimed branch returned HTTP 200 while deliberately NOT refreshing LastHeartbeat, which made an uncredited beat indistinguishable from a credited one at the worker. Once the watchdog re-queued a task (Status -> "pending", claim token rotated, LastHeartbeat cleared) the still-running worker kept beating into that branch, saw success, logged nothing, and carried on executing a lease it no longer held — while the task was re-claimed and executed AGAIN. That is why the Researcher role's ConversationsCreated climbed in multiples of its 12-step budget across runs whose worker log contained zero heartbeat complaints. It now answers 409, the same visible shape the stale-token rejection already used, so the worker logs it. That closes the silent cascade but does NOT explain the FIRST timeout, and the evidence needed for that is missing on this side: the worker reports neither send failures nor rejections across runs the Warchief nonetheless timed out, so "never arrived" and "arrived but uncredited" are currently the same observation. Added an opt-in receipt log (THEORC_HIVE_HEARTBEAT_DIAGNOSTICS=1) recording every beat as credited / stale-token / not-claimed, with the gap since the previous credited beat. Zero-cost when unset — one cached bool read per beat, same idiom as AdapterManager's KV-cache diagnostics. Deliberately diagnostic-first again: the previous two attempts at this bug were fixes aimed at a hypothesis (send timeout, then thread-pool starvation) and the second one measured as having no effect. Making the responder say what it actually did is what turned the 401 into "HMAC mismatch" in one run. Hive suite 120/120. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Three fleet-driven attempts at HV-3's heartbeat-timeout failure landed with only one confirmed effect, each costing a worker restart, a cold 4.7 GB model load and several minutes — and the run meant to validate the receipt logging was void because the probe job finished in 9.6s while the first beat is not due until claim+10s. The underlying question was answerable in milliseconds all along. Extracts HeartbeatCoreAsync from the HTTP handler — the same seam HiveAuthMiddleware.ValidateCore uses, and for the same reason: HttpListenerContext is sealed and cannot be constructed in a test. Adds minimal internal seams to seed a claimed entry and read LastHeartbeat, in the spirit of HivePeerStore.CreateForTest. Result: the queue's bookkeeping is CORRECT. A beat for a live claim advances LastHeartbeat, and keeps advancing across repeated beats — which no single-beat assertion would have covered, and which is the property a job outliving the 45s window actually depends on. Re-queued and rotated-token beats are correctly not credited and are reported as distinct outcomes. That eliminates bookkeeping and leaves DELIVERY: beats are not reaching the queue during long jobs. Note the worker logs nothing on a successful beat, so its silence has never been positive evidence that it sent — the mirror of this change is needed there, a positive signal that a beat was actually dispatched, before any further fix is attempted. 5 new tests, ~150ms total. Hive suite green. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Mirror of the queue-side work. Those tests proved bookkeeping correct, leaving delivery as the open half — but the worker's silence could never settle it, because a successful beat logged nothing. Every "the worker log is clean, so beats are being sent" reading during this investigation was equally consistent with never sending one, and that ambiguity is what kept three hypothesis-driven fixes from being falsifiable. The first successful beat for a task now logs once, with how long after the claim it landed. One line per task rather than per beat: enough to separate "never sent" from "sent, then stopped", without a line every 10s per running job. A 409 also now ends the loop. It means this worker no longer holds the lease — the task was re-queued (not-claimed) or re-claimed by another worker (stale token) — so further beats can never be credited. Stopping turns an endless rejection stream into one clear "lost the lease" line, and prevents a zombie from appearing to keep a re-assigned task alive. The job itself is deliberately NOT cancelled: abandoning in-flight work is a separate policy call, and the eventual result post is rejected on its own token check regardless. Hive suite 125/125. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The heartbeat was never broken. `swarmcli --warchief` never wired an
ArtifactStore, so every PUT /hive/artifacts/{digest} answered 503 from
HiveTaskQueue's "store is null" branch. The worker's upload threw before it
could post its result, the task stayed "claimed" with its heartbeat loop
already cancelled, and 45s later the watchdog re-queued it as a heartbeat
timeout -- three times, then "exhausted 3 attempts after heartbeat loss".
b90fd13's positive confirmation is what settled it. The worker log now reads:
Heartbeat established for 'HV-3 concurrent Researcher on HardcorePC' (10s after claim).
[Researcher] '...' - native agent completed in 2 steps (runtime=NativeRoleRuntime, ...)
Worker loop error: Response status code does not indicate success: 503 (Service Unavailable).
and the Warchief side, with THEORC_HIVE_HEARTBEAT_DIAGNOSTICS=1, logged beats
arriving and being credited at sinceLast=10.2s right up to the job finishing.
Job done, beats landing, result never delivered. Reproduced identically on both
HardcorePC and HardcoreLaptopMSI.
That gap was already known from the other side -- CF-6's acceptance runner needed
a Daemon-hosted Warchief for exactly this reason (2026-07-21) -- but it was
recorded as a property of that runner rather than as a defect, so a campaign whose
jobs emit output files silently required the Daemon. Wired here to mirror
HiveService.cs, ModelStore included: without it GET /hive/models 503s too, which is
the "Approved-model catalog rejected by Warchief: HTTP 503" every worker logs on a
one-minute cycle.
The failure MODE is fixed too. An upload failure must not be able to impersonate a
dead worker: the uploads sat after the try/catch around execution, so the throw
escaped past PostResultAsync into RunLoopAsync's generic handler. Now it fails
closed and visibly -- the result is still posted, marked failed, carrying the real
upload error. That is also HV-4's "job fails visibly" requirement.
Two supporting bits: the queue states at startup whether heartbeat receipt
diagnostics are on, so an empty receipt log can never again be read as "no beat
arrived"; and start-warchief.bat's nested `start ... cmd /c` form is replaced by a
re-entrant one, since its quoting put the launched console in the wrong directory
and wrote `'swarmcli.exe' is not recognized` into warchief.log.
Hive suite 125/125.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ured the wrong quantity Both phases now rest on one build's evidence: hv3_concurrent_20260727_165522.json PASS (both machines, 4/4 jobs native) hv3_sequential_20260727_165823.json PASS (both machines, 6/6 jobs native) The failing concurrent run on the same build minus the artifact-store fix is kept alongside as the paired negative control (…_165114.json): identical lifecycle checks PASS, both Researcher jobs failed. (.orc is gitignored, so these live on the Warchief box, as with every prior HV run.) reservation-persists-between-jobs produced its SECOND false failure here, on a ~27 MB drift, while every role held its reservation correctly. reservedBytes is not the ledger -- GetReservationSnapshot publishes the MAX of the ledger and a live whole-GPU probe, and on a full card the probe wins, so asserting any monotonic property of it asserts that nothing else on the machine may allocate a byte of VRAM. The first false failure was the `> baseline` form, which only held from a cold worker. Both times the check had drifted onto a convenient aggregate instead of the quantity under test. Restated on the role's reservation entry, which is what the decoupling claims: every post-job sample must still show the role holding a reservation, and no role may lose one it held after the previous job. Immune to warm-vs-cold starting state and to whole-GPU drift. Bytes are still recorded as evidence, not asserted -- note a role's reservation legitimately shrinks from a full-model charge to an incremental context charge once another role has the base model resident (5589043712 -> 637534208 while the footprint stayed at 6.2 GB). HV-3 items 1 and 2 are closed across machines. Item 3 (forced role recycle via MarkRoleDegraded) stays out of scope -- still no remote trigger, and adding one is a mutation needing an authenticated control endpoint and its own security review. HV-3 is therefore NOT fully closed and must not be reported as such. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Same submit -> poll -> evidence-JSON shape as the HV-1/HV-2/HV-3 runners, so HV-6
can drive all of them identically.
HV-4 (Tools/Hv4RecoveryRunner): kill / disconnect / ollama / cancel, each on real
jobs mid-flight. Every phase asserts the two things §6 actually rides on beyond its
own subject -- the failure is VISIBLE (terminal status the Warchief reports, not a
hang) and there is NO Ollama substitution. Recovery is proven by USING the worker
again, not by seeing its process exist: a live process says nothing about whether
the role still holds a working executor, which is exactly what a mid-job death puts
in doubt.
Two scope limits are recorded in every evidence file rather than left implicit:
* The plan's item 1 wants cancellation to surface mid-GENERATION on the worker.
There is no remote trigger -- the worker's only inbound listener is
HiveNodeServer (pair/info/native-telemetry/mesh/update) and it has no
task-cancel endpoint, so a campaign cancel is visible on the Warchief while the
worker generates to completion. The cancel phase proves the Warchief-side half
and role reusability, and says so. Same call already made for HV-3's
MarkRoleDegraded item: adding the endpoint is a mutation needing auth and its
own security review.
* --target narrows which workers are EXERCISED without narrowing which are
EXCLUDED. Dropping a box from --worker-* instead would unpin the jobs and let a
phase kill a machine the evidence does not name.
HV-5 (Tools/Hv5TelemetrySweepRunner): one SHARED campaign fanned across every box
(separate campaigns would let per-campaign differences masquerade as per-machine
ones), a fallback + NoKvSlot log sweep, and the diagnosability drill. The drill asks
HV-5's actual question -- is what was RETAINED enough to name the cause without a
live debugger? -- so it asserts on the task's own error text, not on anything only
reachable by logging into the box. An unswept box is recorded as a FAILURE rather
than skipped: in a "zero fallback markers" claim, an unswept box is the claim being
wrong.
Schema agreement is read off the LIVE payload rather than a deserialized DTO, which
would normalize away the exact difference the check exists to find -- a box omitting
a property still deserializes into a default, and the shapes would always agree.
Two bugs found by running them, both fixed here:
* Ssh() folded stderr into its return value. Current OpenSSH prints a multi-line
post-quantum advisory to stderr on EVERY connection, so "how many ollama
processes remain" came back with a banner glued to the number and failed a phase
whose subject had passed.
* The Ollama stop matched the exact process name, missing the tray supervisor
"ollama app" (with a space) that restarts the server within seconds. Absence
never stuck on the laptop.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…isrupted
Caught by running them. On the first two-machine kill run the ssh kill returned
empty output against HardcoreLaptopMSI and the worker simply kept running. The job
then completed normally -- and every check in the phase went green, including
"death-is-visible-not-silent", because "reached a terminal state" is trivially true
of a job that was never disrupted. HardcorePC's half was genuine in the same run
(status=pending, claimedBy=none), so the evidence file read as a clean two-machine
pass with one half fabricated.
Two fixes, both about proving the precondition before judging anything by it -- the
same discipline the ollama phase already had and these two lacked:
* kill-actually-landed: count theorc-warband after the kill and abandon the phase
loudly if anything survived, rather than proceeding to measure an undisturbed
worker.
* "completed" is no longer accepted as visible failure in either the kill or
disconnect phase. A job that finished rode straight through the disruption. The
landed-gate catches the common case, but a kill arriving in a job's last second
would otherwise still slip through green.
Also here: "visible" was redefined for both phases after watching what actually
happens. A killed worker's job is re-queued to pending with its attempt advanced,
and it stays there -- attempts only advance when a worker claims and then goes
silent, and with the box down nobody claims. Demanding a terminal status failed the
CORRECT behaviour (the work is retryable and was not lost) and could only ever pass
by waiting out a timeout. Visibility now means the job stops being attributed to the
dead worker promptly, which is what the Warchief's "heartbeat timeout from <worker>
— re-queued (attempt N)" line reports, and a new check proves the re-queued work is
actually RECOVERED on the restarted worker rather than merely re-queued.
Restart is Stop-then-Start: killing the daemon leaves the scheduled task's own state
ambiguous, and Start-ScheduledTask on a task the scheduler still considers running is
a no-op that would have been blamed on recovery.
HardcorePC kill phase, all five checks green, with the death genuinely observed:
kill/death-is-visible-not-silent PASS status=pending, claimedBy=none
kill/requeued-work-is-recovered PASS same unit completed on NativeRoleRuntime
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ble failure" means Evidence in .orc/hv-4-lane/ (gitignored, on the Warchief box as with every prior HV run): kill HardcorePC + HardcoreLaptopMSI 6/6 checks each disconnect HardcorePC 3/3 ollama HardcorePC + HardcoreLaptopMSI 4/4 cancel HardcorePC 3/3 "Visible failure" had to be defined against what actually happens. A killed or disconnected worker's job is re-queued to pending with its attempt advanced and STAYS there -- attempts only advance when a worker claims and then goes silent, and with the box down nobody claims. The first check demanded a terminal status, which failed the correct behaviour and could only have passed by waiting out a timeout. Visibility is now "the job stops being attributed to the dead worker, promptly", which is what the Warchief's re-queue line reports, plus a new check that the re-queued unit is actually RECOVERED on the restarted worker. Ssh() now retries once. HardcoreLaptopMSI's sshd goes unreachable for minutes while the box stays healthy (ping 5 ms, telemetry answering 200), which is what made the first laptop kill silently no-op. Item 1 stays half-covered and the doc says why: the worker has no task-cancel endpoint, so a campaign cancel is a Warchief-side outcome only while the worker generates to completion. That is a product gap, not a harness one, and closing it is a mutation needing auth and its own security review -- same call as HV-3's MarkRoleDegraded. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…start Yesterday's fix (c4bb6a7) guaranteed the worker gets restarted after every kill, but it did that by moving the restart into a `finally` that runs AFTER the try block -- and the try block's last step was polling for the re-queued unit to be recovered. That poll needs a LIVE worker to answer it (the unit is pinned to this worker via ExcludedWorkerIds, so nobody else can claim it either), and the worker was still dead at that point. HardcorePC's kill lane, green all day, immediately started failing on kill/requeued-work-is-recovered as a direct result: kill/requeued-work-is-recovered: FAIL - the SAME work unit reached status=pending after the restart (claimedBy=none, runtime=none) Restructured with an idempotent EnsureRestartedAsync(), called from two places: inline in the happy path right after the death-visible check (before polling for recovery, since that poll needs the worker back), and from `finally` to still cover the early-return race path. A `restartAttempted` guard keeps it from running twice when both call sites would otherwise fire in the same execution. Also stopped kill/requeued-work-is-recovered from silently vanishing from the evidence when the worker never rejoins at all -- that is the clearest possible failure of the check, not an inapplicable one, and it now records itself as such rather than disappearing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 15
🧹 Nitpick comments (8)
Tools/Hv6RepeatabilityRunner/Program.cs (1)
454-474: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winNo timeout on a lane child process.
await p.WaitForExitAsync()is unbounded, so a single wedged lane runner hangs the entire unattended 3× campaign with no evidence written. A per-lane cap (cancellation token onWaitForExitAsyncplusKill(entireProcessTree: true), recorded as aTIMEOUTverdict) keeps the remaining rounds meaningful.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Tools/Hv6RepeatabilityRunner/Program.cs` around lines 454 - 474, Update RunLaneAsync to enforce a per-lane timeout by awaiting WaitForExitAsync with a cancellation token, and on cancellation kill the child process tree. Record the timeout as a TIMEOUT verdict while preserving stdout/stderr capture and allowing the campaign to continue.Tools/Hv4RecoveryRunner/Program.cs (1)
812-822: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winNew
HttpClientper telemetry probe inside 2s/3s polling loops.
WaitForTelemetryGoneAsync/WaitForTelemetryAsynccall this every couple of seconds for up to 3 minutes, so each phase burns dozens of disposed handlers and their sockets sit in TIME_WAIT. A singlestatic readonly HttpClientwith a 10s timeout would do.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Tools/Hv4RecoveryRunner/Program.cs` around lines 812 - 822, The TryFetchTelemetryAsync method creates and disposes an HttpClient for every telemetry probe. Reuse a single static readonly HttpClient configured with the existing 10-second timeout, and update TryFetchTelemetryAsync to use it while preserving the current request URL, success-status handling, JSON deserialization, and null-on-error behavior.Tools/Hv5TelemetrySweepRunner/Program.cs (1)
256-268: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winWhole log pulled over ssh with
Get-Content -Rawinto memory.Worker logs grow unbounded across an HV-6 campaign; a multi-hundred-MB read per box per round is avoidable by doing the matching remotely (
Select-String -Pattern ... -SimpleMatch) and returning only hits plus a line count, keeping the quotedDetailbehaviour.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Tools/Hv5TelemetrySweepRunner/Program.cs` around lines 256 - 268, The log-reachability check around the SSH command currently retrieves the entire worker log into memory. Update this command to perform the required matching remotely with PowerShell Select-String using the existing pattern and SimpleMatch, returning only matching hits plus a line count; preserve the __MISSING__ handling and the existing quoted Detail behavior for reachable and missing logs.docs/NATIVE_RUNTIME_HIVE_VALIDATION_PLAN.md (1)
634-634: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuemarkdownlint MD040: add a language to these fenced blocks.
textis fine for the evidence excerpts and the lane matrix.Also applies to: 663-663, 703-703, 735-735, 797-797, 872-872
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/NATIVE_RUNTIME_HIVE_VALIDATION_PLAN.md` at line 634, Update the fenced code blocks at the referenced sections of the validation plan to specify the text language, using the existing evidence excerpts and lane matrix content unchanged.Source: Linters/SAST tools
OrchestratorIDE/Services/Hive/HiveTaskQueue.cs (1)
569-582: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueOrphaned doc comment now sits on the wrong method.
The "Atomically selects and claims the best pending task…" summary belonged to
HandleLeaseAsync; the new method was inserted between them, so that text now documentsRecordIneligibility(whose own summary explicitly says it does not change dispatch), andHandleLeaseAsyncat Line 634 is left undocumented. Move it back down.📝 Proposed fix
- /// <summary> - /// Atomically selects and claims the best pending task that the worker can execute. This - /// removes the Phase 3A GET-next/POST-claim race while leaving those endpoints compatible. - /// </summary> /// <summary> /// Records, per pending campaign unit, why the polling worker could not take it, and logs theand re-attach it immediately above
HandleLeaseAsync.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@OrchestratorIDE/Services/Hive/HiveTaskQueue.cs` around lines 569 - 582, Move the “Atomically selects and claims the best pending task…” XML summary from above RecordIneligibility to immediately above HandleLeaseAsync. Keep RecordIneligibility’s existing documentation as its only summary and preserve the summary text unchanged.OrchestratorIDE.UnitTests/HiveUnsatisfiableReasonTests.cs (1)
51-51: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
HiveTaskQueueisIDisposableand starts a 10s watchdog timer — dispose it.Every test constructs one and drops it, leaking a
System.Threading.Timerper case;CheckTimeoutscan also tick mid-assertion in a slow run and mutate the seeded entry.using var queue = new HiveTaskQueue();throughout. Same inHiveHeartbeatBookkeepingTests.cs.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@OrchestratorIDE.UnitTests/HiveUnsatisfiableReasonTests.cs` at line 51, Update all test methods in HiveUnsatisfiableReasonTests and HiveHeartbeatBookkeepingTests that instantiate HiveTaskQueue so each declaration uses a using scope, ensuring the disposable queue and its watchdog timer are released after the test.OrchestratorIDE/Services/Hive/HiveWorkerAgent.cs (1)
344-377: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winOne rejection-body reader, copied into two classes.
HiveWorkerAgent.ReadReasonAsyncandHiveMeshHeartbeat.ReadRejectionReasonAsyncare the same implementation — same 400-char cap, same JSONerrorextraction, same raw-body fallback, same swallow. Extract a single shared internal static (alongsideHiveAuthMiddleware.Clampis a natural home) so the newline-sanitisation fix and any future hardening land once instead of twice.
OrchestratorIDE/Services/Hive/HiveWorkerAgent.cs#L344-L377: delete the local copy and call the shared helper from the three sites that use it.OrchestratorIDE/Services/Hive/HiveMeshHeartbeat.cs#L232-L262: delete the local copy, call the shared helper, and applyReplaceLineEndings(" ")to the fallback inside it.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@OrchestratorIDE/Services/Hive/HiveWorkerAgent.cs` around lines 344 - 377, Extract the duplicate rejection-body parsing from HiveWorkerAgent.ReadReasonAsync and HiveMeshHeartbeat.ReadRejectionReasonAsync into one shared internal static helper near HiveAuthMiddleware.Clamp. Preserve the 400-character cap, JSON error extraction, exception swallowing, and raw-body fallback, while applying ReplaceLineEndings(" ") to the fallback; delete both local implementations and update all three HiveWorkerAgent call sites plus the HiveMeshHeartbeat call site to use the shared helper. Affected sites: OrchestratorIDE/Services/Hive/HiveWorkerAgent.cs:344-377 and OrchestratorIDE/Services/Hive/HiveMeshHeartbeat.cs:232-262.OrchestratorIDE.UnitTests/CampaignIneligibilityExplanationTests.cs (1)
121-121: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse
HiveExecutionKinds.ContainerPackinstead of a hand-typed near-miss literal.
"container-pack"(hyphen) is not the real kind —HiveTaskBundledocuments the vocabulary aslegacy_agent | native_agent | container_pack. The test passes either way because the worker only advertisesNativeAgent, but the literal reads as if it were exercising a real kind and would survive a rename of the constant.♻️ Proposed change
- ExecutionKind = "container-pack", + ExecutionKind = HiveExecutionKinds.ContainerPack,🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@OrchestratorIDE.UnitTests/CampaignIneligibilityExplanationTests.cs` at line 121, In the test setup containing ExecutionKind, replace the hand-typed "container-pack" literal with the existing HiveExecutionKinds.ContainerPack constant. Preserve the test’s other configuration and assertions unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/NATIVE_RUNTIME_HIVE_VALIDATION_PLAN.md`:
- Around line 811-814: Update the mitigation description near the
HardcoreLaptopMSI scenario to state that Hv4RecoveryRunner.Ssh makes three
connection attempts with 10s, 20s, and 30s timeouts and backoff, rather than
saying the driver retries once.
- Around line 861-870: Correct the HV-2/HV-6 design paragraph to match the
shipped `Hv6RepeatabilityRunner` behavior: describe large-context lanes as
running through a separate `--large-only` invocation, with the context size set
once before round 1 and restored once in `finally`. Remove claims about
per-round standard restoration, lane ordering, mid-campaign switching, telemetry
verification, and recording `NOT-RUN`; state that failed reconfiguration throws
and aborts the run.
In `@OrchestratorIDE.UnitTests/HiveHeartbeatBookkeepingTests.cs`:
- Around line 112-121: Replace the short Task.Delay intervals in the heartbeat
advancement tests, including the loop around HeartbeatCoreAsync and
Heartbeat_ForALiveClaim_AdvancesLastHeartbeat, with a sufficiently long delay or
a clock-controlled approach that guarantees DateTime.UtcNow advances before
asserting Is.GreaterThan(last). Preserve the existing heartbeat outcome and
timestamp advancement assertions.
In `@OrchestratorIDE/Services/Hive/HiveMeshHeartbeat.cs`:
- Around line 232-262: Update ReadRejectionReasonAsync so the non-JSON fallback
sanitizes the trimmed remote response by removing or replacing carriage returns
and newlines before returning it for logging. Preserve the existing truncation,
JSON error extraction, and exception-swallowing behavior.
In `@OrchestratorIDE/Services/Hive/HiveTaskQueue.cs`:
- Around line 1065-1102: Update HandleHeartbeatAsync to delegate heartbeat
validation and timestamping entirely to HeartbeatCoreAsync, removing its own
_claimLock acquisition and duplicated claim/token logic. Map
HeartbeatOutcome.NotClaimed and StaleToken to the existing 409 responses and
status payloads, and map Credited to the alive response, preserving null-body
token behavior through the shared method.
- Around line 79-96: Make the IneligibleFor collection in HiveTaskQueue
concurrent-safe by replacing its Dictionary implementation with
ConcurrentDictionary while preserving the existing case-insensitive key comparer
and public property behavior. Ensure RecordIneligibility, HandleLeaseAsync, and
UnsatisfiableReasonFor continue using it without additional locking.
In `@OrchestratorIDE/Services/Hive/HiveWorkerAgent.cs`:
- Around line 490-506: Update UploadOrFailAsync to preserve an existing
execution failure: before replacing status and errorMsg in its general exception
handler, check whether the task is already failed with a recorded error from
ExecuteNativeRoleTaskAsync. Keep the original error and return the empty
artifact list without overwriting taskResult.ErrorMsg; only record the upload
failure when no earlier execution error exists.
- Around line 687-690: Hoist the HttpClient created in the Hive worker beat
logic out of the while loop so a single client is reused for the job’s entire
loop. Keep its 20-second timeout and continue passing it to the blocking Send
call, while preserving the existing request cadence and cancellation behavior.
In `@start-warchief.bat`:
- Around line 43-50: Update the startup block around the cd /d command to abort
immediately when changing to the required SwarmCLI working directory fails.
Ensure the absolute executable launch does not proceed after a failed directory
change, while preserving the existing working-directory and redirect behavior on
success.
In `@Tools/Hv4RecoveryRunner/Program.cs`:
- Around line 882-894: Update the SSH helper in
Tools/Hv4RecoveryRunner/Program.cs:882-894,
Tools/Hv5TelemetrySweepRunner/Program.cs:652-670, and
Tools/Hv6RepeatabilityRunner/Program.cs:426-450 to start asynchronous stdout and
stderr reads concurrently, enforce each existing timeout, kill the entire
process tree on timeout, and await the reads without blocking indefinitely.
Reuse the HV-4 retry behavior in the HV-5 driver, and extract the shared helper
into a linked source file to keep all three drivers consistent.
- Around line 851-861: Update Ssh and its command handling so retries are based
on an explicit success sentinel rather than empty stdout. Ensure silent
side-effecting commands such as Stop-Process and the
Stop-ScheduledTask/Start-ScheduledTask restart append or otherwise return a
consistent sentinel (as disconnect already does with 'unblocked'), while
preserving retry behavior for genuine failures and avoiding repeated execution
after success.
In `@Tools/Hv5TelemetrySweepRunner/Program.cs`:
- Around line 42-48: Update the FallbackMarkers array to remove the bare "11434"
substring and replace it with a marker anchored to an Ollama URL or host shape,
such as ":11434" or "localhost:11434", while preserving the existing fallback
markers.
In `@Tools/Hv6RepeatabilityRunner/Program.cs`:
- Around line 149-159: The round-loop exception path currently lets
reconfiguration or lane-launch failures escape before reports are written. Add a
catch alongside the existing try/finally around the campaign loop, record the
exception details in report.Error, and then allow execution to continue to the
existing JSON/markdown report-writing flow.
- Around line 75-93: Remove the developer-specific fallback values from the
Fleet initialization in Program, including hostnames, Tailscale addresses, task
names, log paths, checkout directories, and worker IDs. Require these values
through the existing argument flow or an environment/JSON fleet configuration,
and fail loudly with a clear validation error when required fleet settings are
missing; preserve only genuinely safe defaults such as the low-VRAM worker
selection when its prerequisite worker configuration is available.
- Around line 217-241: Update the final verdict calculation in the main runner
flow so report.Passed is true only when all rounds pass and report.FleetRestored
is true. Preserve the existing round-count and round-result checks, ensuring a
failed fleet restore produces the failure summary and nonzero exit code.
---
Nitpick comments:
In `@docs/NATIVE_RUNTIME_HIVE_VALIDATION_PLAN.md`:
- Line 634: Update the fenced code blocks at the referenced sections of the
validation plan to specify the text language, using the existing evidence
excerpts and lane matrix content unchanged.
In `@OrchestratorIDE.UnitTests/CampaignIneligibilityExplanationTests.cs`:
- Line 121: In the test setup containing ExecutionKind, replace the hand-typed
"container-pack" literal with the existing HiveExecutionKinds.ContainerPack
constant. Preserve the test’s other configuration and assertions unchanged.
In `@OrchestratorIDE.UnitTests/HiveUnsatisfiableReasonTests.cs`:
- Line 51: Update all test methods in HiveUnsatisfiableReasonTests and
HiveHeartbeatBookkeepingTests that instantiate HiveTaskQueue so each declaration
uses a using scope, ensuring the disposable queue and its watchdog timer are
released after the test.
In `@OrchestratorIDE/Services/Hive/HiveTaskQueue.cs`:
- Around line 569-582: Move the “Atomically selects and claims the best pending
task…” XML summary from above RecordIneligibility to immediately above
HandleLeaseAsync. Keep RecordIneligibility’s existing documentation as its only
summary and preserve the summary text unchanged.
In `@OrchestratorIDE/Services/Hive/HiveWorkerAgent.cs`:
- Around line 344-377: Extract the duplicate rejection-body parsing from
HiveWorkerAgent.ReadReasonAsync and HiveMeshHeartbeat.ReadRejectionReasonAsync
into one shared internal static helper near HiveAuthMiddleware.Clamp. Preserve
the 400-character cap, JSON error extraction, exception swallowing, and raw-body
fallback, while applying ReplaceLineEndings(" ") to the fallback; delete both
local implementations and update all three HiveWorkerAgent call sites plus the
HiveMeshHeartbeat call site to use the shared helper. Affected sites:
OrchestratorIDE/Services/Hive/HiveWorkerAgent.cs:344-377 and
OrchestratorIDE/Services/Hive/HiveMeshHeartbeat.cs:232-262.
In `@Tools/Hv4RecoveryRunner/Program.cs`:
- Around line 812-822: The TryFetchTelemetryAsync method creates and disposes an
HttpClient for every telemetry probe. Reuse a single static readonly HttpClient
configured with the existing 10-second timeout, and update
TryFetchTelemetryAsync to use it while preserving the current request URL,
success-status handling, JSON deserialization, and null-on-error behavior.
In `@Tools/Hv5TelemetrySweepRunner/Program.cs`:
- Around line 256-268: The log-reachability check around the SSH command
currently retrieves the entire worker log into memory. Update this command to
perform the required matching remotely with PowerShell Select-String using the
existing pattern and SimpleMatch, returning only matching hits plus a line
count; preserve the __MISSING__ handling and the existing quoted Detail behavior
for reachable and missing logs.
In `@Tools/Hv6RepeatabilityRunner/Program.cs`:
- Around line 454-474: Update RunLaneAsync to enforce a per-lane timeout by
awaiting WaitForExitAsync with a cancellation token, and on cancellation kill
the child process tree. Record the timeout as a TIMEOUT verdict while preserving
stdout/stderr capture and allowing the campaign to continue.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: fadaf146-1341-4270-ab3f-3a1da002eef0
📒 Files selected for processing (21)
OrchestratorIDE.UnitTests/CampaignIneligibilityExplanationTests.csOrchestratorIDE.UnitTests/HiveAuthSignRoundTripTests.csOrchestratorIDE.UnitTests/HiveHeartbeatBookkeepingTests.csOrchestratorIDE.UnitTests/HivePairingSecretDerivationTests.csOrchestratorIDE.UnitTests/HiveUnsatisfiableReasonTests.csOrchestratorIDE.UnitTests/HiveWorkerErrorChainTests.csOrchestratorIDE/Services/Hive/CampaignContracts.csOrchestratorIDE/Services/Hive/HiveMeshHeartbeat.csOrchestratorIDE/Services/Hive/HiveTaskBundle.csOrchestratorIDE/Services/Hive/HiveTaskQueue.csOrchestratorIDE/Services/Hive/HiveWorkerAgent.csTools/Hv3LifecycleRunner/Program.csTools/Hv4RecoveryRunner/Hv4RecoveryRunner.csprojTools/Hv4RecoveryRunner/Program.csTools/Hv5TelemetrySweepRunner/Hv5TelemetrySweepRunner.csprojTools/Hv5TelemetrySweepRunner/Program.csTools/Hv6RepeatabilityRunner/Hv6RepeatabilityRunner.csprojTools/Hv6RepeatabilityRunner/Program.csTools/SwarmCli/Program.csdocs/NATIVE_RUNTIME_HIVE_VALIDATION_PLAN.mdstart-warchief.bat
🚧 Files skipped from review as they are similar to previous changes (1)
- Tools/Hv3LifecycleRunner/Program.cs
| /// <summary> | ||
| /// Best-effort read of a peer's rejection reason for logging only. Truncated and | ||
| /// exception-swallowing on purpose — an untrusted remote body on a diagnostic path must never | ||
| /// turn a handled HTTP error into a crash. | ||
| /// </summary> | ||
| private static async Task<string?> ReadRejectionReasonAsync( | ||
| HttpResponseMessage response, CancellationToken ct) | ||
| { | ||
| try | ||
| { | ||
| var body = await response.Content.ReadAsStringAsync(ct).ConfigureAwait(false); | ||
| if (string.IsNullOrWhiteSpace(body)) return null; | ||
| if (body.Length > 400) body = body[..400] + "…"; | ||
|
|
||
| try | ||
| { | ||
| using var doc = JsonDocument.Parse(body); | ||
| if (doc.RootElement.ValueKind == JsonValueKind.Object | ||
| && doc.RootElement.TryGetProperty("error", out var err) | ||
| && err.ValueKind == JsonValueKind.String) | ||
| return err.GetString(); | ||
| } | ||
| catch (JsonException) { /* not JSON — fall through to the raw body */ } | ||
|
|
||
| return body.Trim(); | ||
| } | ||
| catch | ||
| { | ||
| return null; | ||
| } | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Consider stripping newlines from the untrusted body before logging.
The non-JSON fallback writes a remote peer's raw response text straight into the log line. Embedded \r\n lets a hostile or merely broken peer forge additional log entries in the very diagnostic an operator is reading. Cheap to neutralise:
🛡️ Proposed fix
- return body.Trim();
+ return body.Trim().ReplaceLineEndings(" ");The duplication of this helper with HiveWorkerAgent.ReadReasonAsync is covered separately below.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /// <summary> | |
| /// Best-effort read of a peer's rejection reason for logging only. Truncated and | |
| /// exception-swallowing on purpose — an untrusted remote body on a diagnostic path must never | |
| /// turn a handled HTTP error into a crash. | |
| /// </summary> | |
| private static async Task<string?> ReadRejectionReasonAsync( | |
| HttpResponseMessage response, CancellationToken ct) | |
| { | |
| try | |
| { | |
| var body = await response.Content.ReadAsStringAsync(ct).ConfigureAwait(false); | |
| if (string.IsNullOrWhiteSpace(body)) return null; | |
| if (body.Length > 400) body = body[..400] + "…"; | |
| try | |
| { | |
| using var doc = JsonDocument.Parse(body); | |
| if (doc.RootElement.ValueKind == JsonValueKind.Object | |
| && doc.RootElement.TryGetProperty("error", out var err) | |
| && err.ValueKind == JsonValueKind.String) | |
| return err.GetString(); | |
| } | |
| catch (JsonException) { /* not JSON — fall through to the raw body */ } | |
| return body.Trim(); | |
| } | |
| catch | |
| { | |
| return null; | |
| } | |
| } | |
| /// <summary> | |
| /// Best-effort read of a peer's rejection reason for logging only. Truncated and | |
| /// exception-swallowing on purpose — an untrusted remote body on a diagnostic path must never | |
| /// turn a handled HTTP error into a crash. | |
| /// </summary> | |
| private static async Task<string?> ReadRejectionReasonAsync( | |
| HttpResponseMessage response, CancellationToken ct) | |
| { | |
| try | |
| { | |
| var body = await response.Content.ReadAsStringAsync(ct).ConfigureAwait(false); | |
| if (string.IsNullOrWhiteSpace(body)) return null; | |
| if (body.Length > 400) body = body[..400] + "…"; | |
| try | |
| { | |
| using var doc = JsonDocument.Parse(body); | |
| if (doc.RootElement.ValueKind == JsonValueKind.Object | |
| && doc.RootElement.TryGetProperty("error", out var err) | |
| && err.ValueKind == JsonValueKind.String) | |
| return err.GetString(); | |
| } | |
| catch (JsonException) { /* not JSON — fall through to the raw body */ } | |
| return body.Trim().ReplaceLineEndings(" "); | |
| } | |
| catch | |
| { | |
| return null; | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@OrchestratorIDE/Services/Hive/HiveMeshHeartbeat.cs` around lines 232 - 262,
Update ReadRejectionReasonAsync so the non-JSON fallback sanitizes the trimmed
remote response by removing or replacing carriage returns and newlines before
returning it for logging. Preserve the existing truncation, JSON error
extraction, and exception-swallowing behavior.
| var fleet = new Fleet | ||
| { | ||
| WorkerAId = GetArg(args, "--worker-a") ?? "HardcorePC", | ||
| WorkerANode = GetArg(args, "--worker-a-node") ?? "http://100.102.190.112:7078", | ||
| WorkerASsh = GetArg(args, "--worker-a-ssh") ?? "100.102.190.112", | ||
| WorkerATask = GetArg(args, "--worker-a-task") ?? "TheOrcWorker", | ||
| WorkerALog = GetArg(args, "--worker-a-log") ?? @"F:\Ai\OrchestratorIDE-dev\worker_hpc.log", | ||
| WorkerBId = GetArg(args, "--worker-b") ?? "HardcoreLaptopMSI", | ||
| WorkerBNode = GetArg(args, "--worker-b-node") ?? "http://100.114.151.4:7078", | ||
| WorkerBSsh = GetArg(args, "--worker-b-ssh") ?? "100.114.151.4", | ||
| WorkerBTask = GetArg(args, "--worker-b-task") ?? "TheOrcLaptopWorker", | ||
| WorkerBLog = GetArg(args, "--worker-b-log") ?? @"C:\Ai\OrchestratorIDE-dev\worker_laptop.log", | ||
| // Defaults to worker A because that is HardcorePC's 6 GB card against the laptop's 8 GB. | ||
| // Override when the fleet shape changes; HV-2's `large` phase is meaningless if this | ||
| // names the box with headroom. | ||
| WorkerADir = GetArg(args, "--worker-a-dir") ?? @"F:\Ai\OrchestratorIDE-dev", | ||
| WorkerBDir = GetArg(args, "--worker-b-dir") ?? @"C:\Ai\OrchestratorIDE-dev", | ||
| LowVramWorkerId = GetArg(args, "--low-vram-worker") ?? GetArg(args, "--worker-a") ?? "HardcorePC", | ||
| }; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Fleet hostnames, Tailscale IPs and local checkout paths are baked in as defaults.
http://100.102.190.112:7078, F:\Ai\OrchestratorIDE-dev, task names, etc. are one developer's environment committed as the fallback for every argument. Anyone running without flags silently targets that fleet. Prefer required arguments (or an env/JSON fleet file) so a missing flag fails loudly instead of resolving to someone else's machines.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Tools/Hv6RepeatabilityRunner/Program.cs` around lines 75 - 93, Remove the
developer-specific fallback values from the Fleet initialization in Program,
including hostnames, Tailscale addresses, task names, log paths, checkout
directories, and worker IDs. Require these values through the existing argument
flow or an environment/JSON fleet configuration, and fail loudly with a clear
validation error when required fleet settings are missing; preserve only
genuinely safe defaults such as the low-VRAM worker selection when its
prerequisite worker configuration is available.
Two more full 3x runs after the restart-ordering fix (76bb267): main campaign (hv2-large excluded): 5 of 9 lanes green in all 3 rounds. Every remaining failure is HardcoreLaptopMSI's job finishing before a kill/disconnect landed -- the same diagnosed ssh/power-plan limit, not a recurrence of the dead-worker bug. No worker was left unrecovered this run. --large-only (hv2-large alone, its own 3x invocation): clean. One new, one-off finding recorded rather than dismissed: R1's hv3-sequential showed ConversationsCreated [9, 9, 10] on HardcorePC (cycles 1->2 landed on the same conversation) while R2 and R3 on the identical fleet were clean ([8, 9, 10] both). Most likely a warm-fleet artifact from the smoke-testing immediately before it, not chased further on a single occurrence. Verdict: the harness and its fixes are solid; "all green" is not yet met, and the gap is one understood external limit (that laptop's Balanced power plan under ssh load) rather than anything in the runtime or queue. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…elper, HV-6 verdict Grok's full review of PR #92 found a BLOCKER; CodeRabbit's found a Critical concurrency bug and 19 more. This fixes the safety-critical ones plus several quick wins, in one pass since they touch overlapping files. BLOCKER (Grok) — RuntimeOrchestrator.cs: a role's VRAM ledger entry could shrink within the same generation. GetConversationForBindingAsync commits EnsureAdmitted's returned requiredBytes, which is the FULL footprint on a fresh load but only the INCREMENTAL cost once a role reuses its own already-resident base. A role that paid the full cost once and later reuses would have its entry silently overwritten with the smaller number -- nothing was actually freed on the GPU. In the static-budget fallback (no live nvidia-smi probe), that ledger is the ONLY signal a later role's admission check has, so the shrunk entry under-counts real usage and can over-admit into an actual OOM. Fixed with a generation-scoped floor (Math.Max), the same scoping EnsureAdmitted already uses a few lines above for thisRolePriorReserved -- a genuine decrease is only ever legitimate alongside a generation change (a real recycle), so the floor must not apply across one. Regression test added (skipped without THEORC_TEST_GGUF, same convention as the rest of this suite). Critical (CodeRabbit) — HiveTaskQueue.cs: IneligibleFor was a plain Dictionary written under _claimLock and read without it from HandleGetTaskAsync (every request runs on its own task), so a status poll landing during a lease poll could throw "Collection was modified" -- swallowed by HandleAsync's blanket catch into a dropped/partial response on exactly the polling path every HV driver depends on. Fixed by making it a ConcurrentDictionary. Major x2 (CodeRabbit) — the Ssh() helper, copy-pasted into three HV drivers, had two bugs: (1) retrying on empty stdout re-ran side-effecting commands that legitimately print nothing (Stop-Process, Stop-ScheduledTask+Start-ScheduledTask) up to 3x -- the restart in particular re-stopped a worker that had just come back up, which is a strong candidate for a good deal of the "HardcoreLaptopMSI flakiness" this campaign spent hours attributing to sshd/power-plan limits; (2) synchronous ReadToEnd() had no timeout of its own, so a genuinely hung ssh session blocked forever regardless of the WaitForExit(ms) that used to follow it. Fixed in all three drivers with an explicit completion marker (success is now "the marker arrived", not "stdout was non-empty") and a real, cancellable timeout that kills the process tree on expiry. Major (CodeRabbit) — Hv6RepeatabilityRunner: an exception inside the round loop (a failed reconfiguration, a missing lane exe) used to propagate past Main and kill the process before any report was written -- exactly backwards for the run whose evidence is most needed when something breaks. Now caught and recorded as report.Error, and report.FleetRestored is folded into the pass/fail verdict and exit code so a run that could not restore the fleet no longer prints PASS. Quick wins (CodeRabbit): "11434" as a bare substring in the fallback sweep matched any timestamp/byte-count/task-id containing those digits -- anchored to ":11434"/"localhost:11434". Task.Delay(10)/(20) in the heartbeat bookkeeping test was below Windows' ~15.6ms DateTime.UtcNow tick granularity -- bumped to 40ms. start-warchief.bat's `cd /d` failure was silently ignored -- now checked. Two stale doc paragraphs corrected to describe the driver as actually shipped (three-attempt ssh retry, not one; HV-6's split design, not its superseded mid-campaign-switch predecessor). 678/691 unit tests pass (13 skipped, all requiring THEORC_TEST_GGUF, same as before this change). Not yet addressed, left as follow-ups: Hv3LifecycleRunner's cross-role aggregation in fresh-conversation checks (Major), HiveMeshHeartbeat's raw-body log-injection risk (Minor), HeartbeatCoreAsync/HandleHeartbeatAsync duplication (Minor), an upload failure that can overwrite an already-diagnosed execution error (Minor), a fresh HttpClient per heartbeat beat (Major/perf), Hv6's baked-in fleet defaults (Minor), and the --leave-hive fail-open/validation gaps in OrchestratorIDE.Daemon/Program.cs (Major, pre-existing). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
docs/NATIVE_RUNTIME_HIVE_VALIDATION_PLAN.md (1)
795-806: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winDo not claim cross-machine coverage for single-machine lanes.
The heading says “PASS on BOTH machines,” but
disconnectandcancelran only on HardcorePC. The final verdict should distinguish the cross-machine kill/Ollama coverage from the HardcorePC-only lanes.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/NATIVE_RUNTIME_HIVE_VALIDATION_PLAN.md` around lines 795 - 806, Update the “Results” heading and summary in the validation plan to distinguish lanes executed on both HardcorePC and HardcoreLaptopMSI from the HardcorePC-only disconnect and cancel lanes. Preserve the existing PASS outcomes, but explicitly label kill and ollama as cross-machine coverage and disconnect and cancel as HardcorePC-only.OrchestratorIDE.UnitTests/RuntimeOrchestratorTests.cs (1)
261-275: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDerive the test budget from the scheduler’s actual estimates.
Line 265 assumes all context, CUDA, and compute costs fit in 40% of the GGUF size. Smaller valid GGUFs can exceed that allowance, causing the first admission to fail before this test reaches shared-base reuse. Size the synthetic card from fresh and reused estimates instead.
Proposed fix
- var loaded = false; - var totalBytes = (long)(sizeBytes * 1.4); + var options = new RuntimeOptions(ContextLength: 2048, GpuLayers: -1); + var freshBytes = OrcScheduler.EstimateRequiredBytes(workerBinding, options); + var reusedBytes = OrcScheduler.EstimateRequiredBytes( + researcherBinding, options, baseWeightsAlreadyResident: true); + Assert.That(reusedBytes, Is.GreaterThan(0).And.LessThan(freshBytes)); + + var loaded = false; + var totalBytes = checked(freshBytes + reusedBytes); VramBudget Provider() => new(totalBytes, loaded ? sizeBytes : 0L); @@ - var options = new RuntimeOptions(ContextLength: 2048, GpuLayers: -1);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@OrchestratorIDE.UnitTests/RuntimeOrchestratorTests.cs` around lines 261 - 275, Update the budget setup in the test around Provider and the RuntimeOptions-based admission flow to derive totalBytes from the scheduler’s actual fresh and reused estimates, rather than multiplying sizeBytes by 1.4. Ensure the synthetic VRAM budget admits one fresh model plus the reused-context increment while still rejecting two full model copies, so the test consistently exercises shared-base reuse for all valid GGUF sizes.Tools/Hv6RepeatabilityRunner/Program.cs (2)
367-397: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winRetrying the fleet reconfiguration command can re-trigger the worker restart it just performed.
The doc comment above
Ssh(Lines 431-439) explicitly calls out this exact risk — "the underlying command also stops/starts a scheduled task, and a spurious retry of THAT is exactly the class of bug that turned a single kill into three in Hv4RecoveryRunner" — but Line 382-389 still hands the combined rewrite+restart+read-back as a single string to the retryingSsh()helper. If the SSH connection drops after the remote script already stopped/restarted the task but before the local process observed the done-marker (exactly the kind of flakiness this file documents forHardcoreLaptopMSI),Ssh()'s retry will re-run the whole script, restarting the worker again mid-campaign.Split the non-idempotent "act" step (single best-effort attempt) from the read-only "verify" step (safe to retry).
🛠️ Proposed fix
- var readBack = await Ssh(ssh, - "powershell -NoProfile -Command \"" + - $"(Get-Content '{script}') -replace 'NATIVECONTEXTSIZE=\\d+','NATIVECONTEXTSIZE={contextSize}' " + - $"| Set-Content '{script}'; " + - $"Stop-ScheduledTask -TaskName {task} -ErrorAction SilentlyContinue; Start-Sleep -Seconds 3; " + - "Get-Process theorc-warband -ErrorAction SilentlyContinue | Stop-Process -Force; " + - $"Start-Sleep -Seconds 2; Start-ScheduledTask -TaskName {task}; Start-Sleep -Seconds 12; " + - $"(Select-String -Path '{script}' -Pattern 'NATIVECONTEXTSIZE=(\\d+)').Matches.Groups[1].Value\""); - - var applied = readBack.Trim(); + // Single best-effort attempt — restarting the worker is NOT safe to silently re-run, + // so this must never go through Ssh()'s retry loop. + await SshOnce(ssh, + "powershell -NoProfile -Command \"" + + $"(Get-Content '{script}') -replace 'NATIVECONTEXTSIZE=\\d+','NATIVECONTEXTSIZE={contextSize}' " + + $"| Set-Content '{script}'; " + + $"Stop-ScheduledTask -TaskName {task} -ErrorAction SilentlyContinue; Start-Sleep -Seconds 3; " + + "Get-Process theorc-warband -ErrorAction SilentlyContinue | Stop-Process -Force; " + + $"Start-Sleep -Seconds 2; Start-ScheduledTask -TaskName {task}; Start-Sleep -Seconds 12\"", + connectTimeoutSec: 30); + + // Read-only — safe to retry. + var readBack = await Ssh(ssh, + $"powershell -NoProfile -Command \"(Select-String -Path '{script}' " + + "-Pattern 'NATIVECONTEXTSIZE=(\\d+)').Matches.Groups[1].Value\""); + + var applied = readBack.Trim();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Tools/Hv6RepeatabilityRunner/Program.cs` around lines 367 - 397, Update SetFleetContextSizeAsync so the non-idempotent rewrite and worker restart command is issued only once, without the retrying Ssh helper; then perform the read-only NATIVECONTEXTSIZE verification in a separate Ssh call that may retry safely. Preserve the existing applied-value comparison and failure reporting for each worker.
499-519: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winNo timeout on lane child-process execution — a hung lane blocks the whole unattended campaign.
Unlike
SshOnce, which enforces a 180s deadline viaCancellationTokenSource,await p.WaitForExitAsync()here has none. HV-6 exists specifically to run 3 rounds unattended; if any lane (hv1..hv5) hangs, the entire multi-round run stalls indefinitely with no evidence written until someone notices and kills it manually.🛠️ Proposed fix
using var p = Process.Start(psi)!; // Read both streams concurrently. Reading one to completion first can deadlock the child // once the other pipe's buffer fills, and these lanes are chatty enough to reach it. var stdoutTask = p.StandardOutput.ReadToEndAsync(); var stderrTask = p.StandardError.ReadToEndAsync(); - await p.WaitForExitAsync(); + using var cts = new CancellationTokenSource(TimeSpan.FromMinutes(15)); + try + { + await p.WaitForExitAsync(cts.Token); + } + catch (OperationCanceledException) + { + try { p.Kill(entireProcessTree: true); } catch { /* best-effort */ } + } var stdout = await stdoutTask; var stderr = await stderrTask; if (!string.IsNullOrWhiteSpace(stderr)) stdout += "\n[stderr]\n" + stderr; - return (p.ExitCode, stdout); + return (cts.IsCancellationRequested ? -1 : p.ExitCode, stdout); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Tools/Hv6RepeatabilityRunner/Program.cs` around lines 499 - 519, Update RunLaneAsync to enforce a finite timeout while awaiting the child process, matching the existing SshOnce 180-second deadline behavior. Use cancellation or equivalent timeout handling around p.WaitForExitAsync, ensure a timed-out lane is terminated and its captured output is still returned or reported, and preserve the concurrent stdout/stderr draining.
🧹 Nitpick comments (1)
Tools/Hv6RepeatabilityRunner/Program.cs (1)
430-495: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Ssh/SshOnceare duplicated near-verbatim across Hv4RecoveryRunner, Hv5TelemetrySweepRunner, and this file.Per the cross-file graph context, the same retry/backoff and done-marker parsing logic exists in
Tools/Hv4RecoveryRunner/Program.csandTools/Hv5TelemetrySweepRunner/Program.cs. This diff's own comment (Lines 431-439) shows fixes are being ported by hand between copies — a sharedTools-level SSH helper library would let a fix land once instead of needing to be replicated (and re-verified) in each runner.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Tools/Hv6RepeatabilityRunner/Program.cs` around lines 430 - 495, Extract the duplicated Ssh and SshOnce retry, timeout, marker, and output-parsing logic into a shared helper under Tools, then update Hv4RecoveryRunner, Hv5TelemetrySweepRunner, and Hv6RepeatabilityRunner to use it. Preserve the existing retry/backoff behavior, SshDoneMarker handling, stderr draining, and failure results while removing the near-identical private implementations from each runner.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/NATIVE_RUNTIME_HIVE_VALIDATION_PLAN.md`:
- Line 937: Update the fenced code block in
NATIVE_RUNTIME_HIVE_VALIDATION_PLAN.md to include the text language identifier
after its opening fence, preserving the block’s contents.
- Around line 957-973: Update the HV-6 verdict in the document to explicitly
retain the unresolved HardcorePC hv3-sequential anomaly as a remaining failure,
alongside the HardcoreLaptopMSI SSH issue. Revise the “Every remaining failure”
statement so it does not attribute all failures solely to HardcoreLaptopMSI, and
preserve the requirement that the [9, 9, 10] anomaly remains open until a rerun
resolves it.
In `@OrchestratorIDE.UnitTests/RuntimeOrchestratorTests.cs`:
- Around line 209-226: Update both GetConversationForBindingAsync calls in the
test to pass explicit RuntimeOptions so EstimateRequiredBytes uses the
context-aware reuse path. After the first call, assert the worker reservation is
greater than sizeBytes, then retain the existing afterReuse >= afterFirstLoad
assertion to verify the ledger does not shrink.
---
Outside diff comments:
In `@docs/NATIVE_RUNTIME_HIVE_VALIDATION_PLAN.md`:
- Around line 795-806: Update the “Results” heading and summary in the
validation plan to distinguish lanes executed on both HardcorePC and
HardcoreLaptopMSI from the HardcorePC-only disconnect and cancel lanes. Preserve
the existing PASS outcomes, but explicitly label kill and ollama as
cross-machine coverage and disconnect and cancel as HardcorePC-only.
In `@OrchestratorIDE.UnitTests/RuntimeOrchestratorTests.cs`:
- Around line 261-275: Update the budget setup in the test around Provider and
the RuntimeOptions-based admission flow to derive totalBytes from the
scheduler’s actual fresh and reused estimates, rather than multiplying sizeBytes
by 1.4. Ensure the synthetic VRAM budget admits one fresh model plus the
reused-context increment while still rejecting two full model copies, so the
test consistently exercises shared-base reuse for all valid GGUF sizes.
In `@Tools/Hv6RepeatabilityRunner/Program.cs`:
- Around line 367-397: Update SetFleetContextSizeAsync so the non-idempotent
rewrite and worker restart command is issued only once, without the retrying Ssh
helper; then perform the read-only NATIVECONTEXTSIZE verification in a separate
Ssh call that may retry safely. Preserve the existing applied-value comparison
and failure reporting for each worker.
- Around line 499-519: Update RunLaneAsync to enforce a finite timeout while
awaiting the child process, matching the existing SshOnce 180-second deadline
behavior. Use cancellation or equivalent timeout handling around
p.WaitForExitAsync, ensure a timed-out lane is terminated and its captured
output is still returned or reported, and preserve the concurrent stdout/stderr
draining.
---
Nitpick comments:
In `@Tools/Hv6RepeatabilityRunner/Program.cs`:
- Around line 430-495: Extract the duplicated Ssh and SshOnce retry, timeout,
marker, and output-parsing logic into a shared helper under Tools, then update
Hv4RecoveryRunner, Hv5TelemetrySweepRunner, and Hv6RepeatabilityRunner to use
it. Preserve the existing retry/backoff behavior, SshDoneMarker handling, stderr
draining, and failure results while removing the near-identical private
implementations from each runner.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: c7cb7bdb-ea14-453f-83e2-8f0d953faf19
📒 Files selected for processing (9)
OrchestratorIDE.UnitTests/HiveHeartbeatBookkeepingTests.csOrchestratorIDE.UnitTests/RuntimeOrchestratorTests.csOrchestratorIDE/Core/Runtime/RuntimeOrchestrator.csOrchestratorIDE/Services/Hive/HiveTaskQueue.csTools/Hv4RecoveryRunner/Program.csTools/Hv5TelemetrySweepRunner/Program.csTools/Hv6RepeatabilityRunner/Program.csdocs/NATIVE_RUNTIME_HIVE_VALIDATION_PLAN.mdstart-warchief.bat
🚧 Files skipped from review as they are similar to previous changes (6)
- OrchestratorIDE.UnitTests/HiveHeartbeatBookkeepingTests.cs
- start-warchief.bat
- Tools/Hv5TelemetrySweepRunner/Program.cs
- OrchestratorIDE/Services/Hive/HiveTaskQueue.cs
- Tools/Hv4RecoveryRunner/Program.cs
- OrchestratorIDE/Core/Runtime/RuntimeOrchestrator.cs
| **With both fixed, the main campaign (3×, `hv2-large` excluded) ran clean apart from the box's own | ||
| known ssh/timing limits:** | ||
|
|
||
| ``` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add a language identifier to the fenced block.
Use a lexer such as text after the opening fence so markdownlint MD040 passes.
🧰 Tools
🪛 markdownlint-cli2 (0.23.0)
[warning] 937-937: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/NATIVE_RUNTIME_HIVE_VALIDATION_PLAN.md` at line 937, Update the fenced
code block in NATIVE_RUNTIME_HIVE_VALIDATION_PLAN.md to include the text
language identifier after its opening fence, preserving the block’s contents.
Source: Linters/SAST tools
| **One new, one-off finding: R1's `hv3-sequential` shows `ConversationsCreated` (after-cycle samples | ||
| only) of `[9, 9, 10]` on HardcorePC** — cycles 1→2 landed on the same conversation instead of a | ||
| fresh one, while R2 (`[8, 9, 10]`) and R3 (`[8, 9, 10]`) on the identical fleet, same session, were | ||
| clean. R1 ran immediately after several minutes of heavy reconfiguration/campaign churn from manual | ||
| smoke-testing earlier in the session, which is the more likely explanation than a reproducible | ||
| defect — but it is recorded here rather than dismissed, per the plan's own precedent that "a | ||
| conversation silently reused and never re-counted" is exactly the failure this check exists to | ||
| catch. Not chased further on a single occurrence; worth a second look if HV-6 reproduces it on a | ||
| cold, uninterrupted fleet. | ||
|
|
||
| **HV-6 verdict: the harness and the fixes it drove are solid; "all green" is not yet met, and the | ||
| gap is a single, understood, external limit.** Every remaining failure across both post-split runs | ||
| is HardcoreLaptopMSI's ssh/scheduling behavior under load, previously diagnosed (Balanced power | ||
| plan) — not the runtime, not the queue, not a silent fallback, and (after the two fixes above) not | ||
| this driver leaving a worker dead. Closing HV-6 fully means either accepting that limit as a | ||
| recorded, permanent caveat on this fleet's evidence, or changing the laptop's power plan (a machine | ||
| setting, not code — not done without confirming first) and re-running. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Retain the unresolved HardcorePC HV-3 anomaly in the final verdict.
The table records hv3-sequential as FAIL in R1, and Lines 957-965 attribute it to HardcorePC’s [9, 9, 10] conversation count. Therefore, “every remaining failure” being a HardcoreLaptopMSI SSH issue is inaccurate; keep this internal anomaly explicitly open unless a rerun resolves it.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/NATIVE_RUNTIME_HIVE_VALIDATION_PLAN.md` around lines 957 - 973, Update
the HV-6 verdict in the document to explicitly retain the unresolved HardcorePC
hv3-sequential anomaly as a remaining failure, alongside the HardcoreLaptopMSI
SSH issue. Revise the “Every remaining failure” statement so it does not
attribute all failures solely to HardcoreLaptopMSI, and preserve the requirement
that the [9, 9, 10] anomaly remains open until a rerun resolves it.
| using (await orchestrator.GetConversationForBindingAsync(workerBinding).ConfigureAwait(false)) | ||
| { | ||
| } | ||
| var afterFirstLoad = orchestrator.GetReservationSnapshot()!.Reservations | ||
| .Single(r => r.Role == RuntimeRole.Worker).Bytes; | ||
| Assert.That(afterFirstLoad, Is.GreaterThan(0), "first (fresh) load must reserve something"); | ||
|
|
||
| // Same role, same binding, base weights now resident from the call above -- this is the | ||
| // reuse admission whose returned requiredBytes is smaller than the first call's. | ||
| using (await orchestrator.GetConversationForBindingAsync(workerBinding).ConfigureAwait(false)) | ||
| { | ||
| } | ||
| var afterReuse = orchestrator.GetReservationSnapshot()!.Reservations | ||
| .Single(r => r.Role == RuntimeRole.Worker).Bytes; | ||
|
|
||
| Assert.That(afterReuse, Is.GreaterThanOrEqualTo(afterFirstLoad), | ||
| "the role's ledger entry must never shrink within the same generation -- the resident " + | ||
| "base model this role itself loaded has not gone anywhere just because THIS call reused it"); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Exercise the reuse estimate rather than legacy admission.
Lines 209 and 218 omit RuntimeOptions; that keeps EstimateRequiredBytes on its legacy full-file-size path for both calls. The ledger would therefore remain unchanged even if the Math.Max floor regressed, so this test does not cover the reported shrink scenario. Pass explicit context options to both calls and assert the first reservation exceeds sizeBytes to prove the context-aware path ran.
Proposed fix
+ var options = new RuntimeOptions(ContextLength: 2048, GpuLayers: -1);
+
- using (await orchestrator.GetConversationForBindingAsync(workerBinding).ConfigureAwait(false))
+ using (await orchestrator.GetConversationForBindingAsync(workerBinding, options).ConfigureAwait(false))
{
}
var afterFirstLoad = orchestrator.GetReservationSnapshot()!.Reservations
.Single(r => r.Role == RuntimeRole.Worker).Bytes;
- Assert.That(afterFirstLoad, Is.GreaterThan(0), "first (fresh) load must reserve something");
+ Assert.That(afterFirstLoad, Is.GreaterThan(sizeBytes),
+ "the fresh context-aware admission must include non-model allocation costs");
- using (await orchestrator.GetConversationForBindingAsync(workerBinding).ConfigureAwait(false))
+ using (await orchestrator.GetConversationForBindingAsync(workerBinding, options).ConfigureAwait(false))
{
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| using (await orchestrator.GetConversationForBindingAsync(workerBinding).ConfigureAwait(false)) | |
| { | |
| } | |
| var afterFirstLoad = orchestrator.GetReservationSnapshot()!.Reservations | |
| .Single(r => r.Role == RuntimeRole.Worker).Bytes; | |
| Assert.That(afterFirstLoad, Is.GreaterThan(0), "first (fresh) load must reserve something"); | |
| // Same role, same binding, base weights now resident from the call above -- this is the | |
| // reuse admission whose returned requiredBytes is smaller than the first call's. | |
| using (await orchestrator.GetConversationForBindingAsync(workerBinding).ConfigureAwait(false)) | |
| { | |
| } | |
| var afterReuse = orchestrator.GetReservationSnapshot()!.Reservations | |
| .Single(r => r.Role == RuntimeRole.Worker).Bytes; | |
| Assert.That(afterReuse, Is.GreaterThanOrEqualTo(afterFirstLoad), | |
| "the role's ledger entry must never shrink within the same generation -- the resident " + | |
| "base model this role itself loaded has not gone anywhere just because THIS call reused it"); | |
| var options = new RuntimeOptions(ContextLength: 2048, GpuLayers: -1); | |
| using (await orchestrator.GetConversationForBindingAsync(workerBinding, options).ConfigureAwait(false)) | |
| { | |
| } | |
| var afterFirstLoad = orchestrator.GetReservationSnapshot()!.Reservations | |
| .Single(r => r.Role == RuntimeRole.Worker).Bytes; | |
| Assert.That(afterFirstLoad, Is.GreaterThan(sizeBytes), | |
| "the fresh context-aware admission must include non-model allocation costs"); | |
| // Same role, same binding, base weights now resident from the call above -- this is the | |
| // reuse admission whose returned requiredBytes is smaller than the first call's. | |
| using (await orchestrator.GetConversationForBindingAsync(workerBinding, options).ConfigureAwait(false)) | |
| { | |
| } | |
| var afterReuse = orchestrator.GetReservationSnapshot()!.Reservations | |
| .Single(r => r.Role == RuntimeRole.Worker).Bytes; | |
| Assert.That(afterReuse, Is.GreaterThanOrEqualTo(afterFirstLoad), | |
| "the role's ledger entry must never shrink within the same generation -- the resident " + | |
| "base model this role itself loaded has not gone anywhere just because THIS call reused it"); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@OrchestratorIDE.UnitTests/RuntimeOrchestratorTests.cs` around lines 209 -
226, Update both GetConversationForBindingAsync calls in the test to pass
explicit RuntimeOptions so EstimateRequiredBytes uses the context-aware reuse
path. After the first call, assert the worker reservation is greater than
sizeBytes, then retain the existing afterReuse >= afterFirstLoad assertion to
verify the ledger does not shrink.
…ned early HV-3's fresh-conversation-per-job check was taking the MAX ConversationsCreated across every resident role instead of the role this phase actually exercises. Invisible until a second role was already resident with a similar-or-higher count: running hv3-concurrent (loads Researcher) immediately before hv3-sequential (loads Coder/Worker) in the same HV-6 round left Researcher's counter flat at 3 while Worker's climbed 1->2->3 underneath it -- and the max-across-roles metric reported [3, 3, 3] for two of three cycles, a false flatline, while the real per-role behavior was completely correct the whole time. Reproduced 3/3 rounds against HardcorePC alone before the fix, confirmed fixed by re-running the exact concurrent-then-sequential sequence that exposed it: ConversationsCreated now correctly reads [2, 3, 4]. SampleAsync takes an optional role filter; RunSequentialAsync passes the runtime role name for the phase's own role (mirroring HiveNativeRoleExecutorAdapter.MapHiveRoleToRuntimeRole's tested mapping locally rather than referencing Core.Runtime from a Tools/ driver). Concurrent's mid-flight sampling is unaffected -- it deliberately wants the true cross-role max, since it is asking whether TWO roles hold reservations at once. Also confirms, via 3 clean rounds run directly against HardcorePC while the laptop was contended by a separate local session: hv4-kill (with yesterday's restart-ordering fix), hv4-cancel, and hv3-concurrent are all solid -- 6/6, 3/3, and 2/2 checks respectively, every round. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…on prompt length Root-caused why HardcoreLaptopMSI's kill/disconnect jobs kept outrunning their own disruption despite the earlier "at least twenty sections" prompt: a single-turn "write a long document" request does NOT reliably occupy MaxSteps. An LLM can (and, per the fleet logs, often does) emit the whole thing in one completion -- `steps: 1` -- so wall-clock time is bounded purely by that one call's raw token-generation speed, and a fast card can finish well under the time it takes this driver to ssh in and apply a kill or firewall rule. LongSpec now asks for 20 separate file-creation steps against a MaxSteps ceiling of 12 -- deliberately more than the loop can ever complete, so the job is guaranteed to run the FULL step budget regardless of model speed, rather than hoping a longer prompt happens to take long enough. Each step costs a full extra model round trip (generate -> execute tool -> re-invoke with extended context), which is overhead a fast GPU cannot shrink away the way it can raw token generation. Verified on the fleet: hv4-kill went from failing on this exact race to 3+ clean rounds after the change (kill's landing is a single ssh Stop-Process call, near -instant). hv4-disconnect needed an additional change -- its landing (create a firewall rule + verify) is inherently slower, several times kill's latency -- so its fixed 3s post-creation sleep (present only so the SAME ssh session's own verification could see the just-created rule) is cut to 1s. A poll-loop version of this was tried first and reverted: it failed 3/3 with an empty read-back, which means the loop's braces/semicolons did not survive the ssh -> cmd -> powershell quoting chain intact -- the same class of failure already called out in this file's comments for the New-NetFirewallRule call itself. The flat-sleep shape is proven to survive that chain; only its duration changed. Both changes are shared by kill and disconnect (one spec, one driver) rather than tuned per-phase, since 20 steps costs kill nothing it wasn't already comfortably beating. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…named hardware limit
Full campaign run on a confirmed-idle fleet (laptop CPU 4% before starting) after
both fixes:
7 of 9 lanes: PASS every round (hv1, hv2-small, hv3-sequential, hv3-concurrent,
hv4-cancel, hv4-ollama, hv5)
hv4-kill: PASS R2, FAIL R1/R3 (HardcoreLaptopMSI only)
hv4-disconnect: FAIL all 3 rounds (HardcoreLaptopMSI only)
Isolated the disconnect failure before accepting it: cancel/ollama/kill's own ssh
calls all succeeded reliably in the same 75-minute run while disconnect's firewall
-rule call failed 3/3 -- narrow enough to be suspicious of a scripting bug rather
than a hardware limit. Reproduced the driver's EXACT command, including the
appended completion-marker suffix, by hand against the idle box -- twice, both
clean. That rules out a quoting/scripting defect. The difference is that in the
real campaign this ssh call fires while the box is actively serving the 20-step
generation job, and disconnect's landing sequence (create + sleep + verify,
several round trips) spends more time exposed to a CPU-saturated sshd than kill's
single near-instant Stop-Process call does -- the same CPU-bound sshd failure
mode diagnosed earlier in this document, narrowed one level further: a function
of how long the ssh action takes, not of the box or command being broken.
Verdict: 7/9 lanes repeatedly, robustly proven across multiple full campaigns
today. The 2 remaining failures are conclusively external and narrow, not a
driver/queue/runtime defect. HV-6 should be read as evidenced-with-one-named
-exception for the §6 decision, not as failing outright.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…rom this driver Cut the pre-verify sleep from 1s to 0s (create+verify stay one ssh call/session -- confirmed safe by hand against the idle box first, so this carries none of the cross-connection risk the file already documents for a two-call split). Result, 3 clean rounds against a confirmed-idle fleet: statistically IDENTICAL to the 1s version -- hv4-kill PASS/PASS/FAIL (same residual race as before), hv4-disconnect FAIL/FAIL/FAIL, every failure the same "block rule not present" symptom. That is the decisive answer, not another data point to argue with: two different sleep durations producing the same outcome proves the sleep was never the bottleneck, and by extension that no command-latency reduction in this driver changes anything here. What varies is whether the ssh session completes AT ALL while this box is CPU-loaded serving the induced job -- a property of sshd under load, not of how fast the command inside the session runs. This driver has no further lever over that. Stopping further code-side attempts at this specific gap. The two remaining paths are unchanged: accept it as a permanent, named caveat on this fleet's evidence, or an OS/hardware-level change to that one machine, which is outside this session's scope to make unilaterally. Retrying the same lever again after a controlled negative result would not be persistence, it would be ignoring the experiment. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…t bottleneck Third and final controlled experiment on the hv4-disconnect gap, after the sleep duration experiments (1s -> 0s, no change) already ruled out command latency. Added real SSH connection multiplexing: WarmSshConnectionAsync establishes an authenticated ControlMaster connection immediately, before the induced job is even submitted, while the box is still idle -- so the one unavoidable CPU-bound key exchange happens on a machine that can spare the cycles for it. Every subsequent Ssh() call reuses it via ControlMaster=auto (falling back to an ordinary connection per call if the warm-up never happened or failed -- no regression risk). Verified by hand first: a command needing 10-30s of connect-timeout retries completed in 0.35s over a pre-warmed connection. Result against the real fleet: in the one round where the pre-warm CONFIRMED succeeded (verified via `-O check`), the disruptive command still failed with the exact same "block rule not present" symptom. That is the decisive data point -- it rules out the SSH transport/handshake layer as the bottleneck entirely, since multiplexing correctly bypassed the repeat-handshake cost and the failure persisted anyway. The actual contention is in remote PROCESS EXECUTION -- spawning powershell.exe, loading the NetSecurity module, calling into the Windows Firewall API -- competing for CPU with the concurrently-running inference job. No SSH-layer change touches that. Three independent levers (sleep duration x2, transport-layer reuse) have now each produced a clean negative or disproved the working theory. Not attempting a fourth of the same class; the remaining paths are what they were before this: accept the gap as a permanent, named caveat, or address it at the OS/hardware level on that one machine, which is outside this session's scope to do unilaterally. The multiplexing infrastructure itself is kept -- real, harmless, and correctly falls back to ordinary per-call connections when unavailable, same as before it existed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…one per beat CodeRabbit finding (HiveWorkerAgent.cs:690, Major): HeartbeatLoop created a fresh HttpClient on every iteration of its 10s-cadence loop, re-paying DNS resolution and connection setup on exactly the path this loop exists to keep timely -- the opposite of what a timing-sensitive heartbeat needs, and the same class of cost this file's own recent history has been fighting (the 5s->20s timeout bump, the dedicated-thread move) for the identical reason: anything that makes a beat late risks the queue's 45s re-queue firing on a healthy worker. Hoisted to once per HeartbeatLoop invocation. Safe without any locking or lifetime bookkeeping: this method already runs on its own dedicated thread per task (HeartbeatLoopAsync spins one Thread per task), so there is no cross-task sharing to reason about -- the client lives and dies with that one thread's `using`, just declared once instead of every beat. Hive suite 136/136. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…odes
Two CodeRabbit findings (Daemon/Program.cs, Major) on the headless --leave-hive
path added in an earlier session:
1. HiveIdentity.Load() silently regenerates a brand-new identity -- different
NodeId, different keys -- on ANY decrypt/parse failure of the stored file,
including transient causes this same codebase already documents as real and
observed (the two-AppData-views collision noted in HiveNodeServer's history).
For most callers that lenient default is reasonable self-healing. For
--leave-hive it is dangerous: its entire contract is "clear hive membership,
keep NodeId/keys/peer-secrets unchanged," and silently operating on a freshly
generated identity instead means the command "succeeds" while abandoning the
membership of an identity that was never touched -- no error, no sign anything
unusual happened.
Added `Load(bool regenerateOnCorruption = true)`. Every existing caller is
unaffected (default unchanged). --leave-hive passes `false`: a stored file
that exists but fails to load now throws instead of regenerating, and the CLI
catches that and refuses to proceed rather than leaving a hive under the wrong
identity. IdentityPath made public so the error can name the actual file.
2. `if (args.Contains("--leave-hive"))` alone doesn't notice a mixed invocation
like `--leave-hive --yes --pair --target host` -- a copy-paste leftover, or a
genuine attempt to chain leave-then-pair atomically, which this daemon does
NOT support in one call (see the "pair immediately after --leave-hive --yes,
before any daemon start" rule elsewhere in the docs, which this exact footgun
motivated). Silently running --leave-hive and dropping --pair/--target/
--show-identity on the floor doesn't tell the operator their command did not
do what its other half implied. Now rejected explicitly with the conflicting
flag named.
No new unit test for the corruption-load path: HiveIdentity.Load is a
process-wide static singleton over a fixed filesystem path with no injectable
seam for a controlled corrupt file, and adding one would be a larger change than
the fix itself for a peripheral finding. Verified instead by full Hive suite
(136/136) and a build-level check of both new code paths.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… heartbeat fix Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…is further Tried raising the spawned PowerShell process's own scheduling priority to High as its first statement -- a real, standard technique for keeping a short-lived administrative task responsive opposite a CPU-heavy workload, verified by hand against the idle box first. Result against the real fleet: 3/3 identical failures, same "block rule not present" symptom as every prior attempt. That result is informative rather than just another negative. Priority elevation only takes effect once a process starts executing statements -- if it made no difference, the bottleneck is not in-process scheduling, it is in PROCESS CREATION itself: the delay between ssh requesting a new remote command and that process actually being scheduled to run anything, including its own priority- boost statement. No script content can touch a delay that happens before the script starts. This identifies a genuinely different remediation on the OTHER side of the contention, recorded in the plan doc: lower the native inference worker's OWN process priority (e.g. BelowNormal) so the OS scheduler naturally prefers any newly-spawned Normal-priority administrative process over it, rather than trying to boost the admin side after the fact -- the same technique background scanners/indexers use to avoid starving foreground work. That is a real, bounded, low-risk change, but it is a PRODUCT-level change to OrchestratorIDE.NativeRuntime, not a Tools/ driver tweak, and needs a decision rather than more unilateral iteration in this file. Four independent levers now tried and ruled out for this specific gap. Nothing further planned in Tools/Hv4RecoveryRunner itself. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… log CodeRabbit finding (HiveMeshHeartbeat.cs:262, Minor): a heartbeat rejection reason was read from an UNTRUSTED remote peer's HTTP response body and logged verbatim, truncated to 400 chars but with no newline stripping. A malicious or compromised peer could return a body containing embedded newlines formatted to look like fake log entries -- a classic log-forging attack -- making forged lines indistinguishable from genuine ones to an operator or an automated log scanner. Fixed at the read site (ReadRejectionReasonAsync) rather than the one current call site, so any future caller gets a safe, single-line value automatically. Collapses all whitespace runs (not just \r\n) to a single space -- keeps the content readable while making it structurally impossible for a peer-controlled string to introduce a new physical line into the log. Sanitize exposed internal so it's unit-testable directly, without mocking the HTTP plumbing around it for what is really a one-function fix. 5 new tests plus the full Hive suite: 141/141. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…s cause CodeRabbit finding (HiveWorkerAgent.cs:506, Minor), in code I wrote earlier today (0e9db76's fail-closed upload handling). UploadOrFailAsync's catch block overwrote errorMsg unconditionally -- so if the agent execution itself already failed for a real reason (status already "failed", errorMsg already set) and the SUBSEQUENT artifact upload then also failed, the generic "Output artifact upload failed: ..." message silently replaced the original, more useful execution error. Losing the real cause behind a wrapper message is exactly the class of bug the error-chain-preservation fix elsewhere in this same file (78874e5) exists to prevent -- this was the same mistake, introduced the same day, on the artifact side instead of the exception side. A secondary upload failure is plausible precisely when it matters most: if the worker or network is degraded enough to fail the upload, that degradation may well be what caused the execution to fail in the first place, and both pieces of information are worth keeping. Fixed by appending rather than replacing when a real execution failure is already recorded; the upload failure only becomes the SOLE error when execution otherwise succeeded and the upload is the entire story. No new unit test: this is a local closure inside ClaimAndExecuteAsync with real HTTP/execution dependencies, and isolating it would need more mocking infrastructure than this fix is worth. Verified by the full Hive suite instead: 141/141, no regressions. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ating it HandleHeartbeatAsync re-implemented the same claimed/stale-token/credit decision that HeartbeatCoreAsync already made for tests, so the two could silently drift. HandleHeartbeatAsync now calls HeartbeatCoreAsync and only translates the outcome to an HTTP response. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…test EnsureAdmitted_ReadmissionAfterOwnLoad_DoesNotShrinkThisRolesLedgerEntry called GetConversationForBindingAsync with no RuntimeOptions, so both calls resolved through EstimateRequiredBytes' options-is-null legacy path -- the same "legacy" file-size number every time, since the reuse discount only exists on the context-aware estimate. The floor logic the test claims to guard was never actually exercised. Pass RuntimeOptions (matching the sibling test in this file) so the second call genuinely produces a smaller number and the floor has something to do. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… null RuntimeOrchestrator.GetReservationSnapshot() returns null before the native runtime has a scheduler/budget wired up (or if the provider throws), which made TotalBytes/ReservedBytes/AvailableBytes/RejectedAdmissionCount null in the /hive/native-telemetry response. Tools/Hv3LifecycleRunner's NativeTelemetry DTO declares these non-nullable, so deserialization failed and the driver reported the whole worker unreachable instead of "not yet admitting" -- a false negative on exactly the signal HV-3/HV-5 sweep for. Default to 0. Also strengthens the ledger-floor regression test per the same review pass: asserts the fresh context-aware reservation exceeds the raw model size, to prove the context-aware estimate path (not the legacy fallback) actually ran. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…v's machines WorkerA/B hostnames, Tailscale IPs, task names and checkout paths were hardcoded as fallback defaults on every fleet argument. Anyone invoking the driver without flags would silently target this developer's own machines. Now required (RequireArg, same fail-loud pattern already used for --model-hash) except the low-VRAM worker id, which is a safe derivation once worker-a is known. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ly open markdownlint MD040: the lane table's fence had no language, add \`text\`. The HV-6 verdict said "every remaining failure" was HardcoreLaptopMSI's ssh/scheduling limit, but R1's hv3-sequential FAIL on HardcorePC (the [9, 9, 10] conversation-count anomaly recorded a few paragraphs above) is a separate, unresolved internal finding, not that same external limit -- it must not be implicitly folded into "all failures are explained." Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…g its return Records the blocker so it's not lost: both remaining paths (accept the disconnect gap as a permanent caveat, or apply the power-plan changes and re-run) require the machine to be reachable, which it currently is not. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
BelowNormal looked like a clean fix against synthetic CPU burners, but real ggml inference disproved it: the native thread pool sets one compute thread to THREAD_PRIORITY_HIGHEST, a +2 offset relative to the process's own class. At BelowNormal (base 6) that thread claws back up to priority 8 - identical to a freshly-spawned admin shell's default - so SSH exec landing under load was a coin flip, not a fix. Idle (base 4) puts the same elevated thread at 6, safely below a fresh shell's 8; verified 3/3 via direct PID-before/after confirmation under real sustained inference load. Full three-round trail in docs/NATIVE_RUNTIME_HIVE_VALIDATION_PLAN.md (HV-6 / hv4-disconnect). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
AppSettings.ExperimentalNativeHiveWorkerEnabled and ExperimentalNativeMainChatEnabled now default to true. Recorded as the explicit product decision NATIVE_RUNTIME_V2_SPEC.md §6 requires, against HV-1 through HV-6 evidence: 7/9 HV-6 lanes robustly green across multiple full fleet campaigns, with one named caveat accepted at decision time (SSH-delivered admin actions against HardcoreLaptopMSI under live inference load) - narrow, isolated to test-harness disruption delivery on one machine, not a HIVE dispatch/scheduling/admission/ fallback defect. Ollama stays fully implemented as the other IModelRuntime backend, just no longer the default construction path. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
HV-3 (docs/NATIVE_RUNTIME_HIVE_VALIDATION_PLAN.md) asserts that a role's
residency returns to baseline BETWEEN jobs while its VRAM reservation does not --
the documented decoupling between the two. On a headless worker that assertion
was unobservable: the daemon wired NativeTelemetryProvider to
GetReservationSnapshot() alone, so ActiveCount/ConversationsCreated/Status
existed in-process and nowhere else. Same shape of gap HV-2 found and closed for
admission counters, one level down.
The response stays additive. Reservation fields keep their exact top-level
position because Tools/Hv2SchedulingRunner binds them there, and HV-6 re-runs
that driver unattended 3x -- nesting them to make the payload tidier would break
a driver the campaign depends on for no gain.
AdapterRoleResidency.Binding is projected to display names rather than
serialized whole: it carries absolute GGUF/adapter paths and this endpoint is
unauthenticated (same posture as /hive/info). The phase asserts on role, counts
and status; the worker's local filesystem layout is not part of that and does
not need publishing to any caller that can reach the port.
Co-Authored-By: Claude Opus 5 noreply@anthropic.com
Summary by CodeRabbit
--leave-hivemode with--yesconfirmation and clearer operator output.