# Plan Review Panel — samsung-account-intake

**Plan**: `.claude/plans/samsung-account-intake-m1.plan.md` · **Plan version**: `sha256:9406bacb2a9f758bad0950c885bc7e8cbaa077785783da36d76fb79e6a01cf92`
**Verdict**: `divergent` via `multi-agent`
**Quorum**: 4/3 responses · 4 distinct roles (of 4 fielded) · passed=false
**Layers**: L1 converged · L2 divergent · L3 not fired (classification=exit-nonzero blocking=true exit=1)
**Halted at**: `5.2e`

> Reason: L2 quorum not satisfied: 4 blocking finding(s): security/HIGH, security/FAIL, invariant/HIGH, invariant/FAIL

## Findings

| Perspective | Severity | Claim | Evidence |
|---|---|---|---|
| architect | LOW | The `#9` seal does not cover the whole invariant it claims ("되살리기를 HTTP 로 켤 수 없다"). It only checks the `ResignAgencyFcDto` class body, the first `agencyFcService.resign(` in one controller file, and `this.resign(` inside one service file. If any other module injects `AgencyFcService` and calls `.resign(dto, admin, {restoreSubscribe: body.x})`, or a different DTO forwards the flag, `#9` still passes. Today there are exactly 3 call sites, so this is not a present defect, but the gate enforces a file-local version of a repo-wide claim. | .claude/plans/samsung-account-intake-m1.gates.mjs:195-213 (`dto.indexOf('class ResignAgencyFcDto')`, `ctl.indexOf('agencyFcService.resign(')`, `svc.matchAll(/this\\.resign\\(/g)`); a grep of `\\.resign\\(` across insuniverse-server/apps finds only fc-agency.service.ts:1060,1063 and agency-fc-admin.controller.ts:459 |
| architect | LOW | `#9` treats any 3-argument `this.resign(` call in fc-agency.service.ts as a leak. The plan names milestone 5's intake-confirm path as the first legitimate opt-in caller. If that path lives in the same service, M1's frozen seal would reject the intended consumer. M5 will have to rework the seal rather than just attach to it. | plan §Summary: "첫 소비처는 마일스톤 5 의 인테이크 확정 경로다"; gates.mjs:210 `if (a.length > 2) leaked.push(...)` |
| security | HIGH | Check #10 is supposed to keep the plaintext-password attachments out of history, but it can pass without checking anything. Its watch list is built from `git status` the first time #1 runs, and only if baseline.json does not exist yet. If the attachments are committed before that first run (for example `git add -A` then commit, which the repo's commit-before-review rule encourages), they no longer appear in `git status`. The baseline then records `attachments: []` and #10 prints `#10 ok  감시 대상 0건` forever. #8 does not catch it either, because committed files are also invisible to `git status`. Both gates stay green in exactly the state they exist to prevent. | gates.mjs:90 `attachments: recs.filter((r) => !inWork(r.path))` comes only from the porcelain snapshot, with no check against `git ls-files`/HEAD. gates.mjs:219 loops only over `b.scope.attachments`, so an empty list means zero checks and the ok message at :226. Plan:220-226 regenerates the baseline only when `[ ! -f "$B" ]`; the only dirty guard at :221 is on the submodule, not the repo root. Plan:126 marks the baseline as CREATE, so the first run can come after a commit. Observation 16 says the `계정등록` xlsx/tsv contain real names and plaintext passwords. |
| security | MEDIUM | The plan says #8 hands attachments to #10 so that credential file names stay out of the committed Files to Change table. But baseline.json is itself a committed artifact (CREATE in Files to Change), and it writes those same attachment paths verbatim, including the Korean `계정등록`/`계정삭제` request file names and the `mail.google.com_mail_u_0_.png` screenshot name. The file names end up in history anyway, just in a different file. | Plan:300 says `표에 자격증명 파일명을 싣지 않기 위해서다`. Plan:126 lists `.claude/plans/samsung-account-intake-m1.baseline.json \| CREATE`. gates.mjs:90 writes `attachments` paths into that file. The git status snapshot shows the root untracked files `양식 계정등록(17차 요청 0908).xlsx` etc. |
| security | MEDIUM | The scope-gate exemption in #8 trusts a hash that the editable file supplies about itself. `pre_existing` entries (path, xy, sha) in baseline.json are never checked against anything that stops later edits. baseline.json is in Files to Change, so editing it is not flagged. Adding an entry whose sha matches the post-change content of any out-of-table file silently exempts that file from #8. | gates.mjs:177-184 exempts a path when `key(path, xy, sha(currentContent))` is found in `b.scope.pre_existing`. Nothing covers baseline.json with a digest or ties it to the plan hash. Plan:126 lists baseline.json as an allowed change, and gates.mjs:75 excludes `out` from its own snapshot. |
| test | MEDIUM | Gates #3, #4 and #5 only check that `it` blocks with the right names exist and passed. They never look at what those tests assert. So the core claims could be backed by empty or wrong assertions and still go green: 'resign({useAt:1}) does not touch the subscription by default' and 'the stop path is unchanged'. This is the over-permissive direction. Nothing makes the named spec fail against the pre-fix code, and the plan's own negative proof comes from a probe spec that has since been deleted. | gates.mjs:112-130 `spec-names` only reads `a.title` and `a.status` (`byName.get(n) !== 'passed'`). Plan #3 `# 음성:` cites '이 창에서 작성·실행 후 삭제한 `__probe.spec.ts`' (observation 5), and says '이 검사는 수정 전 코드에서 반드시 떨어진다'. Before the spec exists, the only failure is `필수 it 누락`, which does not show that the behaviour assertion fails on old code. |
| test | LOW | Gate #4 only checks that the four reason strings appear somewhere in the service file. It does not tie each reason to the branch that should produce it. A swapped `expired` / `not-cancelled` classification would pass the source check, and would also pass #4 if the spec's expectations were written to match the implementation. | plan.md:257 `grep -q "'$w'" "$SVC"` checks for presence anywhere in fc-agency.service.ts. Acceptance line 356 says '사유로 분류된다 — 게이트 #4'. |
| invariant | HIGH | Gate #8 stops seeing the submodule once the implementer commits there. The scope check drops the submodule's gitlink record and reads only the submodule's uncommitted changes. After a commit inside insuniverse-server (the plan expects one: Acceptance mentions '서브모듈 커밋의 push'), no submodule change is visible. #8 still prints 'ok 변경 N경로 전건 표 안', so an extra, unlisted file committed in the submodule passes without being seen. Nothing compares the submodule HEAD to the pin 4d798f55. | .claude/plans/samsung-account-intake-m1.gates.mjs:183 `.filter((r) => r.path !== planPath && r.path !== subDir && !watched.has(r.path))`; plan Validation line 310 `git -C "$S" status --porcelain -z -uall` (uncommitted changes only); plan line 368 '서브모듈 커밋의 push 와 부모 포인터 bump — (미검증)'; project CLAUDE.md §4 '커밋한 뒤 리뷰어를 돌린다' |
| invariant | MEDIUM | #1's clean-tree guard is weaker than the plan says. It only checks for uncommitted files in the submodule, not whether the submodule HEAD is still the pin. If the baseline file is missing after a submodule commit (fresh worktree, a redo slug, or the uncommitted baseline was deleted), the guard passes on a clean tree. The baseline is then taken with the new code and spec already in place, so any new failures they cause are frozen as 'already red before work' and #2/#6 absorb them. This is the exact H3 hole the plan says the guard closes. | plan Validation lines 209-210 claim `status --porcelain` 빈 문자열 guard prevents '이 창이 만든 spec 의 붉음이 착수 전 정상으로 동결'; lines 218-221 check only `git ls-tree HEAD "$S"` (the parent's recorded pointer, not the submodule HEAD) and `git -C "$S" status --porcelain -uall`; gates.mjs:99-110 jest-regress trusts the baseline's failing_suites |
| invariant | MEDIUM | #10's watch list is taken once, when the baseline is created, and never recomputed. The baseline only counts a changed file as an attachment if it sits outside .claude/ and insuniverse-server/. Suppose a credential xlsx is moved under .claude/ and the baseline is then regenerated: it is recorded as pre-existing, so #8 exempts it (same path, status and sha), and #10 no longer watches it. Both gates pass while the file stays exposed and can be committed. | gates.mjs:74 `inWork = p.startsWith('.claude/') \|\| p.startsWith('insuniverse-server/')`; gates.mjs:86-90 pre_existing vs attachments split; gates.mjs:219 attach-guard iterates only `b.scope.attachments`; plan line 220 baseline regenerates whenever `! -f "$B"` |
| invariant | LOW | #8's PRD revert guard can be switched off with an environment variable. Setting M1_PRD_EXPECT=complete makes the gate accept a PRD row already marked complete, and the only signal is a warning on stderr. The gate never checks that the milestone has actually closed. | plan Validation lines 306-308 `case "$EXPECT" in in-progress\|complete) ;;` and `[warn] ... 1>&2`; gates.mjs:189 accepts whatever expect is |
| invariant | LOW | #4's reason-vocabulary check only greps the whole service file for the quoted word, so a comment satisfies it. It does not prove the reason is actually produced. | plan Validation line 257 `grep -q "'$w'" "$SVC"` |

