feat(monitoring): register a monitored kind as data, not as four hardcodes

[chenmingwei23]

Problem / Motivation

A monitored kind was spelled out wherever it mattered, and adding a kind meant
finding every place. There are four such places, and they are not the same
shape. The distinction matters, because the next person will otherwise find the
fourth one and think it was missed.

Three are entry allowlists -- they decide what a caller may ask for:

1. mcp_tools/control.py, a JSON-Schema enum, in two tools (monitor_watch and
   monitor_update)
2. validation.py, a FieldSpec(allowed=frozenset(...)), in two schemas
3. dashboard/handlers/autonudge.py::_monitor_config, an if guard

The fourth is not an allowlist. monitoring/shadow.py refused anything but one
kind because the persistence-only probe path is only implemented for that kind.
That is a statement about what code exists, not about what a caller may request, and
the two must not be modelled the same way.

There is also a hole underneath all four: MonitorState itself validates nothing
about kind. It accepts "gh-pr", "calendar", any non-empty string. The
allowlists are the only thing constraining it, so a path that bypasses them
persists a monitor under a kind nothing implements.

Why it matters

Being precise about provenance, because the weaker version of this argument is the
one worth refusing: docs/request-for-change/rfc-consolidated-monitor.md (status:
draft) does NOT contain the word registry, or kind. What its item 5 specifies is
exposing the probe contract as an SDK -- implement identity() and observe(),
inherit scheduling, dedupe, coalescing, epoch reset and failure handling. An SDK
with pluggable implementations needs some answer to which implementations exist,
but the RFC does not name a registry, and the module spec that does is in flight in
#9368 rather than on main.

So the direct case is the smaller one, and it stands on its own: four hardcodes in
four files is a dispatch branch spread across a codebase, and consolidating them
removes four places to forget. A shared objective vocabulary would have the same
defect one level down -- review_ready is not a sentence about a calendar or a
ticket, so a shared list would make every new kind edit a common set.

This is also the change that tests whether the substrate premise is true, so the
finding is worth stating plainly: it holds. monitoring/decision.py contains
zero references to kind or objective, so registering a kind as data forces no
change to the decision engine. MonitorProbe and MonitorProbeResult are unchanged.
Nothing here adds a branch, a special case, or a per-kind hook to a shared layer.

What changed

monitoring/registry.py holds one MonitorKind entry per kind: its name, the
objectives that kind declares, whether a caller may name it, and whether the
persistence-only path is implemented for it.

The three entry allowlists now derive from the registry. shadow.py asks whether
the kind declares the capability rather than matching a literal -- modelled as an
allowlist the registry polices, a kind added later would silently inherit a claim
about a path it has never run.

Two kinds are registered, because MonitorState.kind already carries two
vocabularies. github_pull_request is armed through monitor_watch, is publicly
armable, and has the shadow path implemented. gh-pr is armed only internally, by
infer_monitor from a loop's own message text via probes.targets.infer, so it is
registered without being requestable. Keeping publicly_armable separate from
supports_shadow is what lets the entry allowlists stay exactly as narrow as they
were while the registry still tells the truth about what exists.

Both mutation boundaries now refuse a kind/objective pair no kind declares.
add_monitor is the one place a caller-supplied kind reaches persistence;
update_monitor is the one place an objective can be repointed afterwards. The flat
objective allowlist both sit behind is a UNION across every publicly armable kind,
so it admits an objective a different kind declares and structurally cannot
express the pairing -- it is a first filter, never the whole check. Both boundaries
know the kind, so both check.

Deserialization is deliberately untouched: refusing there would quarantine existing
records rather than reject new ones, which reaches further than this change should.

Two refusal messages changed wording: the dashboard guard's, and shadow.py's --
where one message became two, separating a kind whose shadow path is unimplemented
from an objective the kind does not declare. Those are the caller-visible
differences in the diff.

probes/__init__.py is deliberately left alone. Its if keys on the gh-pr
vocabulary, which has no objective concept, and it is the irq subsystem's own
deferral -- a future consumer of this registry rather than its subject.

Tests

test/test_monitor_kind_registry.py is new. The load-bearing ones are the tests
that would fail if the design collapsed back:

• the derived allowlists resolve to exactly the values the hardcodes carried, kinds
  {github_pull_request} and objectives {review_ready}, asserted against both the
  registry and MONITOR_WATCH_SCHEMA
• a new kind declaring a new objective does not widen another kind's set, and an
  objective one kind declares is not thereby legal for another -- this is the test
  that fails if objectives ever move into one shared enum
• supports_shadow is a declared capability: the kind that implements it says so, a
  registered kind without it says false, and an unregistered kind says false rather
  than inheriting a claim
• publicly_armable and supports_shadow are independent, since they answer
  different questions
• the internally-armed kind is registered, is not publicly namable, and still
  supports its objective, so registered-not-requestable does not mean unusable
