feat: rule lifecycle, three severity corrections, and the probe's own conformance (WP-14) - #15
Merged
Merged
Conversation
Extends SN-HND-001 from the MVP tier to v1: three artifacts a company can
take -- the sentinel scanner, an extracted Go server kit, and a fleet service.
Section 3 exists because applying this repository's own precedence rule --
where the spec and the repo disagree, the spec wins -- found four places where
the repo is currently wrong, each verified against a live fetch rather than
inferred:
* broker's error codes sit in -32000..-32019, which the revision retired
("new implementations SHOULD NOT use codes from this sub-range at all").
CLAUDE.md and HANDOFF 7.2 both mandate it. They are wrong.
* MUST/server-info-echoed and MUST/tools-list-is-deterministic grade SHOULDs
as MUSTs, and SHOULD/tools-sorted-by-name demands something the spec never
asks for at any severity. That is the false-positive class MEASUREMENTS.md
promises is zero.
* the probe never sends clientCapabilities, which is Required: Yes, so every
request the harness sends is malformed.
* the deprecation inventory stamps all six features 2026-07-28; the registry
says includeContext is 2025-11-25 and HTTP+SSE is 2025-03-26, and neither
removal window is expressible as +12 months.
Rule ids are permanent, so the severity corrections deprecate and supersede
rather than edit -- which requires a rule lifecycle the catalog does not have
yet, and gets one in WP-14.
The differentiator is gray-box evidence: the README already names what would
settle each of the five unverifiable MUSTs, and v1 accepts it. Absence of
evidence still returns INDETERMINATE and never a pass.
16 work packages, WP-14 through WP-29.
The gap is that no rule reads them, not that they are absent.
17 tasks across five work packages: the four verified spec debts, then five new rule modules taking the catalog from 35 rules to roughly 74. Three rules were corrected while writing the plan, against spec text fetched rather than remembered: the MRTR new-id requirement is a client obligation and has no server rule; the notification 202 is conditional and demanding it outright would be a false positive; and GET/DELETE 405 is a SHOULD. The Origin rule uses an RFC 2606 .invalid origin so that a served request is evidence of no validation rather than of a permissive allowlist.
…editing a published id Rule ids are permanent (HANDOFF 8.8): once published, an id never changes meaning, or every historical report becomes uninterpretable. That leaves no way to fix a rule whose severity turns out to be wrong -- which three of them are. BaseRule gains introduced_in, deprecated_in and superseded_by, and Registry.all excludes deprecated rules unless asked. validate_registry now requires a deprecated rule to name a successor that exists, forbids two LIVE rules from sharing a slug, and explicitly permits a deprecated rule and its successor to share one -- which is the whole point of the mechanism. Also introduces the SENTINEL/ namespace for rules this project believes in that the specification does not require. They carry a rationale instead of a citation, because a citation field pointing at nothing is how a catalog starts lying.
Three omissions, each grounds for a conformant server to reject every request
the harness sends:
* clientCapabilities is Required: Yes on every request. client.py imported
the key and never set it.
* "Every POST request to the MCP endpoint MUST include an
MCP-Protocol-Version header." The probe sent Mcp-Method and Mcp-Name and
nothing else, so a server enforcing this would answer 400 HeaderMismatch
to everything -- and the harness would report it as broken.
* "The client MUST include an Accept header listing both application/json
and text/event-stream."
The header value is read back out of meta(version=version) rather than from
self.protocol_version, so header and body come from one source and cannot
disagree by accident; when the caller asks for an unversioned body the header
is omitted rather than manufactured, which is exactly the mismatch the header
rule exists to detect.
Literal is the point; wrong is not. Every override is preserved so a rule can
still break one field deliberately.
…sible one The registry says includeContext was deprecated 2025-11-25 and HTTP+SSE 2025-03-26; the inventory stamped all six 2026-07-28 and computed +12 months from it. Worse, two removal windows are not date arithmetic at all. includeContext's "Follows Sampling (SEP-2577)", and HTTP+SSE's is "three months after SEP-2596 reaches Final" -- an event that has not happened. Computing twelve months from includeContext's own deprecation date would have produced 2026-11-25, eight months early. The window is now one of three variants. FollowsFeature chases the chain, so includeContext inherits Sampling's 2027-07-28. AfterEvent is a dead end by construction -- it holds a condition and a SEP and nothing else, and months_remaining returns None for it -- so no code path can turn it into a number. A report that prints a date it cannot know is worse than one that prints the condition, because the number gets put in a plan and treated as a deadline. The six registry rows are checked in verbatim and a test compares against them, so the next registry change fails a test instead of drifting silently.
…ace can fail A SENTINEL/ rule is an opinion this project holds and the specification does not. Letting one fail --gate must would make a conformance verdict unfalsifiable, so gate() filters to Namespace.MCP before deciding the exit code. The text report marks a deprecated finding and names its successor; the JSON report gains namespace, deprecated and supersededBy per finding. schemaVersion stays at 1 -- adding fields is additive, and the v2 bump is WP-24.
serverInfo is "Required: No" under "Servers SHOULD include the following field in every result's _meta". Tool ordering is "Servers SHOULD return tools in a deterministic order". And the spec asks for deterministic, never for sorted -- a stable but unsorted manifest conforms fully, so tools-sorted-by-name was not a spec rule at any severity and moves to the beyond-spec namespace. That is the false-positive class MEASUREMENTS.md publishes as zero. The measurement was honest; the conformant fixture just happens to satisfy all three, so none of them ever fired. Ids are permanent, so all three are deprecated and superseded rather than edited, and --include-deprecated-rules reproduces an archived report. The three check bodies move verbatim into catalog/checks.py so a deprecated rule and its successor cannot drift apart -- which would make the deprecation notice a lie. Live catalog: 35 rules, 31 MUST + 4 SHOULD.
…SARIF report/sarif.py emitted "helpUri": "" and a help text ending in a bare "Specification: " with nothing after it for a SENTINEL/ rule -- a citation pointing at nothing, in the one report format a reviewer reads inside GitHub. That is precisely the failure the namespace was introduced to make impossible, so it must not be reintroduced by the renderer. A beyond-spec rule now omits helpUri entirely and carries its rationale instead, with the namespace and lifecycle in properties. Also: `sentinel catalog validate` counted deprecated rules in its headline, so it reported 38 where a scan runs 35. The headline is now the live count and the deprecated rules get their own line naming each successor. And two assertions predated the namespace: severity-in-the-id and citation-as-helpUri are MCP-namespace properties, and validate_registry already scopes both. The tests now do too, plus a new one asserting the beyond-spec case positively rather than merely exempting it.
"This set ... MUST NOT vary per-connection or as a side effect of other requests on the connection." Proving that needs two connections; the deprecated rule made twenty calls on one, so a server returning a stable list per connection and a different stable list on the next passed it. Compares membership, not order, and holds the credential constant -- the spec permits the set to vary by authorization, and order is graded SHOULD by the successor rule, so comparing either would make one defect fail two rules.
The non-conformant fixture now returns a different SET of tools per connection, not merely a different order, so MUST/tools-list-connection-independent has something to catch. SEEDED_VIOLATIONS names the three successors instead of the deprecated ids. Two things the reconciliation turned up that were not in the plan: scripts/measure.py counted only MUST failures as detections, so the three non-MUST seeds would have been scored as misses and published as a recall regression. Recall is now computed per namespace/severity bucket. MUST recall stays 100% (25/25); all 28 seeded violations are detected. The README's broker scan example omitted --token. resources/read is authenticated, so the command as printed exits 1 on resource-not-found-is- invalid-params -- CI has always passed a token. The example now does too, and says why. runbook.md, demo/README.md and scripts/demo.py each hard-coded "26 MUST failures", and MIGRATION.md's deterministic-ordering entry named two deprecated ids and recommended calling tools/list a hundred times -- which is precisely the test that cannot see the per-connection MUST.
This was referenced Aug 24, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements §3.2, §3.3, §3.4 and §4.1 of
docs/superpowers/specs/2026-08-23-sentinel-v1-design.md.Applying this repository's own precedence rule — where the spec and the repo disagree, the spec wins — found four places where the repo was wrong. Each was verified against a live fetch of the specification rather than inferred.
The probe was sending malformed requests
Three omissions, each grounds for a conformant server to reject every request the harness sends:
io.modelcontextprotocol/clientCapabilitiesis Required: Yes.client.pyimported the key and never set it.MCP-Protocol-Versionheader." The probe sentMcp-MethodandMcp-Nameand nothing else. A server enforcing this answers400 HeaderMismatchto everything — and the harness would have reported it as catastrophically broken.Acceptheader listing bothapplication/jsonandtext/event-stream."This survived only because neither fixture enforces any of the three. The header value is now derived from the same source as the body's
_meta, so they cannot disagree by accident.Three rules demanded more than the specification does
That is the false-positive class
MEASUREMENTS.mdpublishes as zero. The measurement was honest; the conformant fixture just happens to satisfy all three.MUST/server-info-echoedSHOULD/server-info-echoedserverInfois "Required: No" under "Servers SHOULD include…"MUST/tools-list-is-deterministicSHOULD/tools-list-is-deterministicSHOULD/tools-sorted-by-nameSENTINEL/STYLE/tools-sorted-by-nameThe MUST in that same paragraph is a different property — "MUST NOT vary per-connection" — which the deprecated rule never tested, because all twenty of its calls shared one connection.
MUST/tools-list-connection-independentopens a second one.Rule IDs are permanent (HANDOFF §8.8), so all three are deprecated and superseded rather than edited. That required a lifecycle the catalog did not have, plus the
SENTINEL/namespace for rules this project believes in that the spec does not require — they carry arationaleinstead of acitation, and no spec gate can ever fail on one.The deprecation inventory reported two wrong dates and one impossible one
The registry says
includeContextwas deprecated2025-11-25and HTTP+SSE2025-03-26; the inventory stamped all six2026-07-28. Worse, two removal windows are not date arithmetic at all:includeContext's "Follows Sampling", and HTTP+SSE's is "Three months after SEP-2596 reaches Final" — an event that has not happened. Computing +12 months fromincludeContext's own date would have produced2026-11-25, eight months early.AfterEventis now a dead end by construction — it holds a condition and a SEP and nothing else — so no code path can turn it into a number. A report that prints a date it cannot know is worse than one that prints the condition, because the number gets put in a plan and treated as a deadline.Found while reconciling
report/sarif.pyemitted"helpUri": ""and a bare"Specification: "for a beyond-spec rule — a citation pointing at nothing, in the one report format a reviewer reads inside GitHub.scripts/measure.pycounted only MUST failures as detections, so the non-MUST seeds would have been published as a recall regression.--token, so the command as printed exits 1. CI has always passed one.Registry.__len__counted deprecated rules whileall()excluded them, socatalog validatereported 38 where a scan runs 35.Verification
Test plan
make checkgreenmake upstack (thestripe/smokescreenimage failed to pull locally; postgres/broker/envoy were up and current)