## Refutation attempted

| Perspective | Verdict | What was attacked |
|---|---|---|
| architect | pass | 1. Checked the plan's claims against the code. It says `resign` ignores `useAt` and unsubscribes every target; fc-agency.service.ts:801-815 confirms this. 2. Checked the claim of 3 call sites by grepping `.resign(` across the whole server app; the only hits are service :1060/:1063 and controller :459. 3. Checked the claim that the controller's `@Roles` includes `corp`; controller :456 confirms it. 4. Checked the plan's cited pattern for the 'not expired' definition; service :1331-1332 matches the plan (`subState == '해지' && !subDisableCompletedAt`). 5. Checked that `getAgencyFcForSubscribe` already selects `s.sub_disable_completed_at` (repository :441), so the new field has a source to mirror. 6. Checked that adding 2 fields to `getAgencySubscribe` is additive: the type readers at service :1461/:1474/:2490 only index into the result. 7. Tested whether the opt-in really stays off the HTTP path: it is a separate method argument, it is not on the DTO, and the controller call keeps 2 arguments. The `#9` checks are real but file-scoped (LOW). 8. Tested whether the design leaves milestone 5 a clean place to attach: the return value exists but has no consumer yet, which the plan states openly, and the `#9` seal clashes with M5's intended caller (LOW). 9. Checked whether `#8` and `#10` split ownership cleanly: `scope-check` excludes only `attachments`, and `attach-guard` covers exactly that list. I found no HIGH or CRITICAL structural defect. |
| security | fail | 1. Checked that corp cannot switch on the restore through HTTP. The plan keeps `restoreSubscribe` out of the DTO, and #9 (gates.mjs:193-214) checks the DTO class body, the number of arguments in the controller call, and every `this.resign(` call inside the service. I could not land a body-binding path, only parser-evasion ideas (for example the field inherited from a base DTO class), and I have no evidence that such a class exists, so I did not report it. 2. Confirmed the opt-in is a method argument that no M1 code passes, so a request cannot change billing state. 3. Checked whether the corp-visible change (`sub_disabled_at` no longer rewritten) could escalate anything. It removes a write, so no. 4. Traced the credential-attachment leak end to end: commit before the first baseline run leads to an empty watch list, so #10 passes with nothing checked and #8 cannot see the committed files. This one landed. 5. Checked whether #8's handoff lets new root files slip through. It does not: only paths present when the baseline is taken are exempted. 6. Checked whether the `M1_PRD_EXPECT` override can be forced. It is limited to two allowed values. 7. Checked whether the baseline leaks absolute paths or the machine name. Paths are repo-relative porcelain output, so no; the only leak is file names (reported). 8. Checked whether the plan logs plaintext passwords. M1 does not touch password handling. |
| test | pass | Read the plan, the PRD and the whole gates.mjs. - Checked that all 11 subcommands exist and match the Validation calls. - Attacked #9 optin-seal for a DTO class-body truncation hole. Multi-line decorators are indented `  })`, so `indexOf('\\n}')` lands on the real class end at agency-fc.dto.ts:145. No hole. - Attacked #7 for a false green. The first `.select(` after `async getAgencySubscribe(` is the real select (repository.ts:531-540). Today it has neither `memId:` nor `subDisableCompletedAt:`, so #7 does fail before the change, as claimed. - Confirmed the expiry mirror `subState=='해지' && !subDisableCompletedAt` at service.ts:1331-1332, and the unconditional `isUnsubscribed:1` loop at service.ts:808-813. - Searched for an existing spec that pins the old resign behaviour. No spec file under fc/agency mentions resign. - Checked the #2 and #6 machine-failure handling: a missing summary line fails, and rc!=0 with no parsed error files fails. - Checked #5: the suite count is derived from the baseline rather than hard-coded. - Checked #8: files in scope before the change are only excluded when path, status code and sha all match. Found no HIGH or CRITICAL defect. The remaining weakness is that the gates check test names, not what the tests assert. |
| invariant | fail | Read the plan, the PRD, and every subcommand in gates.mjs, and checked the controller @Roles and resign citations (they hold). For each check I traced what happens when things go wrong: jest missing or crashing with no summary line, tsc dead with a non-zero exit and no output, a missing report or spec, an empty group of test names, a changed pin, a missing baseline, a baseline regenerated after a commit, a submodule commit hiding changes from status, attachments moved into an exempt directory, the M1_PRD_EXPECT override, a default of true on the opt-in parameter (the 'default:' test catches this, so not reported), and parser misses in #7/#9, which fail closed. Rollback: the plan offers a one-commit revert, which is plausible because the parent pointer bump is done by hand; no test pins it, but the plan discloses that. Receipt and digest anchoring are not touched by this plan. |

## 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": "divergent",
  "source": "multi-agent",
  "layers": {
    "l1": "converged",
    "l2": "divergent",
    "l3": "not fired (classification=exit-nonzero blocking=true exit=1)"
  },
  "quorum": {
    "responded": 4,
    "required": 3,
    "roles": 4,
    "of": 4,
    "passed": false
  },
  "wall_clock_ms": 350591,
  "halt_stage": "5.2e",
  "backlog_appended": null,
  "backlog_skipped_nonblocking": null,
  "granted": 4,
  "reviewed_plan_hash": "sha256:9406bacb2a9f758bad0950c885bc7e8cbaa077785783da36d76fb79e6a01cf92",
  "plan_path": ".claude/plans/samsung-account-intake-m1.plan.md",
  "recorded_at": "2026-09-16T07:33:25.310Z"
}
```