test(hardening): establish extension fault containment matrix - #580
Conversation
ac06a84 to
7c4f8ab
Compare
SunSi12138
left a comment
There was a problem hiding this comment.
整体方向正确:tracker 保持 test/inventory-only,矩阵走真实 production paths,并且当前 head 的 Extension fault containment / PR Fast / Pending+codec / CodeQL / PR Extended 都是 green。我有 3 个需要在把 #576 视为可关闭前补齐的点:
-
永久 gate 的 path filter 漏掉 telemetry containment helper。 workflow 监听
SharpLinkTelemetry.cs,但 #581 的 observer isolation 实际已经拆到src/SharpLink.Abstractions/SharpLinkTelemetry.ObserverIsolation.cs。以后如果只改这个 helper,Extension fault containment不会触发,正好绕过新加的 telemetry regression gate。请至少把该文件纳入 paths;更稳妥的是核对所有被这套 P0 matrix 覆盖的 lifecycle implementation 文件,避免类似漏触发。 -
当前 telemetry P0 行只覆盖 start-phase observer fault,不覆盖 completion-phase observer fault。
MeterListenerFaultShouldNotReplaceBusinessResultOrPoisonReuse注入的是sharplink.calls.started,Activity 行注入的是 sampler;这不能证明一个已经产生 authoritative RPC result 后,calls.completed/calls.failed或ActivityStopped抛异常不会替换结果或截断剩余 accounting。#581 本身明确包含 completion path 风险,而且 #576 policy 要求发现的 child bug 修复后把 regression 纳回 tracker matrix。建议把 completion meter/activity fault 各纳入ExtensionFaultContainmentTests,或让这个永久 workflow 明确运行等价的 child regression tests。 -
Closes #576目前早于 issue 的关闭条件。 #576 明确要求 lifecycle-sensitive extension points 有 deterministic fault-vs-cancel/deadline/stop coverage,并要求允许重入的关键 callback 有 targeted reentrancy/deadlock coverage。当前 inventory 虽然引用了若干既有 race/abandonment tests,但没有把这些两项关闭条件逐项映射出来,新的 P0 suite 也没有 cancellation/reentrancy cases。要么补齐/明确引用能满足这些条件的现有测试并说明覆盖关系,要么暂时不要让 #580 自动关闭 #576,等后续矩阵完成再收口。
除此之外,没有发现当前新增 fixtures/tests 在 interceptor、admission、retry、codec、stream、service factory/disposal、reuse proof 上的明显 lifecycle/correctness 问题。
11619e0 to
3adcdf2
Compare
7c4f8ab to
6de81f5
Compare
e036588 to
3adcdf2
Compare
a5f31b0 to
8206633
Compare
bab63c1 to
7aaa021
Compare
SunSi12138
left a comment
There was a problem hiding this comment.
第二轮复审:上一轮 3 个 finding 都已经实质处理到位。helper-only telemetry 触发、completion-side observer faults,以及 fault-vs-cancel/deadline/stop + reentrancy 都有对应的永久测试/owner inventory;当前 head 的 Extension matrix、Fast、Pending+codec、CodeQL、Extended 也有成功记录。
当前还有两个需要在最终 merge 前收口的点:
-
#584 squash 后请把 #580 真正 restack 到新的
dev。 #584 已 squash merge 为f643aee53e411fa2a6825cf484787069468f9c64,但 #580 head7aaa021...仍保留 pre-squash #584 ancestry。现在 PR 元数据因此变成 4 commits / 9 files,GitHub compare 的 merge-base 仍是旧d328ff5...,Files changed 又把已经合入 dev 的 4 个 telemetry files 展示出来。已核对SharpLinkTelemetry.cs和SharpLinkTelemetry.ObserverIsolation.cs在dev@f643...与当前 head 的 blob SHA 完全相同,所以这不是新的 product diff,而是 ancestry/audit noise。请从dev@f643...重新放置 tracker-only commit,使 PR 回到真正的 1 commit / 5 tracker files;新 head 再跑 exact-head gates。 -
永久 workflow 的 production path filter 还漏了直接 owner。 具体见 inline:至少
SharpLinkClient.Telemetry.cs与PendingRequestTable.cs应触发这套 matrix,前者直接拥有StartClientCall -> scope.Complete/stream telemetry unwind,后者直接拥有AddPendingRequests(+1/-1)与 producer-cancellation callback 生命周期;只改这些文件时当前Extension fault containment不会运行。
除此之外,目前没有新的 code/test blocker:P0 owner/reuse、dual MoveNext+Dispose failure、cancel/deadline/stop terminal precedence、interceptor/metric re-entry,以及 #81/#86 cross-check 的实现方向都通过复审。
SunSi12138
left a comment
There was a problem hiding this comment.
继续复审:当前 head 7aaa0213606d7c35e1f859a06dbc54150a57a7a7 没有吸收上一轮两个 merge blocker,因此结论不变,且 stale-base 风险现在更大。
-
请先真正 restack 到当前
dev。PR 仍是 4 commits / 9 files,compare merge-base 仍落在旧d328ff5...,已合入 dev 的 #584 telemetry 4 files 仍重复出现在 Files changed;当前dev已前进到fd8832ce7c4b66dee21b2e14ffb7400f4ebab21a。自 PR basef643aee...之后,dev 又合入了 runtime request-timeout 和 runtime retry-policy generation,两者直接改变 #576 matrix 中 deadline/retry extension lifecycle 的生产路径,所以旧 base 上的 Extension/Fast/Extended 绿灯不能作为最终关闭 #576 的证据。请将 tracker-only commit 重放到最新 dev,使 diff 回到预期的 workflow/doc/3 个 tracker test files,并在新 head 上重跑 gates。 -
permanent workflow path filter 仍缺
src/SharpLink.Client/SharpLinkClient.Telemetry.cs和src/SharpLink.Client/PendingRequestTable.cs。前者拥有 call/stream telemetry terminal unwind,后者拥有 pending registration/release 与 producer-cancellation observer;只改这些直接 owner 时当前 matrix 仍不会触发。上一轮 inline threadPRRT_kwDOROK3ys6f8Q9P仍 unresolved。
矩阵测试本身(P0 owner/reuse、MoveNext+Dispose secondary fault、cancel/deadline/stop precedence、interceptor/metric re-entry、#81/#86 cross-check)本轮没有发现新的 correctness 问题。
7aaa021 to
25767ce
Compare
25767ce to
c269d53
Compare
c269d53 to
37b8a49
Compare
SunSi12138
left a comment
There was a problem hiding this comment.
最终复审通过,未发现新的 code/test blocker。
已重新核对当前最终栈:dev@3bccdefd75fbb1f7e04ad6816f615d198004230e,head 37b8a4984bb9432860f8edc5d6f115e7b644ac75,PR 现在确实是 1 commit / 5 个 tracker/test/workflow 文件,没有 #584 或其它 production ancestry/diff。
上一轮 permanent-gate finding 也已正确收口:extension-fault-validation.yml 现在监听 PendingRequestTable.cs 以及 SharpLinkClient*.cs,覆盖 SharpLinkClient.Telemetry.cs 和后续拆出的 runtime timeout/retry/admission/circuit-breaker/heartbeat lifecycle partial owners,同时保留 telemetry observer-isolation helper 触发。
Exact-head evidence 已实际核对而非只看 PR 描述:Extension fault containment run 34177580354 checkout 37b8a498...,P0 13/13、authoritative completion telemetry 3/3、lifecycle/reentrancy 5/5 全绿,artifact 10037790253 digest 51eb5397fded74a97baf2500822b9fcf41d66d13af0d0c4985b991318bab2db7;同 head PR Fast、CodeQL、PR Extended 也均成功。
因此当前 #580 从 code review 角度可以进入最终合并流程。由于 PR 作者/审核身份相同,这里只记录 COMMENT,不自我 APPROVE;本次也不执行合并或关闭 #576。
Closes #576
Scope
Completes the release-hardening inventory and deterministic fault-containment matrix for application-owned extension code. This PR is tracker/test/workflow-only; the telemetry production fix discovered by the matrix was already merged separately in #584.
Review follow-up
dev. The old pre-squash fix(telemetry): isolate external observer failures #584 ancestry and already-merged telemetry product files are gone from the PR diff. During reviewdevcontinued through feat(client): update request timeout policy at runtime #593/feat(client): update retry policy at runtime #594/feat(client): reconfigure endpoint circuit breaker at runtime #597/feat(client): update heartbeat configuration at runtime #598, so the tracker was repeatedly rebuilt rather than treating stale-base CI as final evidence.extension-fault-validation.ymlnow watches:src/SharpLink.Client/PendingRequestTable.cssrc/SharpLink.Client/SharpLinkClient*.csThis includes the requested
SharpLinkClient.Telemetry.csboundary and the runtime timeout/retry/admission/circuit-breaker/heartbeat partial owners that landed while review was in progress, so implementation-only lifecycle changes cannot silently bypass the matrix.SharpLinkTelemetry.ObserverIsolation.cs), and P0 still includes authoritative-completion observer failures (calls.completed,calls.failed,ActivityStopped).Hard gates
Inventory / release cross-check
doc/extension-fault-containment.mdrecords invocation phase, sync/async behavior, already-owned resources, expected failure surface, terminal owner, cleanup owner, and exact test coverage. It cross-checks #81 single-owner/single-terminal invariants and #86 release validation; the focused matrix does not replace PR Extended.Final stack
Base
dev:3bccdefd75fbb1f7e04ad6816f615d198004230e(#598 runtime heartbeat configuration).Final clean head:
37b8a4984bb9432860f8edc5d6f115e7b644ac75.The PR is exactly one commit / five tracker files on top of current
dev; no merged telemetry or other product files remain in the diff.Final-head validation
34177580354: PASS10037790253, digestsha256:51eb5397fded74a97baf2500822b9fcf41d66d13af0d0c4985b991318bab2db737b8a4984bb9432860f8edc5d6f115e7b644ac75summary.json:invariant=true, tiersp0+p1-targeted34177580396: PASS — allocation, formatting, maintainability, Release build, Unit/Generator/Load tests34177580409: PASS34177789111: PASS — Debug/Release builds, admission rollback, full Integration Tests, NativeAOT transport/topology smoke, pack, NuGet contract verification/package smoke, Demo Oneway, Load Smoke, and desktop cross-platform codec compatibility