• update_monitor refuses an objective the monitor's own kind does not declare, and
  the refusal does not half-apply
• shadow refuses a registered kind that lacks the capability, with a message naming
  the missing capability rather than an unknown kind
• arming refuses an unregistered kind before persistence, with the test first
  showing that MonitorState itself still accepts it
• malformed entries are refused: no name, no objectives, a mutable set, an empty
  objective, a non-boolean flag

Review rounds

Both design lanes are advisory, and neither has ever produced a structured finding,
so no span exists to claim and no disposition comment is warranted. Recording the
outcomes here instead. Design Review is now PASS.

Round 1, accepted. update_monitor persisted an objective with only the flat
union as a filter, behind a schema enum that now derives from the cross-kind union.
Today the union is {review_ready} so nothing changes, but the body already claimed
pairing was enforced at the boundary that knows both, which was false for exactly
that one. Fixed with a test driving update_monitor directly. Also: is_registered_kind
had zero consumers and is deleted; the isinstance guards duplicated the mypy gate;
and the provenance overclaim was corrected.

Round 2, accepted. MonitorKind.__post_init__ is gone. Nothing constructs a
MonitorKind outside this module's own static literal, so runtime validation only
ever checked a literal -- the tests moved to iterate _KINDS instead, which covers
the same ground and also covers an entry added later, with no shipped code. The
docstring claim is scoped to the four converted boundaries, since other spellings do
survive elsewhere. And infer_monitor now uses REVIEW_READY rather than repeating
the literal, since that file already imports the registry.

Round 2, one finding answered a different way than proposed. GH_PR = "gh-pr" is
genuinely defined twice -- here and at probes/__init__.py:25, whose targets.infer
produces the spelling this registry must recognise. The proposed fix was to have
probes/ import the registry's constant. Measured before deciding: the two packages
have no import edge in either direction today, so that would create the first one
between them, in the subsystem whose registry shape is deliberately deferred. The
reverse is worse -- validation.py imports this registry and is imported almost
everywhere, so pulling probes in would drag the irq subsystem behind every
validator.

So the duplication stays and the drift is closed instead: a test reads the kind
targets.infer actually produces and asserts this registry knows it, plus a direct
equality on the two constants. Verified non-vacuous by perturbing this module's
spelling, which fails with "the two spellings have drifted apart". That turns "must
silently stay in sync" into "cannot drift undetected" without coupling two packages
ahead of the work that unifies them.

Held. monitoring/github_pull_request.py:688 returns "review_ready" as a
reason code in an observation, not as an objective. It is the same string for a
different concept, and replacing it with REVIEW_READY would assert a coupling that
does not exist.

On the repo's own stance. probes/__init__.py:35-38 records that "the shape of a
registry is best decided by the second probe's real needs rather than guessed before
it exists," which is the opposite stance for the adjacent layer, and it is right. It
is about the shape of a probe registry -- what an implementation must provide --
and this module deliberately holds none of that: no probe factories, no identity()
or observe(), no scheduling contract. It consolidates four allowlists that already
exist and already all say the same thing. That is why probes/__init__.py is
untouched here and remains free to decide its own shape when its second probe lands.

A green board here does NOT mean the code is verified

This is stacked on feat/monitor-plural-probe, so it runs the smaller check set: 18
checks against the 63 that run on a main-based pull request. The absent ones are
every test job and every deterministic gate -- Backend, Frontend, Gateway and
Electron tests, the builds, E2E, Coverage, Backend Lint & Type Check, CodeQL, Docs
Lint, Brand Name, Feature Map, Focus Cue, Harness Parity, Testpaths Coverage,
Vendored Tree Integrity. A green board here means the review lanes are satisfied and
says nothing about whether the code works, and PR Readiness here aggregates a set
that excludes the tests.

Merge order. This is the third of three stacked changes. Each base merges before
the one above it retargets to main, and the full 63 checks run on this only after
both bases have landed. Do not merge on the stacked green.

Manual verification

CI runs no tests on a stacked PR, so this is the only test evidence, and its gaps are
stated rather than implied.

Run and passing after the latest change: test_monitor_kind_registry.py (25),
test_autonudge.py + test_monitor_persistence.py (213), and
test_monitor_behaviour_golden.py + test_github_pull_request_monitor.py +
test_monitor_controller.py + test_monitor_decision.py (240). GOLDEN_DIGEST and
all 28 per-group digests are unmoved.

Run and passing on the previous revision but NOT re-run on this one:
test_monitor_mcp.py, test_validation.py, test_monitor_directive_apply.py,
test_monitor_start_ack.py, test_autonudge_handlers_cov80.py (240 together). A
local sandbox policy began refusing those invocations partway through the round. The
reason they are low risk is specific rather than general: this revision's only
source changes outside the registry module are a comment, the removal of validation
whose sole subject was this module's static literal, and one literal replaced by the
constant of identical value. None of the five suites touches any of that. It is still
a gap, and the deterministic gates below are unaffected by it.

