# Plan Review Panel — alert-console-redesign

**Plan**: `.claude/plans/alert-console-redesign.plan.md` · **Plan version**: `sha256:fc7c3f8749da88434753cd1651ddf513f75af822b85f940b57fc7c61541a8a83`
**Verdict**: `unavailable` via `multi-agent`
**Quorum**: 4/2 responses · 4 distinct roles (of 4 fielded) · passed=false
**Layers**: L1 converged · L2 divergent · L3 not fired
**Halted at**: `5.2e`

> Reason: plan changed after L2 read it: reviewed sha256:fc7c3f8749da… but current is sha256:bdeca17d6ab3… — recovery is to rerun L2 against the current plan, NOT to reseal (DD13)

## Findings

| Perspective | Severity | Claim | Evidence |
|---|---|---|---|
| architect | HIGH | vendor-targets helper function signatures are inconsistent between Task 3 and Design Decision D5 | Task 3 (line 318) specifies: '`vendorTargets.activeVendorsFor(ruleId)` 로 검증한다' — function takes only ruleId. But D5 (line 188) specifies: '`activeVendorsFor(svc, ruleId)` · `legacyUrlsFor(svc, ruleId)`' — both functions take TreatProviderService as first parameter. This inconsistency violates the design intent stated in D5: to enable AlertService to call the helper without needing TreatProviderService injected ('그래야 `AlertService` 가 새 의존을 주입받지 않고 이미 가진 것을 넘겨 부를 수 있고'). If Task 3's incomplete signature is implemented, the design's dependency avoidance goal will fail. |
| architect | HIGH | AlertService.sendTest cannot compute legacyUrls with the stated design without TreatProviderService dependency | D5 (line 188) states the helper functions take TreatProviderService as a parameter to compute legacy URLs and vendor lists. Task 3 (line 318) states sendTest must 'compute legacyUrls' using the shared helper. However, AlertService currently has no TreatProviderService injected (verified by grep of alert.service.ts constructor at lines 107-130). The plan claims (D5 line 188) AlertService can avoid new dependencies ('새 의존을 주입받지 않고'), but this contradicts the requirement that the helper functions take TreatProviderService as a parameter. Either: (a) AlertService must receive TreatProviderService as a new dependency (contradicting D5's claim), or (b) the design is incomplete about how AlertService obtains the vendor/legacy URL data. |
| architect | MEDIUM | R1 Design Critique absorption claim is not reflected in the current plan Tasks | Design Critique section line 629 documents R1 feedback: '`GET /rules` 호출이 불필요하다 (LOW)' with absorption verdict '흡수 — Task 5 에서 제거, `/monitor` 하나로' (absorbed — removed from Task 5, use /monitor only). However, Task 5 (line 337) explicitly states: 'データは **두 호출**로 채운다 — `GET /admin/alert/monitor` 가..., `GET /admin/alert/rules` 가...' (data uses **two calls** — /monitor and /rules). The absorption claim was not actually implemented. While R2-R3 re-added the /rules call as necessary for the higher-priority F1 finding, the R1 Design Critique table creates a false historical record by claiming absorption that doesn't match the current Tasks. |
| security | MEDIUM | Plan requires unknown-hint pre-blocking but does not clearly specify implementation path or response encoding | D5b lines 221-223 explicitly state unknown-hint 'must be caught before passing [to destinationsFor]' and notes 'if caught after, destinationsFor has already fallen back to default channels'. However, Task 3 action steps (a-f) at line 318 do not place a distinct validation-and-exclusion step before calling destinationsFor. The plan provides no implementation guidance for encoding an invalid hint in `excluded: [{channelId, reason}][]` response, since hint validation does not involve a specific channel. Line 601 explicitly marks semantic correctness as '(미검증)' (unverified), meaning the gate cannot distinguish a dummy test from real validation. If implementers miss this requirement, passing an invalid channelHint to destinationsFor would silently fall back to ruleId-based selection (alert.service.ts lines 385-397 show the fallback: if hintCh is falsy, base.length === 0 triggers line 394 ruleId-based selection), allowing channels not intended for that rule to receive test messages. |
| security | MEDIUM | Test validation gate #13 cannot verify semantic correctness of unknown-hint contract | Lines 525-552 describe validation check #13: it runs jest with -t flag for each of the 5 contract names and checks that total>=1, pending=0, failed=0. However, this only verifies the test runs and passes, not that it actually validates the contract. Line 601 in Acceptance criteria explicitly states: '단언이 계약과 의미상 맞는지는 `(미검증)` — 게이트가 출력에 그 사각지대를 적는다' (whether assertions match the contract semantically is unverified — the gate documents this blind spot). A dummy test like `it('unknown-hint pre-blocking', () => expect(true).toBe(true))` would pass the gate. Lines 549-550 only check that string literals exist, not that they are used correctly. |
| test | CRITICAL | sendTest method includes 5 safety contracts that guarantee narrow-by-ruleId behavior (D5b, UI10) | Plan line 601 acknowledges Validation #13 blind spot: '단언이 계약과 의미상 맞는지는 `(미검증)` — 게이트가 출력에 그 사각지대를 적는다'. Validation #13 (lines 525-552) can only verify test names exist and jest passes, not that test assertions verify destinations are actually narrowed. A test that asserts `expect(true).toBe(true)` with the contract name passes all gates. |
| test | HIGH | Test sends actually constrain destinations to ruleId-specific channels (main claim, UI10, D5) | No Validation gate verifies the behavior. Validation #4 checks parameter exists, Validation #5 checks withLabel is called, but no test verifies returned destinations are subset of destinationsFor(ruleId) vs. full channel list. The claim appears in lines 156-161 (D5) and line 317 (Task 3) but no Validation tests it — only contract names. |
| test | HIGH | Manual render observations (breadcrumb count, vendor row expansion, legacy display) are accurate (Task 8, Gates #10 #14) | Gates #10 (line 481) and #14 (line 559) check: `grep -Eq 'keyword[^:]*:.*[0-9]'` — verifying a number was written, not that it's correct. Plan line 708 from R3 review shows 'Task 8 은 treat-vendor 하나만 관측하라 했는데 Acceptance 는 두 규칙을 주장했다' — observation completeness not verified. No test runner in admin (INV1 blocks dependencies), so no automated render verification possible. |
| test | HIGH | Shared vendor-targets.ts helper correctly extracts activeVendorsFor and legacyUrlsFor logic (D5, Task 3, line 84) | Validation #16 (lines 563-572) checks file exists and private activeVendors is gone, but doesn't verify the extracted helper logic is correct or identical to both original implementations. A broken helper passes the gate as long as the file exists and deletion succeeds. Validation #16 line 570: checks only `private async activeVendors(` count == 0, not correctness of the new function. |
| test | MEDIUM | Render notes file in Acceptance 'has 8 items with observations' (line 597-599, gate #10) | Gate #10 (line 481) requires keywords '브레드크럼, 줄바꿈, 가로폭, 테스트발송' with at least one digit. But Task 8 (line 356) says 'two rules' must be observed separately: '벤더행-treat-monitor, 벤더행-treat-vendor, 전역레거시-treat-monitor, 전역레거시-treat-vendor'. Gate #14 (line 559) uses same keywords, so a single observation of one rule can satisfy both promises. Task 8 explicitly requires rule-by-rule observation but gate allows aggregate keywords. |
| test | MEDIUM | Exactly 4 columns get `.al-td-wrap` exception (white-space: normal, max-width: 560px) as specified in D4 table (lines 145-154) | Validation #2 (lines 390-402) only checks the class selector '.al-td-wrap' exists at least once in AlertPages.css. It doesn't verify which elements/columns actually have the class applied. Someone could add the class to 2 or 5 columns and validation still passes. The specific 4 columns (억제 중인 경고·내용, 스누즈·내용, 최근 발송·내용, 상태·error) are named in Task 6-7 Mirror refs but no gate verifies the scoping. |
| invariant | CRITICAL | Gate #9 (impeccable detect) passes silently when CSS artifact doesn't exist, creating a skip predicate that masks missing prerequisite work | .claude/plans/alert-console-redesign.plan.md lines 466-473. The gate runs `detect.mjs --json --scope layout "$A"` on a directory. If CSS files are never created (Task 1 fails), the directory contains no CSS and `detect` returns an empty JSON array `[]` with length 0. Line 473 checks `[ "$N" = "0" ] \|\| false` which passes when N=0, even though the prerequisite work (Task 1: create AlertPages.css) was never done. Line 369 acknowledges 'the procedure for negative test case is undecided.' This is fail-open drift: a gate that cannot distinguish 'artifact missing' from 'artifact clean'. |
| invariant | HIGH | Gate #14 (render observation verification) depends on unanchored manual notes that cannot fail-closed; acceptance criterion is marked unverifiable yet remains in completion list | .claude/plans/alert-console-redesign.plan.md lines 554-561 (gate #14) and lines 604-605 (acceptance). Gate #14 runs `grep -Eq "$K[^:]*:.*[0-9]"` on a notes file for 4 keywords including '벤더행-treat-monitor'. Task 8 (lines 355-359) requires manual render observation with no automated check. Line 604-605 acceptance states '전역 레거시 목적지가 유일한 미발송 축이며 그 사실이 행 안에 있다 `(미검증)`' — explicitly unverified. The gate passes if: (1) notes file keywords appear with ANY number, even incorrect values; (2) notes file is never written (grep returns 0, false is truthy). No integrity binding (hash/digest) anchors notes file state to code. Acceptance criterion is in the completion list despite documented impossibility of verification (line 604: 'admin 에 테스트 러너가 없고 INV1 이 도입을 막는다'). |
| invariant | HIGH | Gate #15 uses grep substring matching instead of semantic verification; cannot detect if channelHint/testTargets are actually wired in control flow | .claude/plans/alert-console-redesign.plan.md lines 515-523. Gate #15 checks presence of keywords across 5 files using `grep -q 'channelHint'` and `grep -q 'testTargets'`. This is layer 3 (command ≠ gate): grep cannot verify that (1) channelHint is actually used in the sendTest() signature vs. in a comment, (2) the parameters are wired to the service method, (3) the values flow through to the API response. A developer could add `// TODO: use channelHint` and the gate passes. The Codex review R3 (line 704) showed this exact incomplete wiring: Task 3 and Task 4 were supposed to add the axis but didn't have it in the API contract until Task 4b was added. |
| invariant | MEDIUM | Gate #13 verifies contract names exist but not that they exercise the actual control flow; relies on grep for sendTest call count which can be gamed | .claude/plans/alert-console-redesign.plan.md lines 525-552. Gate #13 line 544-546 runs `grep -c 'sendTest('` to verify >= 5 calls to the function. This counts substring occurrences which could appear in comments, docs, or unused code paths. The jest verification at lines 531-541 is better (runs actual tests with `-t` for each contract name), but if the spec file has dummy placeholder tests with the contract names, they would execute and pass (though subsequent code grep for reason codes at line 548-550 provides some friction). The gate does not verify that each jest test actually exercises the `sendTest()` function with real arguments — it only counts test names and output counts. |
| invariant | MEDIUM | Task 3 specifies optional channelHint parameter but plan does not document what happens with missing or invalid values; Codex review shows this was incomplete at initial gate passage | .claude/plans/alert-console-redesign.plan.md Task 3 lines 318-322 states sendTest accepts `{ruleId?, channelHint?, channelId?, text?}` where channelHint is optional. Line 322 mentions validation via `vendorTargets.activeVendorsFor(ruleId)` as a "선택 인자" but the action description doesn't specify: what if channelHint is invalid/not in the active list? Plan says it will 'not send' and 'exclude' it, but validation is described as happening 'before sending' (line 318). Codex review R3 line 704 explicitly notes this was not initially addressed: '벤더 라우팅 축이 구현 가능한 API 계약에 없다' — the plan had the design but no API contract, discovered during codex review. This indicates the planning gate itself did not catch incomplete specification. |
| invariant | MEDIUM | Plan includes acceptance criteria marked (미검증) unverified due to inherent platform limitations, yet these remain in completion criteria instead of fail-closed alternatives | .claude/plans/alert-console-redesign.plan.md lines 600-606 acceptance list. Line 604 states '벤더 차원 규칙 2종 각각이 활성 벤더 수 + 1 행으로 펼쳐지고… — 게이트 #14 는 그 관측이 **규칙 이름별로 숫자와 함께 기록됐는지**까지만 본다. 적힌 숫자가 실제 렌더와 같은지는 `(미검증)` — admin 에 테스트 러너가 없고 INV1 이 도입을 막는다.' This is documented as checking that the keyword exists with a number, not that the number is correct or that both rules are actually observed. A gate that cannot verify correctness (by design constraint) should not be in acceptance unless a fail-closed alternative is provided. Current gate will pass even with incorrect numbers. |

## Refutation attempted

| Perspective | Verdict | What was attacked |
|---|---|---|
| architect | fail | Verified Citations: Confirmed accuracy of G1-G12 ground truth observations; verified existing file structures (AlertSettingPage.tsx inline styles at lines 16-43, TreatMonitorPage.css tokens and patterns at lines 1-18, alert controller structure and guards at lines 37-39, Header.Breadcrumb unconditional rendering at lines 46-79). Examined Design Decision D1-D5 for abstraction integrity and boundary leaks: D1's CSS duplication strategy with token value identity is acceptable given regression constraints; D2's breadcrumb placement avoids AdminLayout modifications correctly per D6; D3-D4 maintain existing patterns. CRITICAL STRUCTURAL ISSUE FOUND: D5's design intent relies on helper functions taking TreatProviderService as a parameter to avoid new AlertService dependencies, but Task 3's action description omits this parameter in the activeVendorsFor call signature, breaking the design's coupling avoidance mechanism. Task 3 shows `activeVendorsFor(ruleId)` while D5 specifies `activeVendorsFor(svc, ruleId)` — an inconsistency that prevents the stated design goal of AlertService avoiding TreatProviderService injection. Additionally, AlertService has no current TreatProviderService dependency (verified: grep of constructor shows @Optional @Inject(ALERT_RULE) only), and plan provides no mechanism for it to obtain vendor/legacy URL data without this dependency or a new layering violation. Secondary issue: R1 Design Critique falsely documents absorption of 'remove GET /rules' but current Task 5 uses two calls (/monitor + /rules), though this was correctly superseded by R2-R3 findings. |
| security | fail | Attacked: 1. **Trust boundary on ruleId**: Verified that ruleId is validated against ruleCatalog (alert-admin.controller.ts line 170) and DTO uses @Matches(RULE_ID_PATTERN) with pattern `/^[a-z0-9-]+$/` (alert.entity.ts line 56). No path traversal or injection vectors found. Passed. 2. **Environment label fail-closed**: Verified that envLabel() checked at alert.service.ts line 478 with fail-closed return (skipped: 'env-label-missing'). Also checked at module init (line 135) and fallback paths (line 1290-1292). Redundancy is intentional per design. Passed. 3. **URL masking in responses**: Verified that all controller responses mask URLs before returning to client (alert-admin.controller.ts lines 92, 264). Passed. 4. **Intersection logic correctness**: Plan explicitly specifies intersection formula (D5b lines 202-205) and creates test contract #13 `sendTest 계약: channelId 교집합` to verify it. Implementation would be caught by this contract test. Passed. 5. **Silent fallback risk**: Found that destinationsFor (alert.service.ts lines 375-421) silently falls back from hint-based to ruleId-based channel selection if hint doesn't match any channel (lines 384-399). Plan identifies this at D5b line 216 and lines 221-223, requiring pre-check. However, the pre-check requirement is not sufficiently implemented in Task 3 specification — this is the identified defect. 6. **Invalid channelId handling**: Verified that invalid channelIds (negative, non-existent) would result in empty intersection, yielding empty results. No bypass found. 7. **DTO validation**: Verified @IsInt(), @IsOptional(), @IsString(), @MaxLength(64), @Matches(RULE_ID_PATTERN) decorators block common injections. Passed. Could not land these as concrete defects (no evidence path found): - Leakage of secrets in error messages (maskWebhookUrl applied consistently) - Privilege escalation via permission bypass (guards already in place) - Database corruption or stale ruleCatalog attacks (outside plan scope, existing infrastructure) |
| test | fail | Attacked: (1) Validation #13 semantic verification claim via explicit blind-spot acknowledgment (line 551-552); (2) Main UI10/D5 claim of destination narrowing — found no gate that tests actual narrowing, only method name existence; (3) Manual observation gates #10/#14 — found they only verify numbers were written, not accuracy, and Task 8 requires per-rule observations but gates allow cross-rule satisfaction; (4) Shared helper correctness — found only file existence checked, not logic correctness; (5) CSS exception scoping — found only class existence checked, not application to correct 4 columns. Could not refute: (6) Token parity (Validation #11 does byte-compare); (7) File existence/basic structure (Validations #2-4 check these adequately). Did not access submodule code due to worktree structure; analysis based on plan text and validation gate logic. |
| invariant | fail | Examined: (1) Gate #9 impeccable detect behavior when CSS missing vs. when present — empty directory still returns JSON array length 0, gate passes either way. (2) Gate #14 notes file anchoring: no hash binding, just grep for keywords + numbers. (3) Gate #15 substring grep vs. semantic verification of wiring. (4) Gate #13 contract names: jest execution count verification present but grep for call count can be bypassed. (5) Task 3 optional parameter validation scope gaps. (6) Acceptance criteria marked unverifiable but listed as complete. Traced the skip predicate path: no CSS files → detect returns [] → length 0 → gate passes silently. Tested mental execution of fail cases: gate #14 with missing notes file, gate #14 with wrong observation numbers, gate #9 with Task 1 incomplete. All show silent pass or no binding to actual values. No rollback mechanism issue found (ALERT_ENV_LABEL consistently fail-closed like production). No accounting issues in task assignment. |

## Measurement

<!-- Written by plan-review/cli.js record on EVERY exit path, pass or halt.
     Machine-readable; do not hand-edit. A null field means the axis was
     not observed, never that it was zero. -->

```json
{
  "verdict": "unavailable",
  "source": "multi-agent",
  "layers": {
    "l1": "converged",
    "l2": "divergent",
    "l3": "not fired"
  },
  "quorum": {
    "responded": 4,
    "required": 2,
    "roles": 4,
    "of": 4,
    "passed": false
  },
  "wall_clock_ms": 453696,
  "halt_stage": "5.2e",
  "granted": 4,
  "reviewed_plan_hash": "sha256:fc7c3f8749da88434753cd1651ddf513f75af822b85f940b57fc7c61541a8a83",
  "plan_path": ".claude/plans/alert-console-redesign.plan.md",
  "recorded_at": "2026-08-19T04:04:40.542Z"
}
```