diff --git a/.claude/agent-memory/review-review-issue-fixer/MEMORY.md b/.claude/agent-memory/review-review-issue-fixer/MEMORY.md new file mode 100644 index 0000000..1d4d072 --- /dev/null +++ b/.claude/agent-memory/review-review-issue-fixer/MEMORY.md @@ -0,0 +1 @@ +- [Dashboard fix patterns](project_dashboard_fix_patterns.md) — @honmoon/policy value imports resolve; eslint type-specifier rule; oklch contrast compositing gotchas diff --git a/.claude/agent-memory/review-review-issue-fixer/project_dashboard_fix_patterns.md b/.claude/agent-memory/review-review-issue-fixer/project_dashboard_fix_patterns.md new file mode 100644 index 0000000..051ef51 --- /dev/null +++ b/.claude/agent-memory/review-review-issue-fixer/project_dashboard_fix_patterns.md @@ -0,0 +1,22 @@ +--- +name: dashboard-fix-patterns +description: Honmoon dashboard fix constraints — @honmoon/policy value imports resolve, eslint import-style rule, oklch contrast math +metadata: + type: project +--- + +Fixing review findings in `apps/dashboard`: + +- **Value imports from `@honmoon/policy` build fine.** Its package.json `exports` points at raw + `./src/index.ts`, so Vite transpiles it directly — no dist build step needed. A finding that says + "skip if the value import cannot resolve" can be answered with a `build` run, not guessed. + **Why:** several dashboard files use `import type` only, which is erased and proves nothing at + typecheck time. **How to apply:** only `bun run --filter '@honmoon/dashboard' build` is evidence. +- **eslint enforces `import/consistent-type-specifier-style`.** Inline `type` specifiers mixed into + a value import (`import { X, type Y }`) error out; it must be split into a separate + `import type { Y }` line. **Why:** repo lint config. **How to apply:** when a finding literally + prescribes the inline form, apply it then run `bunx eslint --fix` on that file. +- **Contrast findings cite oklch tokens in `src/index.css`.** Composite translucent surfaces in + gamma-encoded sRGB (not linear) or the ratio comes out optimistic. Real backdrops nest + `--surface-soft` over `--surface-glass` over `--bg`, which is worse than the two-layer case a + finding usually quotes — check the three-layer stack too. diff --git a/.please/docs/review/rejected-findings.jsonl b/.please/docs/review/rejected-findings.jsonl index 581a173..d4b0213 100644 --- a/.please/docs/review/rejected-findings.jsonl +++ b/.please/docs/review/rejected-findings.jsonl @@ -1 +1,11 @@ {"key":"c289642ddef91a0d","file":"crates/honmoon-proxy/tests/redaction.rs","title":"The cache-stability wire test does not model a growing conversation prefix","reason":"Evidence misattribution: the growing-prefix scenario is covered by public_path_extending_the_history_preserves_the_redacted_prefix in crates/honmoon-core/tests/secret_tokenizer_public_api.rs:192-230 (plus two adjacent position-independence tests); the wire path delegates all tokenization to honmoon-core, so a prefix-instability regression fails at the core layer","rejected_at":"2026-07-20T12:42:25Z","source":"review-code"} +{"key":"c7b3265912ddbd2e","file":"apps/dashboard/src/App.tsx","title":"Gateway reachability status is completely hidden below md breakpoint","reason":"By design: every route renders its own visible ErrorNote (role=alert, no responsive hiding) when its poll fails, so narrow-viewport users still get an explicit unreachability signal; hiding the header capsule below md is a deliberate responsive trade-off","rejected_at":"2026-09-03T12:33:15Z","source":"code-review"} +{"key":"680f01c120f9d897","file":"apps/dashboard/src/components/Overview.tsx","title":"Overview's featured approval assumes array order that Approvals page does not","reason":"Factually wrong: honmoon-proxy approval.rs pending() sorts by monotonic id (oldest first, tested by pending_is_sorted_by_id) and mgmt list_approvals returns it unchanged; demo shim also pushes in registration order, so approvals[0] is always the oldest","rejected_at":"2026-09-03T12:33:15Z","source":"code-review"} +{"key":"c855f8b101ee73d7","file":"apps/dashboard/src/index.css","title":"Policy editor textarea permanently disables the focus-visible outline","reason":"Factually wrong: :focus-visible (index.css:129) is unlayered while .policy-editor textarea { outline:none } is inside @layer components; unlayered declarations beat layered ones regardless of specificity. Verified empirically: keyboard focus renders the 2px accent outline","rejected_at":"2026-09-03T12:33:15Z","source":"code-review"} +{"key":"88a1b1c4d3e6a220","file":"apps/dashboard/src/index.css","title":"Policy editor gutter width doesn't match the reserved code padding","reason":"Factually wrong: Tailwind v4 Preflight sets box-sizing:border-box, so .line-no width:48px includes padding-right:14px; text right edge sits at 34px and the 58px editor padding leaves a 24px gap","rejected_at":"2026-09-03T12:33:16Z","source":"code-review"} +{"key":"5cd7bd3baeb114cf","file":"apps/dashboard/src/index.css","title":"Prism gutter line numbers use absolute positioning with no positioned per-line ancestor","reason":"Empirically refuted: built demo and measured .line-no spans; each has a distinct monotonically increasing top (~21.25px apart) aligned to its line. Abspos with top/bottom auto resolves to the inline static position per CSS 2.1 10.6.4","rejected_at":"2026-09-03T12:33:16Z","source":"code-review"} +{"key":"09d20f43afff0463","file":"apps/dashboard/src/components/Overview.tsx","title":"Overview 'Latest decisions' rule column vanishes on mobile with no fallback","reason":"out of scope: style/taste — hiding a secondary column below md is a responsive design choice, not a WCAG failure or logic error; the full Audit route keeps every column","rejected_at":"2026-09-03T12:33:16Z","source":"code-review"} +{"key":"988459f6c5a04fd0","file":"apps/dashboard/src/components/Overview.tsx","title":"Overview's ApprovalStrip and Approvals' ApprovalCard track busy state in separate hook instances, allowing a duplicate approve/reject across a page switch","reason":"By design / no observable effect: approval.rs resolve() uses .remove(&id)? so a second approve/reject returns 404 'no such pending approval' — no double-application; per-list busyIds makes no cross-surface claim, and the trigger requires a second aimed click inside one same-origin POST round trip across a route change","rejected_at":"2026-09-03T12:39:57Z","source":"code-review"} +{"key":"71a632542061c550","file":"apps/dashboard/src/components/ApprovalActions.tsx","title":"ApprovalActions summary doc-comment example omits the colon the code actually renders","reason":"Pure style: the JSDoc e.g. illustrates the summary content, not the literal aria-label string; the templates sit a few lines below in the same file. No observable effect","rejected_at":"2026-09-03T12:52:50Z","source":"code-review"} +{"key":"b4811fcbf7b5071c","file":"apps/dashboard/src/components/PolicyView.tsx","title":"Prism-highlighted YAML gutter numbering can nest across multi-line tokens","reason":"Empirically refuted: prism-yaml only spans lines for block scalars, and Prism output closes every span so line-no spans are validly nested children; .token.string sets only color, so positioning is unaffected. Verified live in the built demo with a block scalar: identical left, even top spacing, normal font-style","rejected_at":"2026-09-03T12:52:50Z","source":"code-review"} +{"key":"4892e37e99f3a60c","file":"apps/dashboard/src/components/DecisionBadge.tsx","title":"DecisionBadge docstring overclaims that glyph distinguishes approved/rejected from allowed/denied","reason":"Pure style: 'glyph and label carry the meaning; color supports' mirrors the design-wide a11y comment on .verdict in index.css and does not claim the glyph alone disambiguates each pair; the label text keeps them distinct. No observable effect","rejected_at":"2026-09-03T13:07:35Z","source":"code-review"} diff --git a/apps/dashboard/AGENTS.md b/apps/dashboard/AGENTS.md index c25a04c..c014c97 100644 --- a/apps/dashboard/AGENTS.md +++ b/apps/dashboard/AGENTS.md @@ -39,8 +39,9 @@ rust-embed handler needs a rewrite rule). Unknown hashes fall back to Overview: | Path | What | |------|------| | `src/main.tsx` / `src/App.tsx` | Entry point; hash router + shell over the four views. | -| `src/components/` | `Overview`, `AuditLog`, `PolicyView`, `Approvals`, `DecisionBadge`. | -| `src/api.ts` / `src/hooks.ts` / `src/format.ts` | Typed management-API client, `usePolling`, formatters. | +| `src/components/` | `Overview`, `AuditLog`, `PolicyView`, `Approvals`, plus shared pieces: `DecisionBadge` (glyph + mono verdict pill), `ApprovalActions` (Deny / Approve pair), `ui` (`Panel` double bezel, `PageHead`, `SectionHead`, `ErrorNote`, `PanelState`). | +| `src/api.ts` / `src/hooks.ts` / `src/format.ts` | Typed management-API client, `usePolling` + `useApprovalActions` (per-id busy set), formatters. | +| `src/index.css` | G2 "Barrier Membrane" tokens (oklch, dark primary / light secondary), Tailwind `@theme` bridge, and the few component classes (`bezel`/`glass`, `verdict-*`, `action-*`, Prism YAML token colors). | | `demo/demo-mode.js` | Demo shim: patches `window.fetch` with in-memory fixtures, runs a scripted timeline, mounts the "demo" badge. Plain browser JS, no bundler. | | `demo/build.ts` | Copies `dist/` → `dist-demo/` and injects the shim's `