Ebbe BaßandClaude Sonnet 5.5
24de5878ed
Includes the BMAD install config, the initiative ticket tree, the portal context-resources scripts, and the .gitignore change that tracks BMAD files (generated _bmad/render/ is ignored). Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Sk72AxLwQnYttF6VFSxQLt
58 lines
6.8 KiB
Markdown
58 lines
6.8 KiB
Markdown
# PRD Quality Review — ping-mee Self-Service Portal
|
||
|
||
## Overall verdict
|
||
Grade: Good. The PRD has a clear thesis (an admin-free self-service loop for an AD homelab), honest scope boundaries, and unusually good security reasoning for its size. The risk is in a handful of places where a requirement reads as testable but is not yet defined: the throttling requirement, the recipient list for announcements, and link-based actions that a mail scanner can trigger by accident. None block UX work; three should be fixed before architecture.
|
||
|
||
## Decision-readiness — strong
|
||
Decisions are stated as decisions and carry their reasons: separate Admin and Publisher groups (§3), no expired-password flow (§2.2), no SVG (FR-12, §5), opt-out means no mail at all (FR-20), pending detection via the MD5 file (FR-14). The cost of "ship everything" is named (§6.1 note, brief) rather than hidden. Open Questions are genuinely open.
|
||
|
||
### Findings
|
||
- **low** Admin-changeable security settings (FR-23) — An Admin session can weaken the failed-attempt limit or session time. Settings changes are audited (FR-28), but there are no floor values. *Fix:* add a consequence that limits have minimum values.
|
||
|
||
## Substance over theater — adequate
|
||
No persona theater (four journeys, each drives requirements) and no innovation theater (the brief admits there is no moat). §8.3 and §8.4 are thinner: "failures are reported in plain language" and "meets WCAG 2.1 AA" carry no thresholds, and there is no statement about availability expectations.
|
||
|
||
### Findings
|
||
- **medium** Reliability has no targets (§8.3) — Says what is reported, not what is expected. *Fix:* state explicitly that there is no availability target for the homelab release, or set one (for example "restart-safe, no data loss on restart").
|
||
- **low** Upload processing time (FR-12) — No bound. *Fix:* add a testable limit (for example "a 10 MB HEIC is processed within 10 seconds or fails with a message").
|
||
|
||
## Strategic coherence — adequate
|
||
The thesis is coherent and the features serve it. Success metrics validate the thesis and counter-metrics are present. The weakness is that "everything ships together" leaves downstream workflows with no dependency order.
|
||
|
||
### Findings
|
||
- **medium** No build-order guidance (§6.1) — Epics and stories will have to infer that sign-in and roles come first, the audit log is needed by everything, and the Admin UI depends on runtime settings. *Fix:* add a short dependency note (not phasing) under §6.1.
|
||
- **medium** Metrics are hard to measure (SM-1, SM-2) — "0 manual changes" and "all of them" cannot be observed from the Portal, because manual work happens outside it. *Fix:* state the measurement method (for example the Admin's own count, plus audit-log counts of self-service actions).
|
||
|
||
## Done-ness clarity — adequate
|
||
Most FRs have testable consequences. Weak spots are where the behavior is implied rather than defined.
|
||
|
||
### Findings
|
||
- **high** FR-4 "Throttle requests" — "failed attempts" is undefined: sign-in happens at authentik, so the Portal never sees failed sign-ins. The only failed attempts the Portal sees are in the password-change and e-mail-change forms (FR-6, FR-10). Which endpoints does FR-4 cover, and are the limits per User, per IP, or both? *Fix:* list the covered endpoints (password change, e-mail change, confirmation and undo links, uploads) and state limits for each; keep FR-6 as the account-lockout protection.
|
||
- **high** FR-10 / FR-11 links act on GET — Mail clients and security scanners open links automatically. A scanner could confirm an e-mail change or trigger an undo. *Fix:* the link opens a page that shows the pending action and requires a click (POST); add this as a consequence to both.
|
||
- **medium** FR-12 minimum input size — Rejects over 10 MB but says nothing about small images (for example 200×200). The sync script resizes to 448 px, so a tiny source gets upscaled. *Fix:* define a minimum size (reject, or accept with a warning).
|
||
- **medium** FR-1 disabled and locked accounts — Not covered. *Fix:* a consequence that a User whose AD account is disabled or locked cannot start or keep a Session.
|
||
- **medium** FR-14 relies on timing in the script — `<sam>.md5` is written before the AD attributes are set. If the AD write fails afterwards, the Portal sees a match and clears Pending while the picture is not live. *Fix:* note this in the addendum and add a consequence that the Portal also compares the `thumbnailPhoto` change or accepts the limitation explicitly.
|
||
|
||
## Scope honesty — strong
|
||
Non-Goals do real work (no admin console, no reset flow, no SVG, no cn/name). Assumptions are tagged inline (16) and indexed. Open Questions Q-1 to Q-7 are specific. The "everything ships together" decision is attributed to the Admin, not smoothed over.
|
||
|
||
### Findings
|
||
- **high** Recipient list is undefined (FR-20, FR-21) — "New Users are opted in", but the Portal only learns about a User at first sign-in. Do Users who have never signed in get downtime e-mails? Users without a `mail` value? This decides who is told about downtime. *Fix:* add an Open Question and a requirement for the recipient source (all Users in the OU with a valid `mail`, versus only Users who have signed in).
|
||
- **medium** Time zone is unspecified (FR-17) — Downtime windows and scheduled publish times have no time zone rule. *Fix:* add Q-8: store UTC, display in a configured time zone, and name the zone in e-mails.
|
||
- **medium** Contradiction on anonymous visitors (§2.2 vs FR-22) — §2.2 says anonymous visitors see nothing except the redirect, but FR-22 says backgrounds are served to anonymous visitors. *Fix:* decide where backgrounds show (after sign-in only, or on a pre-sign-in landing page) and align both.
|
||
|
||
## Downstream usability — adequate
|
||
IDs are contiguous and cross-references resolve. Journeys have named protagonists. Glossary is present, with some gaps.
|
||
|
||
### Findings
|
||
- **medium** Glossary gaps — "audit log", "draft", "withdrawn", "severity" and "downtime window" are used as terms but not defined. *Fix:* add them, and use "Audit Entry" or "audit log" consistently.
|
||
- **low** Casing drift in the Vision (§1) — "user" and "admin" in lower case where §3 defines User and Admin. *Fix:* capitalize, or accept the Vision as informal prose.
|
||
|
||
## Shape fit — strong
|
||
A capability-first PRD with short journeys suits a single-operator internal tool with several user roles. The weight on security requirements matches the internet exposure.
|
||
|
||
## Mechanical notes
|
||
- IDs: FR-1 to FR-29, UJ-1 to UJ-4, SM-1 to SM-5 and C1 to C3, Q-1 to Q-7 are contiguous with no unresolved references.
|
||
- Assumptions Index: roundtrip is incomplete. The inline tags in FR-3, FR-12 and FR-14 are indexed; the "notify again" default and the audit-failure behavior are not tagged.
|
||
- Required sections for an internal tool are present; no compliance section is needed, but GDPR basics sit under §8.2 as an assumption.
|