Merge fix/c3-worker-residuals: WRK-33 observed command-task wrapper, WRK-32 pre-baseline pump + recency contract documented, WRK-34 Watchdog CommandMethod label
This commit is contained in:
@@ -18,7 +18,7 @@ This plan is executed as **seven parallel batches**, each by one subagent workin
|
|||||||
| C | `fix/c3-tracker-hygiene` | GWC-31 + housekeeping (close GWC-16/IPC-08/IPC-31, refresh TST-06/TST-17, register WRK-31) | sonnet | Cross-file comment/ID sweep needs care about what each ID *means*, but no logic changes | Not started |
|
| C | `fix/c3-tracker-hygiene` | GWC-31 + housekeeping (close GWC-16/IPC-08/IPC-31, refresh TST-06/TST-17, register WRK-31) | sonnet | Cross-file comment/ID sweep needs care about what each ID *means*, but no logic changes | Not started |
|
||||||
| D | `fix/c3-dashboard-acl` | SEC-40, SEC-41, GWC-38 | opus | Authorization semantics, fail-closed decisions, claims re-resolution; each needs a regression test | Not started |
|
| D | `fix/c3-dashboard-acl` | SEC-40, SEC-41, GWC-38 | opus | Authorization semantics, fail-closed decisions, claims re-resolution; each needs a regression test | Not started |
|
||||||
| E | `fix/c3-ipc-boundaries` | IPC-33, IPC-39, IPC-34, IPC-35 | opus | Changes the WorkerClient command lifecycle, the options validator, and the worker drain budget; race-sensitive | Not started |
|
| E | `fix/c3-ipc-boundaries` | IPC-33, IPC-39, IPC-34, IPC-35 | opus | Changes the WorkerClient command lifecycle, the options validator, and the worker drain budget; race-sensitive | Not started |
|
||||||
| F | `fix/c3-worker-residuals` | WRK-32, WRK-33 (+WRK-34 opportunistic) | sonnet | Small, precisely located worker edits + doc prose; cannot be built on macOS so the agent's job is a careful, minimal diff | Not started |
|
| F | `fix/c3-worker-residuals` | WRK-32, WRK-33 (+WRK-34 opportunistic) | sonnet | Small, precisely located worker edits + doc prose; cannot be built on macOS so the agent's job is a careful, minimal diff | Done-pending-windev |
|
||||||
| G | `fix/c3-session-lifecycle` | GWC-32, GWC-34, GWC-37 | opus | Distributor/dispose lifecycle races and an alarm-monitor re-resolve path; needs lock-discipline reasoning | Not started |
|
| G | `fix/c3-session-lifecycle` | GWC-32, GWC-34, GWC-37 | opus | Distributor/dispose lifecycle races and an alarm-monitor re-resolve path; needs lock-discipline reasoning | Not started |
|
||||||
|
|
||||||
**Out of scope for this execution (recorded, not dropped):**
|
**Out of scope for this execution (recorded, not dropped):**
|
||||||
@@ -66,6 +66,7 @@ This plan is executed as **seven parallel batches**, each by one subagent workin
|
|||||||
- WRK-32: one `pumpStep()` before capturing the `CurrentVersion` baseline in `MxAccessCommandExecutor.cs`; rewrite the executor doc comment (`:996-1003`) to state the recency-based contract; add the same to `docs/MxAccessWorkerInstanceDesign.md` write-completion section and the reply-field comment in `mxaccess_gateway.proto` (**comment-only proto edit → regenerate all generated artifacts in the same commit**, per the repo rule; if regen tooling is unavailable on macOS, skip the proto comment and say so).
|
- WRK-32: one `pumpStep()` before capturing the `CurrentVersion` baseline in `MxAccessCommandExecutor.cs`; rewrite the executor doc comment (`:996-1003`) to state the recency-based contract; add the same to `docs/MxAccessWorkerInstanceDesign.md` write-completion section and the reply-field comment in `mxaccess_gateway.proto` (**comment-only proto edit → regenerate all generated artifacts in the same commit**, per the repo rule; if regen tooling is unavailable on macOS, skip the proto comment and say so).
|
||||||
- WRK-34 if trivial: `CreateFault(StaHung, "Watchdog", …)` with the correlation id moved into the message.
|
- WRK-34 if trivial: `CreateFault(StaHung, "Watchdog", …)` with the correlation id moved into the message.
|
||||||
- Cannot build on macOS: keep diff minimal, net48-safe (no init-only/positional records). Verified on windev afterwards.
|
- Cannot build on macOS: keep diff minimal, net48-safe (no init-only/positional records). Verified on windev afterwards.
|
||||||
|
- **Done-pending-windev** (`93edb1b` WRK-33, `20a3eef` WRK-32, `3679e59` WRK-34) on `fix/c3-worker-residuals`. WRK-32 applied `CaptureCompletionBaseline` (pump-then-snapshot) uniformly to all four write call sites (Write/Write2/WriteSecured/WriteSecured2), not just the two the finding cited as evidence, since the correlation gap is identical at all four. **Deferred**: the `mxaccess_gateway.proto` reply-field comment was not edited — a comment-only proto change still requires regenerating every generated artifact (C#, Go, Python, Java, Rust, protoset) per the repo rule, and this macOS box cannot verify all five client generator pins for a Low-severity doc-only change; the contract is documented in `docs/MxAccessWorkerInstanceDesign.md` instead. `NonWindows.slnx` builds clean (0 warnings/errors); worker/Worker.Tests unverified until windev runs `MxAccessCommandExecutorTests`/`WorkerPipeSessionTests`.
|
||||||
|
|
||||||
### G — GWC-32, GWC-34, GWC-37 (opus)
|
### G — GWC-32, GWC-34, GWC-37 (opus)
|
||||||
- GWC-32: `DisposeAsync` transitions `_state` to `Closed` under `_syncRoot` before completing channels (or `if (_disposed || IsTerminalState()) return;` in `StageWorkerEvent`); update the XML doc to name all three completion paths. Test: dispose a Ready client mid-dispatch → no `ProtocolViolation` fault/metric.
|
- GWC-32: `DisposeAsync` transitions `_state` to `Closed` under `_syncRoot` before completing channels (or `if (_disposed || IsTerminalState()) return;` in `StageWorkerEvent`); update the XML doc to name all three completion paths. Test: dispose a Ready client mid-dispatch → no `ProtocolViolation` fault/metric.
|
||||||
|
|||||||
@@ -317,6 +317,30 @@ whose write workload is effectively fire-and-forget — no consumer reads the
|
|||||||
reply's `statuses` — can lower it, or set `0` to skip the wait entirely and
|
reply's `statuses` — can lower it, or set `0` to skip the wait entirely and
|
||||||
reply on acceptance alone.
|
reply on acceptance alone.
|
||||||
|
|
||||||
|
**Write-completion correlation is recency-based, not per-call.** MXAccess's
|
||||||
|
`OnWriteComplete` callback carries no correlation token — only the server/item
|
||||||
|
handle pair — so `MxAccessCommandExecutor` correlates a completion to a write
|
||||||
|
by *version*: it snapshots `MxAccessWriteCompletionCache.CurrentVersion` for
|
||||||
|
the handle as a baseline before the COM call, then reports the first
|
||||||
|
completion whose version exceeds that baseline as the write's outcome
|
||||||
|
(`AwaitWriteCompletion`). This is exact for the common case, but if an earlier
|
||||||
|
write to the same handle times out unconfirmed — its `OnWriteComplete` still
|
||||||
|
undelivered when the wait gives up — and the caller immediately issues another
|
||||||
|
write to the same tag, the stale callback can dispatch during the second
|
||||||
|
write's pump-wait and be reported as *its* outcome. The executor mitigates the
|
||||||
|
most common instance of this (`CaptureCompletionBaseline`) by running one
|
||||||
|
`pumpStep()` immediately before capturing the baseline, so a completion already
|
||||||
|
queued for the handle folds into the baseline instead of being available to
|
||||||
|
ride the next write's reply; this closes the gap for a callback that arrived
|
||||||
|
before the new write started, but a callback that MXAccess queues after that
|
||||||
|
pump step and before the COM call returns (or during the wait itself) is still
|
||||||
|
indistinguishable from the new write's own completion. Full closure is not
|
||||||
|
possible without an MXAccess-side correlation token, which does not exist —
|
||||||
|
this is a property of the COM surface, not a gap this worker introduced (a
|
||||||
|
native client correlating by handle has the identical ambiguity). The exposure
|
||||||
|
is bounded: `statuses` is an enrichment field, and `Hresult`/`ProtocolStatus`
|
||||||
|
stay acceptance-only regardless of which completion (if any) was attributed.
|
||||||
|
|
||||||
## COM Creation
|
## COM Creation
|
||||||
|
|
||||||
The MXAccess analysis source at `C:\Users\dohertj2\Desktop\mxaccess` identifies
|
The MXAccess analysis source at `C:\Users\dohertj2\Desktop\mxaccess` identifies
|
||||||
|
|||||||
@@ -1232,7 +1232,14 @@ public sealed class WorkerPipeSession
|
|||||||
_activeCommandTasks.Add(commandTask);
|
_activeCommandTasks.Add(commandTask);
|
||||||
}
|
}
|
||||||
|
|
||||||
_ = ObserveCommandTaskAsync(commandTask);
|
// WRK-33: every other discarded task in this file is wrapped with ObserveFaultWhenever
|
||||||
|
// (NEXT-04 discipline) so a fault reaching the finalizer unobserved is impossible. This
|
||||||
|
// wrapper task was the one exception — ObserveCommandTaskAsync only swallows
|
||||||
|
// OperationCanceledException, so any other exception escaping ProcessCommandAsync's own
|
||||||
|
// catch path (e.g. a future edit, or a failure past TryWriteFaultAsync's narrow filter)
|
||||||
|
// would fault the discarded task with no observer. Close the gap the same way the rest of
|
||||||
|
// the file does.
|
||||||
|
ObserveFaultWhenever(ObserveCommandTaskAsync(commandTask));
|
||||||
}
|
}
|
||||||
|
|
||||||
private async Task ObserveCommandTaskAsync(Task commandTask)
|
private async Task ObserveCommandTaskAsync(Task commandTask)
|
||||||
@@ -1381,11 +1388,18 @@ public sealed class WorkerPipeSession
|
|||||||
// the StaHung fault just sent. Without this the heartbeat loop keeps
|
// the StaHung fault just sent. Without this the heartbeat loop keeps
|
||||||
// advertising a non-faulted state that contradicts the fault.
|
// advertising a non-faulted state that contradicts the fault.
|
||||||
_state = WorkerState.Faulted;
|
_state = WorkerState.Faulted;
|
||||||
|
// WRK-34: CommandMethod is a method-name field, not a correlation-id field, so a
|
||||||
|
// consumer reading it (dashboard, gateway logs) shouldn't see a bare correlation id
|
||||||
|
// there. Use a stable label and carry the stuck command's correlation id (if any) in
|
||||||
|
// the diagnostic message instead.
|
||||||
|
string correlationSuffix = string.IsNullOrEmpty(snapshot.CurrentCommandCorrelationId)
|
||||||
|
? string.Empty
|
||||||
|
: $" Stuck command correlation id: {snapshot.CurrentCommandCorrelationId}.";
|
||||||
await TryWriteFaultAsync(
|
await TryWriteFaultAsync(
|
||||||
CreateFault(
|
CreateFault(
|
||||||
WorkerFaultCategory.StaHung,
|
WorkerFaultCategory.StaHung,
|
||||||
snapshot.CurrentCommandCorrelationId,
|
"Watchdog",
|
||||||
$"STA activity is stale by {staleFor}."),
|
$"STA activity is stale by {staleFor}.{correlationSuffix}"),
|
||||||
cancellationToken).ConfigureAwait(false);
|
cancellationToken).ConfigureAwait(false);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -445,7 +445,8 @@ public sealed class MxAccessCommandExecutor : IStaCommandExecutor
|
|||||||
// also fire-and-forget in MXAccess, and its commit outcome only exists
|
// also fire-and-forget in MXAccess, and its commit outcome only exists
|
||||||
// in the later OnWriteComplete callback.
|
// in the later OnWriteComplete callback.
|
||||||
MxAccessWriteCompletionCache completionCache = session.WriteCompletionCache;
|
MxAccessWriteCompletionCache completionCache = session.WriteCompletionCache;
|
||||||
ulong completionBaseline = completionCache.CurrentVersion(
|
ulong completionBaseline = CaptureCompletionBaseline(
|
||||||
|
completionCache,
|
||||||
writeCommand.ServerHandle,
|
writeCommand.ServerHandle,
|
||||||
writeCommand.ItemHandle);
|
writeCommand.ItemHandle);
|
||||||
|
|
||||||
@@ -485,7 +486,8 @@ public sealed class MxAccessCommandExecutor : IStaCommandExecutor
|
|||||||
|
|
||||||
// Same pre-call baseline rule as ExecuteWriteSecured.
|
// Same pre-call baseline rule as ExecuteWriteSecured.
|
||||||
MxAccessWriteCompletionCache completionCache = session.WriteCompletionCache;
|
MxAccessWriteCompletionCache completionCache = session.WriteCompletionCache;
|
||||||
ulong completionBaseline = completionCache.CurrentVersion(
|
ulong completionBaseline = CaptureCompletionBaseline(
|
||||||
|
completionCache,
|
||||||
write2Command.ServerHandle,
|
write2Command.ServerHandle,
|
||||||
write2Command.ItemHandle);
|
write2Command.ItemHandle);
|
||||||
|
|
||||||
@@ -523,7 +525,8 @@ public sealed class MxAccessCommandExecutor : IStaCommandExecutor
|
|||||||
// immediately after WriteSecured bumps the version past this snapshot,
|
// immediately after WriteSecured bumps the version past this snapshot,
|
||||||
// so a fast commit still correlates (no missed-callback window).
|
// so a fast commit still correlates (no missed-callback window).
|
||||||
MxAccessWriteCompletionCache completionCache = session.WriteCompletionCache;
|
MxAccessWriteCompletionCache completionCache = session.WriteCompletionCache;
|
||||||
ulong completionBaseline = completionCache.CurrentVersion(
|
ulong completionBaseline = CaptureCompletionBaseline(
|
||||||
|
completionCache,
|
||||||
writeSecuredCommand.ServerHandle,
|
writeSecuredCommand.ServerHandle,
|
||||||
writeSecuredCommand.ItemHandle);
|
writeSecuredCommand.ItemHandle);
|
||||||
|
|
||||||
@@ -564,7 +567,8 @@ public sealed class MxAccessCommandExecutor : IStaCommandExecutor
|
|||||||
|
|
||||||
// Same pre-call baseline rule as ExecuteWriteSecured.
|
// Same pre-call baseline rule as ExecuteWriteSecured.
|
||||||
MxAccessWriteCompletionCache completionCache = session.WriteCompletionCache;
|
MxAccessWriteCompletionCache completionCache = session.WriteCompletionCache;
|
||||||
ulong completionBaseline = completionCache.CurrentVersion(
|
ulong completionBaseline = CaptureCompletionBaseline(
|
||||||
|
completionCache,
|
||||||
writeSecured2Command.ServerHandle,
|
writeSecured2Command.ServerHandle,
|
||||||
writeSecured2Command.ItemHandle);
|
writeSecured2Command.ItemHandle);
|
||||||
|
|
||||||
@@ -994,7 +998,31 @@ public sealed class MxAccessCommandExecutor : IStaCommandExecutor
|
|||||||
}
|
}
|
||||||
|
|
||||||
/// <summary>
|
/// <summary>
|
||||||
/// Bounded pump-wait for the OnWriteComplete row matching a
|
/// Captures the write-completion baseline for <paramref name="serverHandle"/> /
|
||||||
|
/// <paramref name="itemHandle"/> immediately before a Write/Write2/WriteSecured/
|
||||||
|
/// WriteSecured2 COM call. Pumps once first (WRK-32) so an <c>OnWriteComplete</c>
|
||||||
|
/// already queued for this handle — most commonly the still-undelivered callback of an
|
||||||
|
/// earlier write to the same tag that this worker gave up waiting on — folds into the
|
||||||
|
/// baseline instead of being available to ride this write's reply.
|
||||||
|
/// </summary>
|
||||||
|
/// <remarks>
|
||||||
|
/// This narrows, but does not close, the correlation gap documented on
|
||||||
|
/// <see cref="AwaitWriteCompletion"/>: a completion that MXAccess queues after this pump
|
||||||
|
/// step but before the COM call returns is still indistinguishable from this write's own
|
||||||
|
/// completion, because <c>OnWriteComplete</c> carries no per-call token — only the
|
||||||
|
/// server/item handle pair.
|
||||||
|
/// </remarks>
|
||||||
|
private ulong CaptureCompletionBaseline(
|
||||||
|
MxAccessWriteCompletionCache completionCache,
|
||||||
|
int serverHandle,
|
||||||
|
int itemHandle)
|
||||||
|
{
|
||||||
|
pumpStep();
|
||||||
|
return completionCache.CurrentVersion(serverHandle, itemHandle);
|
||||||
|
}
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// Bounded pump-wait for the OnWriteComplete row matching a Write/Write2/
|
||||||
/// WriteSecured/WriteSecured2 call, copied onto the reply when it
|
/// WriteSecured/WriteSecured2 call, copied onto the reply when it
|
||||||
/// arrives in time. The executor holds the STA thread but pumps
|
/// arrives in time. The executor holds the STA thread but pumps
|
||||||
/// Windows messages each poll (ReadBulk precedent) so the COM callback
|
/// Windows messages each poll (ReadBulk precedent) so the COM callback
|
||||||
@@ -1002,6 +1030,21 @@ public sealed class MxAccessCommandExecutor : IStaCommandExecutor
|
|||||||
/// statuses — the consumer's unconfirmed path, never a synthesized
|
/// statuses — the consumer's unconfirmed path, never a synthesized
|
||||||
/// failure. Protocol status/hresult stay acceptance-only either way.
|
/// failure. Protocol status/hresult stay acceptance-only either way.
|
||||||
/// </summary>
|
/// </summary>
|
||||||
|
/// <remarks>
|
||||||
|
/// Correlation is recency-based per handle, not per call: MXAccess's
|
||||||
|
/// <c>OnWriteComplete</c> callback carries no correlation token, only the server/item
|
||||||
|
/// handle pair, so this method reports the first completion whose version exceeds
|
||||||
|
/// <paramref name="completionBaseline"/> as this write's outcome. If an earlier write to
|
||||||
|
/// the same handle times out unconfirmed and its callback is still undelivered, that
|
||||||
|
/// stale completion can dispatch during this write's pump-wait and ride this reply
|
||||||
|
/// instead — <see cref="CaptureCompletionBaseline"/>'s pre-call pump only folds in a
|
||||||
|
/// completion already queued before this write started; it cannot fold in one queued
|
||||||
|
/// during the COM call or the wait itself. The <c>Hresult</c>/<c>ProtocolStatus</c>
|
||||||
|
/// fields stay acceptance-only regardless, so the exposure is limited to the enrichment
|
||||||
|
/// <c>statuses</c> field, and it is inherent to the COM surface — a native client
|
||||||
|
/// correlating by handle has the identical ambiguity. Full closure needs an
|
||||||
|
/// MXAccess-side correlation token, which does not exist.
|
||||||
|
/// </remarks>
|
||||||
private void AwaitWriteCompletion(
|
private void AwaitWriteCompletion(
|
||||||
MxCommandReply reply,
|
MxCommandReply reply,
|
||||||
MxAccessWriteCompletionCache completionCache,
|
MxAccessWriteCompletionCache completionCache,
|
||||||
|
|||||||
Reference in New Issue
Block a user