Gates run locally, all passing: black (the repository's own diff-scoped gate), isort,
flake8, mypy over monitoring/, autonudge.py, validation.py,
mcp_tools/control.py and the dashboard handler in one invocation, plus
comment-history, sync-IO, subprocess-encoding, brand-name, feature-map, focus-cue,
changelog-history, harness-parity, loop-bound-locks, builtin-skill-scope,
testpaths-coverage, vendored-manifest and docs-lint. NOT run locally: the
repository-wide mypy src/kiro_crew/, the frontend gates (no frontend changes) and
cfn-lint (no templates changed).

Import direction was measured, not assumed: validation.py already imported
monitoring.models; this registry imports nothing from kiro_crew; nothing in
monitoring/ or probes/ imports validation; and monitoring/ and probes/ do
not import each other in either direction. No cycle, and no new edge between
packages.

Related Issues

no linked issue: this is the third of three sequenced foundation changes for the
monitor substrate, and it closes no tracked defect on its own. The omission is
deliberate.

Checklist

• At most two commits (one is the norm), with a Conventional Commits title
• Existing tests pass and new tests added for new functionality
• Self-review completed; code follows project style guidelines
• Documentation updated (if applicable) -- N/A, the spec lands separately
• No secrets, credentials, or internal references in the diff

[github-actions]

Design Review (Fable 5) — ✅ PASS

Design-level review of 2c085ed59f30a0aed9a9b748c1f3c89f145d177f — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

Design-Verdict: PASS

A behavior-preserving consolidation that closes a real persistence hole; alternatives were measured (import edges, shared enums, the probes registry deferral), not assumed.

The derived allowlists are pinned equal to the old hardcodes, the allowlist/capability distinction in shadow.py is the right modelling ("a kind added later would silently inherit a claim about a path it has never run"), and the new kind_supports_objective refusals at add_monitor/update_monitor close the gap the flat union structurally cannot express — at the only two boundaries that know both halves. The gh-pr duplication-with-drift-test is a sound trade over creating the first import edge between monitoring/ and probes/ ahead of the work that unifies them. The stated verification gaps and the do-not-merge-on-stacked-green constraint are already explicit in the description; honoring the merge order is the one thing the human merging must do.

[DESIGN-REVIEWED] 2c085ed

[github-actions]

Opus 4.8 Review — ✅ no blocking findings

Reviewed 2c085ed59f30a0aed9a9b748c1f3c89f145d177f — this comment is updated in place on each push.

Review details

No findings.

[OPUS-REVIEWED] 2c085ed

Verdict parsed from the review's SHA-scoped output markers for commit 2c085ed59f30a0aed9a9b748c1f3c89f145d177f.

False positive or not applicable? A repository writer can comment:
/ai-review override fable 2c085ed59f30a0aed9a9b748c1f3c89f145d177f: <one-sentence reason>

[github-actions]

First Principles Review (Fable 5) — ✅ PASS

Premise-level review of 2c085ed59f30a0aed9a9b748c1f3c89f145d177f — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

First-Principles-Verdict: PASS

Registry covers only the four allowlists: arming still hardcodes GitHub at parse_github_pull_request_target (dashboard handler, control.py) and controller.py:104's sole provider — confirm #9368 owns those three.

What this change ships

Inventory (8 items) — 8 justified

Intent: make the monitored-kind vocabulary one data table so the four per-kind gates stop being four places to forget — an ADDITION (consolidation plus boundary hardening), honestly framed.

1. A monitored kind is now one registry entry; MCP enums, validator sets and the dashboard guard derive from it (4 duplicate spellings counted and removed) — justified
2. Arming a kind/objective pair no kind declares is refused before persistence — justified (controller.py:104 probes any kind with the GitHub provider; MonitorState validates nothing)
3. Repointing a monitor's objective is refused unless its own kind declares it — justified
4. Shadow's single refusal split into two messages: unimplemented kind vs undeclared objective (grepped: no test pinned the old wording) — justified
5. Dashboard arm-refusal wording changed — justified
6. gh-pr registered as a second, not-publicly-armable kind — justified (runtime consumer counted: update_monitor reaches inferred loops, so omission would regress patching them)
7. GH_PR spelled in two modules with drift pinned by test — justified (import-edge refusal litigated in round 2)
8. infer_monitor now uses the REVIEW_READY constant — justified

[FIRST-PRINCIPLES-REVIEWED] 2c085ed

[github-actions]

GPT 5.6 Review — ✅ no blocking findings

GPT 5.6 completed its review of 2c085ed59f30a0aed9a9b748c1f3c89f145d177f and found no blocking issues.

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] 2c085ed

False positive or not applicable? A repository writer can comment:
/ai-review override gpt 2c085ed59f30a0aed9a9b748c1f3c89f145d177f: <one-sentence reason>