# LEDGER — Permissions Screen Redesign + Enforcement Hardening

**Feature:** إعادة تصميم شاشة الصلاحيات `/core/roles` + كتالوج وصف الصلاحيات (BE) + تصليب تطبيق الصلاحيات في الواجهة (بالأولوية) + قاعدة «الإعداد ياخد صلاحيته».
**Date started:** 2026-07-13
**Install:** moonui2 · BE `/home/moonui2/moon-erp-be` (branch `hazemdev2`) · FE `/home/moonui2/public_html/moon-erp` · Dev DB `moonui2_dev_be` (⛔ never fresh/wipe).
**Source plan (approved):** `knowledge-base/plans/permissions-screen-redesign-analysis.html`
**Companion analysis:** `knowledge-base/plans/convert-buttons-permission-analysis.html` (convert-button gaps fold into WP4b/WP4c).

## 🎯 GOAL (owner acceptance test — do NOT stop until met)
> النظام كله شغّال صح: (1) شاشة `/core/roles` الجديدة (مصفوفة مورد×إجراء + info لكل صلاحية + coverage lint + قوالب + وضع «حسب الشاشة» + شارة «API فقط» للـ144) تظهر وتُحفظ صح على `/app` الحيّ؛ (2) كتالوج وصف الصلاحيات يخدم الشاشة عبر `GET /core/permissions`؛ (3) البوابات الخشنة أُنزلت لمستوى الإجراء بالترتيب: محاسبة ← مبيعات ← مشتريات ← مخزون ← إنتاج ← HR ← الباقي (الخطير/المالي أولًا داخل كل موديول) — وما لم يُغطَّ **مُسجَّل** لا مُدّعى اكتماله؛ (4) قاعدة «الإعداد ياخد صلاحيته» مطبّقة (عمود `permission` على `setting_definitions` يُفحَص لكل إعداد) + سدّ فجوة `*.settings.manage` الثمانية؛ (5) باگ `hrm.training` مُصلَّح. كل WP: Codex + (Fable على [FIN]/محوري) + اختبار أخضر مقابل الأساس + build أخضر + نشر `/app` بعد أي FE. لا merge/main (المالك بيعمل `/fullpush`). الاكتمال يُؤكَّد بالتشغيل/الاختبار.

## Decisions (approved — do NOT re-litigate)
1. **مصدر الوصف = كتالوج BE** يعمّم `LisScreenCatalog` (`label_ar/en, desc_ar/en, danger, kind, screen`)، مع fallback FE للـi18n ثم الاسم المركّب.
2. **ترتيب WP4** = محاسبة(مالي) ← مبيعات ← مشتريات ← مخزون ← إنتاج ← HR ← الباقي (LIS/core/pos/clinic). داخل كل موديول: الخطير/المالي أولًا (delete/approve/post/convert). مرحلي؛ سجّل ما لم يُغطَّ.
3. **الـ144 «بلا واجهة»** تظل قابلة للمنح بشارة «API فقط — لا شاشة بعد» (لا تُخفى).
4. **قاعدة الإعدادات** = عمود `permission` على `setting_definitions` يُفحَص لكل إعداد + سدّ فجوة `*.settings.manage` الثمانية (accounting, inventory, crm, nphies, einvoicing, pos, production, clinic) + توثيق العُرف.
5. **طرح المصفوفة** = استبدال كامل للقائمة المسطّحة، مع الحفاظ على كل الموجود (القوالب/الاسم/النطاق/الصفحة الرئيسية/تبويب المنيوهات/module-activation).

## WP table
| WP | Scope (dense) | Repo | Depends | Review | Migration? | Status | Commits |
|----|---------------|------|---------|--------|-----------|--------|---------|
| WP1 | 🔄 كتالوج الصلاحيات BE: **PHP `PermissionCatalog` class** (يعمّم LisScreenCatalog، لا migration — أقل خطرًا) يوفّر label/desc/danger/kind/resource/screen لكل صلاحية؛ يبدأ محاسبة+مبيعات+مشتريات+core؛ إثراء `GET /core/permissions` بدمج الميتاداتا. Fallback للصلاحيات غير الموصوفة. | BE | — | Codex+Fable | no (PHP catalog) | ✅ done | `0a587a82`+`671aae0d6` · new test 5/5, re-run 71/0 · Codex clean · Fable 6 danger+1 soften applied |
| WP2 | 🔄 إصلاحات دقيقة: تصحيح بوابة `hrm.training` المعلّقة (FE) + إضافة `<module>.settings.manage` للـ8 موديولات الناقصة (BE seeder). | BE+FE | — | Codex | no (seeder) | ✅ done | BE `dc54c0db6` (8× settings.view+manage) · FE `9c3a36749` (hrm.training gate) · 13 tests green · Codex clean · seeded to dev · /app deployed |
| WP5 | 🔄 قاعدة الإعدادات: عمود `permission` على `setting_definitions` + فحص لكل إعداد في مسار الكتابة + توثيق العُرف. [FIN-schema] | BE | WP2 | Codex+Fable | yes | ✅ done | `148dbf626` + fixes `c614c7522` · migration on dev (fwd-only) · SettingApi 13/13, Enforcement 15/15, 36 passed · Fable APPROVE+3 · code-review 2 IMPORTANT → all fixed |
| WP3 | إعادة تصميم `/core/roles` (FE): مصفوفة مورد×إجراء + شريط وحدات + بحث/فلاتر + قوالب + info popover + coverage lint + وضع «حسب الشاشة» + شارة «API فقط». يستهلك كتالوج WP1 + fallback. استبدال كامل. | FE | WP1 | Codex+Fable | — | ✅ done | `8206fc4b8` + fixes `6cb47566d` · build green · Codex 6 findings + Fable CHANGES-NEEDED → all 13 applied · /app deployed · CHANGELOG `00f118521` |
| WP4a | تصليب محاسبة | FE | WP1 | Codex | — | ✅ done | `3d36af34f` · 92 gates, **0 dead** (verified by comm diff) · 16 screens/~50 controls · 2 pre-existing WRONG gates fixed · build green · /app deployed |
| WP4b | تصليب مبيعات (+ أزرار التحويل) | FE | WP1 | Codex | — | ✅ done | `5f355491` · 21 new gates, 45 total, **0 dead** (verified) · **الشكوى الأصلية اتحلّت: زرار التحويل مبوّب على sales.orders.create** · 2 asymmetries fixed · /app deployed |
| WP4c | تصليب مشتريات (+ أزرار التحويل) | FE | WP1 | Codex | — | ✅ done | `c268cc80f` · 14 new gates, 40 total, **0 dead** · asymmetry fixed in ALL 4 approval docs · 1 too-permissive gate narrowed · /app deployed |
| WP4d | تصليب مخزون + **`appCanAll` (طبقة مشتركة)** | FE | WP1 | Codex | — | ✅ done | `44fe2fb2e` · 8 new gates + appCanAll directive on 3 chained controls · 452 gates app-wide, **0 dead** · /app deployed |
| WP4e | تصليب إنتاج + إصلاح الرجعة | FE | WP1 | Codex | — | ✅ done | `8d5953ac0` · 43 production gates, **0 dead** · 4 wrong/asymmetric gates fixed · returns regression fixed in BOTH sales+purchases · /app deployed |
| WP4f | تصليب الموارد البشرية | FE | WP1 | Codex | — | ✅ done | `9bafcbd1b` · 75 HR gates, **0 dead** · 4 wrong gates fixed (2 MONEY) · **+ BE security hole closed `2278a6f0f`** · /app deployed |
| WP4g | تصليب الباقي (LIS/core/POS/عيادة) | FE | WP1 | Codex | — | ✅ done | `577a41e48` + `e42ca635e` · 209 gate sites · **523 gates app-wide, 0 dead** · privilege leak + wrong gate fixed · pricing-matrix readonly · /app deployed |

## DAG / serialization
- **BE writers (serialize):** WP1 → WP2(BE part) → WP5. One writer per repo at a time.
- **FE writers (serialize):** WP2(FE nav fix) → WP3 → WP4a → … → WP4g. Never two FE sub-agents concurrently.
- BE and FE WPs may interleave but each repo has one writer at a time.
- WP3 & WP4* depend on WP1 (catalog endpoint shape for WP3; WP4 only needs the FE gate patterns — independent of WP1 in practice, but sequenced after to keep one FE writer).

## Baseline (pre-existing test state — recorded 2026-07-13)
Ran (sqlite): RoleApiTest, RoleLifecycleTest, AdditiveRolePermissionsTest, Wp8aActionPermissionsTest, SettingApiTest, SetupPermissionTest → **8 failed / 87 passed**. Pre-existing failures (NOT ours — do not blame):
1. Tenant-scoping family (pre-existing, verified against stashed baseline): `RoleLifecycleTest::C2`, `RoleTenantScopingTest` ×2 (`lab role index hides another companys roles`, `assigning another companys role to a new user is rejected`). Also `AdminApiTest > import test data auto-populates sales account settings by code` (50 !== 5).
2. `SettingApiTest` ×7 (can list/filter definitions, list/get/update settings, idempotent) — ALL fail with `UniqueConstraintViolationException` on `setting_definitions.setting_key`: **`accounting.ar_parent_account` is seeded TWICE in `SettingDefinitionSeeder`**. → WP5 touches this seeder; fix the duplicate as part of WP5 (these 7 should go GREEN after WP5).
- FE: `ng build` green pre-start → **CONFIRMED green 2026-07-13** (only benign html2canvas CommonJS warning).

## Deferrals (discovered mid-flight)
- ✅ RESOLVED in WP4d: `appCanAll` directive added (`shared/directives/can.directive.ts` + `PermissionService.hasAllPermissions`, super-admin bypass preserved), applied to the 3 chained controls.
- ✅ RESOLVED in WP4e (regression fixed in BOTH modules — Sales had the same bug in the OPPOSITE direction: its quick-process was gated on `.post` only for ALL statuses, so a post-only user could click it on a DRAFT row and strand the return at a 403). ~~[REGRESSION]~~ Purchases-returns AUTO path (no workflow): an already-`approved` return now HIDES the quick-process button from a `.post`-only user, because the all-of gate demands `.approve` too. The workflow branch has a dedicated post-only button; the auto branch does not. Fix: add a post-only fallback button in the auto branch (`@if (r.status === 'approved')` → `*appCan="'purchases.returns.post'"`).
- ~~[SHARED LAYER] `*appCan` is ANY-OF only~~ Two Purchases controls genuinely need all-of semantics: Returns quick-process (chains submit→approve→post, needs `.approve` AND `.post`) and Bills inline Pay-Now (needs `payments.create` AND `payments.post`). Both are currently gated on the terminal/primary permission only, so a user holding one but not the other still sees the button and 403s. Fix = add an `appCanAll` directive next to `can.directive.ts` (ONE shared-layer fix, per the house rule) and use it for chained actions. Same pattern likely exists in Sales (returns Save&Process).
- **[Follow-up] 3 pre-existing DEAD permissions remain** (grantable, enforced by nothing, no settings to gate): `qms.settings.manage`, `cmms.settings.manage`, `webstore.settings.view`. Same disease this feature cured elsewhere; recommend deleting them from the catalog.
- **[BE gap, outside FE scope] Purchase order & bill "Duplicate" buttons 404 for everyone** — the FE calls `POST /purchases/orders/{id}/duplicate` and `POST /purchases/bills/{id}/duplicate`, but neither route exists in `Modules/Purchases/routes/api.php`. Buttons were gated on the respective `.create` (correct if/when the BE route lands), but the routes are missing.
- **[BE-only, no FE control] Purchases permissions with no button:** `purchases.orders.manage` (BE 'send order to supplier' — no Send button exists in the FE), `purchases.grns.update` (no GRN edit button), `purchases.payments.update` (no payment edit button), `purchases.settings.view|manage` (Purchases settings redirects to /core/settings).
- **[FE, queued after WP4a] Setup wizard swallows 403s.** `features/setup/setup-wizard.component.ts:897` writes settings with `.pipe(catchError(() => of(null)))` — a permission 403 is silently discarded and the wizard reports SUCCESS while the GL mapping never saves. Found during WP5 review. Mitigated in practice by the WP5 additive backfill (admin now holds the perm), but the swallow is a latent bug for any genuinely-unauthorized user. Fix: surface the error / disable the fields.
- **[WP4 coverage] ~420 coarse action gates total.** WP4a-g close them by module in the owner's priority order, danger/financial first. Each sub-WP logs what it did NOT gate here rather than claiming completeness.

## Progress log
- 2026-07-13: Phase A started — workspace + LEDGER created; baseline running.

- 2026-07-13: WP1 🔄 started (PermissionCatalog + enrich /core/permissions).
- 2026-07-13: WP1 ✅ done. PermissionCatalog (326 keys, Core/Acc/Sales/Purch) + enriched /core/permissions (backward-compat). Codex: clean/Low. Fable [FIN]: 6 danger overrides (fx-revaluation.run, transfers.create, petty-cash.create, ar-ap.create, checks-issued/received.change-status, opening-balances.confirm) + softened sales.quotations.reject — applied & tested (5/5). Baseline preserved (71/0 on re-run files). SHAs 0a587a82, 671aae0d6.
- 2026-07-13: WP2 🔄 started (hrm.training gate fix + 8× settings.manage).
- 2026-07-13: WP2 ✅ done. hrm.training menu retargeted to real training .view perms (both nav sources); 8× <module>.settings.view+manage seeded (accounting/inventory/crm/nphies/einvoicing/pos/production/clinic) — additive, seeded into moonui2_dev_be. 13 BE tests green; FE build green; /app deployed. Codex clean (5/5). SHAs BE dc54c0db6, FE 9c3a36749.
- 2026-07-13: WP5 🔄 started (setting_definitions.permission column + de-dupe seeder + per-setting enforcement).
- 2026-07-13: WP5 implemented (`148dbf626`) — awaiting review gate. **Baseline correction (deviation, recorded):** the brief's stated bug was WRONG — `accounting.ar_parent_account` is NOT duplicated in SettingDefinitionSeeder and run() already used updateOrCreate. Real cause: the moonstack bridge migration (2026_06_17_220000) runs SettingDefinitionSeeder DURING `migrate`, so the key exists when RefreshDatabase hands the test a DB; the TEST's seedDefinitions() used create() -> UNIQUE violation. Fix landed in the TEST. Result: SettingApiTest 7 failed -> 13/13 GREEN; new SettingPermissionEnforcementTest 10/10. Migration 2026_07_13_100000 ran forward-only on moonui2_dev_be (no fresh/wipe).
  - [FIN] calls flagged for review: (1) declared permission REPLACES core.settings.manage (narrows, not ORs) -> required moving update/updateBulk OFF route middleware; (2) ensureDefaultAccounts still on plain core.settings.manage (possible GL bypass); (3) seeder/column ordering hazard on client-update path (Schema::hasColumn guard).
  - Newly-noted pre-existing failure (NOT ours, verified on stashed baseline): `AdminApiTest > import test data auto-populates sales account settings by code` (50 !== 5).
- 2026-07-13: WP3 implemented (`8206fc4b8`) — build green; awaiting review gate. Honest partials: by-screen mode is resource-shaped not screen-shaped (no BE screen catalog); coverage-lint uses a 13-rule static map only.
  - Fable design review: CHANGES-NEEDED. **Safety bug: presets in global/search view apply across ALL 15 modules** (one 'Full' click = system-wide grant). Also: use PrimeNG native [indeterminate] (exists) instead of CSS hack; strengthen danger styling; green coverage strip lies when no deps evaluated -> gate it; td.res display:flex breaks table-cell; sticky headers; a11y ariaLabels; hardcoded Arabic comma; dead code from old accordion.

- 2026-07-13: WP3 ✅ done. Roles screen = resource×action matrix + per-permission info + danger + presets + coverage-lint + API-only badge + by-screen. Reviews: Codex (6 findings) + Fable (CHANGES-NEEDED) — BOTH independently caught the **preset blast-radius safety bug** (presets applied across all 15 modules in global/search view). All 13 findings fixed in `6cb47566d`: preset scoped to active module + hidden in global view; module tri-state/count now one scope; "Full" preset genuinely full (incl. delete, red pill); labels recompute on lang switch; native PrimeNG [indeterminate]; stronger danger styling + dark override; coverage green gated on >=1 evaluated dep (no more lying green); td.res table-cell fix; sticky headers; a11y ariaLabels; catalog danger?? fallback; locale comma; dead code removed. Invariant verified: selectedPermissions Set → save payload byte-for-byte unchanged. Build green, /app DEPLOYED (owner can test live). SHAs FE 8206fc4b8, 6cb47566d; CHANGELOG 00f118521.
- ⚠️ TOOLING (recorded): **Codex sandbox is unavailable on this host** — `bwrap` fails (`max_user_namespaces=0`; the ENOSPC is a misleading errno). It worked for WP1/WP2/WP3 then failed for WP5. Documented fallback in use: an Opus `code-reviewer` agent stands in for Codex when it fails.

- 2026-07-13: WP4a ✅ Accounting. 92 action gates, ZERO dead (independently re-verified: comm diff of FE gates vs BE catalog = empty). **Key finding propagated to the WP4 method brief: the required permission is declared in the BE CONTROLLER MIDDLEWARE, not derivable from the action name** (dispose→.update, budget approve→.update, year-end reopen→.execute, fiscal-year delete→.create...). Deriving from naming alone would have created dead gates. Also fixed 2 pre-existing WRONG gates (button shown → 403): accounts import-template (needs .create AND .delete, it truncates) and opening-balances add-invoice (BE puts addInvoice under .create).
  - WP4a deferrals (BE-only, no FE control exists to gate): zakat.*, withholding-tax.*, fx-revaluation.*, integrity.view, accounting.settings.* (no FE screen at all); receipt/payment-vouchers.approve (no approve button in FE); bank-reconciliations.unmatch (no button); fiscal-years.close (no button); periods.reopen (no button). Left ungated deliberately: fiscal-years EDIT button (no .update perm exists AND BE registers apiResource except('update') → the button looks already-dead/broken — flagged as a separate likely bug); ledger/cash-movement (read-only reports); ar-ap generate-statement (read-only).

- 2026-07-13: WP5 ✅ done. Per-setting permission rule LIVE. Reviews: Fable [FIN] APPROVE + 3 hardening; code-reviewer (Codex fallback) found 2 IMPORTANT — the big one: **the stock `admin` role was silently locked out of the 9 GL settings** (built by a `core.*`/`*.view` prefix filter, so it holds core.settings.manage but not accounting.settings.manage) AND the Setup wizard swallows 403s (`catchError(() => of(null))`) → it would report SUCCESS while the GL mapping never saved. Fixed per this repo's no-regression convention: additive backfill grants `accounting.settings.manage` to every role holding `core.settings.manage` (custom roles included), PLUS an explicit grant in createRoles() so a FRESH install's admin isn't locked out either (the implementer caught that the backfill alone misses fresh installs — runAdditive() never runs there). Also: ensureDefaultAccounts now AND-gated on both perms (it was writing 5 of the 9 gated keys + creating GL accounts with no accounting perm); gate reads the SETTING_PERMISSIONS const first (fail-effective, not fail-open); migration firstOrCreates the Permission row (else the gate activates with nobody able to pass it); updateBulk bounded max:200 + N+1 killed; 403 message moved to lang files. NEW TEST asserts the REAL seeded roles: owner ✅ admin ✅ accountant ✅ employee ❌ — this is what stops a future prefix-filter change from silently locking a role out again. 36 tests passed. SHAs 148dbf626, c614c7522.

- 2026-07-13: WP4b ✅ Sales. 21 new gates (45 total in features/sales), ZERO dead (independently verified). **THE OWNER'S ORIGINAL COMPLAINT IS FIXED**: quotations.component.html:146 `*appCan="'sales.orders.create'"` on the blue-arrow convertToOrder — removing sales.orders.create from a user now HIDES the button (BE already 403'd; the gap was purely FE). All 5 convert/create buttons gated on the TARGET doc's .create, each confirmed against the BE controller middleware: q->order=sales.orders.create, order->delivery=sales.delivery_notes.create (underscore!), order->invoice / invoice remaining-delivery / close-short = sales.invoices.create (the last two live in SalesInvoiceController — a naive gate would have been WRONG/dead; the method finding paid off).
  - 2 asymmetries fixed: invoices had APPROVE gated but REJECT ungated (both list + view dialog) — same BE middleware, so a user without the perm saw a Reject that 403s. Same for returns submit-for-approval.
  - Deferrals: dialog footer save buttons left ungated (each dialog is only reachable via an already-gated opener; create-vs-update perm differs by dialog mode → needs a mode-aware expression, not a static string). View/print/export = read-only, route-gated. `sales.reports.view` is the ONLY sales permission with no action button (route-gated only — correct). No orphan sales permissions.

- 2026-07-13: WP4c ✅ Purchases. 14 new gates (40 total), ZERO dead (verified). Convert buttons BE-confirmed: order->GRN=purchases.grns.create, order->bill & GRN->bill=purchases.bills.create, bill->payment=purchases.payments.create. **request->order correctly uses `purchases.requests.convert` NOT `purchases.orders.create`** — the catalog-guess would have been WRONG here; the BE middleware puts convertFromRequest on requests.convert. (Third time the method finding saved a wrong gate.)
  - **ASYMMETRY BUG confirmed in ALL FOUR approval documents** (requests, orders, bills, returns): approve/reject gated but SUBMIT-FOR-APPROVAL ungated, though the BE puts submitApproval on the same middleware. Fixed in all four.
  - **WRONG (too-permissive) gate fixed:** New Bill was `*appCan="['purchases.bills.create','purchases.bills.create_direct']"` — but *appCan is ANY-OF and the BE always requires bills.create (create_direct is an extra in-body check). A create_direct-only user saw the button and 403'd. Narrowed to bills.create.
  - Payments/landed-cost/supplier-prices needed no changes — already correctly gated (landed-cost delete correctly uses landed_cost.create, matching its middleware).

- 2026-07-13: WP4d ✅ Inventory + shared `appCanAll`. Directive added ONCE in the shared layer (house rule), applied to 3 chained controls whose all-of semantics were previously wrong (Purchases returns quick-process, Bills inline Pay-Now, Sales returns Save&Process — each needed BOTH perms; any-of let a half-permissioned user strand the doc mid-chain). Inventory: 8 new gates; the rest were already correctly gated. **No asymmetry bug in Inventory** (BE has no submit/reject endpoints there). App-wide verification: 452 gate strings, ZERO dead.
  - Naming trap caught again: inventory-counts "Confirm adjustment" calls the ADJUSTMENTS controller → needs `inventory.adjustments.approve`, not a counts perm.
  - **BE gaps found (FE calls endpoints that DON'T EXIST):** `StockAdjustmentService.update()/delete()` (routes expose only index/store/show/approve/cancel) and `OpeningBalanceInventoryService.approve()/update()/delete()` (routes expose only index/show/store/bulk/cancel). Those 6 buttons cannot work today. Gated on each resource's real `.create` perm (no dead gate) and flagged for a BE decision.
  - Deferred: counts "quick mode" chain (fires from the shared form-dialog Save; quick mode is a BE setting not a user choice — gating New Count with all-of would be MORE restrictive than the BE). Recommend a BE-side/dialog-level fix.
  - No-FE-control perms: inventory.reorder.notify, inventory.counts.cancel, inventory.costing.*, inventory.settings.*, inventory.receipts.create_manual.

- 2026-07-13: WP4e ✅ Production (+ regression fix). Root cause of the returns issue: `quickProcess()` BRANCHES ON STATUS (draft→submit+approve+post, pending→approve+post, approved→post only) — so one all-of gate over the whole button was wrong for the `approved` row. Split per-status in BOTH Purchases and Sales. **Sales had the mirror bug** (post-only gate on all statuses → post-only user strands a draft).
  - Production: 43 gates, ZERO dead. **4 pre-existing WRONG/ASYMMETRIC gates fixed:** (1) staging bulk-stage was on `orders.update` but BE requires `production.consume` (403); (2) toll create-order-from-line was on `orders.create` but BE requires `toll_contracts.create` (classic derive-from-name error); (3) QC asymmetry — qcRelease/qcReject gated, **qcRequest UNGATED** on the same middleware; (4) recall used any-of but BE stacks BOTH middlewares → now appCanAll.
  - Naming traps caught: calendars store/update/destroy → `production.centers.update` (no `calendars.*` perm exists); MPS approve → `production.mrp.run` (no `.approve`); receiveFg → `orders.update` (not `.complete`).
  - **DEAD CATALOG ENTRIES found:** `production.quality.inspect` + `production.quality.disposition` are seeded but NO BE middleware uses them and no FE control exists — likely superseded by `production.qc.release`. Recommend removing from the catalog (they are grantable but do nothing anywhere).
  - Unreachable perm: `production.orders.start` — the FE only reaches start via ShopFloorController (gated `shopfloor.operate`), never via ProductionOrderController@start.
  - Deferred: `variances` (BE gates index AND updateStatus on the same `production.costing` → anyone who can load the page already holds the action perm; also the actions are TS-built MenuItem[] which *appCan can't reach).

- 2026-07-13: WP4f ✅ HR. 75 gates, ZERO dead. **4 pre-existing WRONG gates fixed, 2 of them MONEY:** payroll-adjustment pay-out was gated `hrm.payroll.pay` but the BE requires `hrm.payroll.create` (PayrollAdjustmentController maps store/update/destroy/payOut → .create) — so the button was shown to people who'd 403 AND hidden from the people who could actually do it (both screens). Attendance check-out was on `.update` while check-in was on `.create` — same BE perm (asymmetry); request-correction likewise wrong.
- 2026-07-13: 🔒 **BE SECURITY HOLE FOUND & CLOSED (`2278a6f0f`) [FIN].** `PayrollAdjustmentController::approve` and `::reject` were ROUTED but appeared in NO middleware() entry — **any authenticated user could approve/reject a payroll adjustment** (a money action with zero permission check). Now gated on `hrm.payroll.approve` (consistent with PayrollController's own approve). New `PayrollAdjustmentPermissionTest` 4/4 proves deny-without / allow-with. NOTE the test trap: `approve(Payroll $payroll,...)` route-model-binds and SubstituteBindings runs BEFORE the permission middleware, so a dummy id 404s and the deny-test would pass for the WRONG reason — the test creates a REAL Payroll row (same trap documented in Wp8aActionPermissionsTest). CHANGELOG entry added.
  - HR deferrals / dead ends found: MANY FE buttons call BE routes that DON'T EXIST (loans edit/delete/reject, EOS calculate/edit/delete/cancel, performance-reviews edit/delete/submit/approve/complete, attendance CRUD, offers edit/delete, application delete, training-session delete) — `features/hr/CLAUDE.md` documents them as if they exist; that doc is WRONG. Left ungated (no BE route, no middleware, no catalog perm → any gate would be invented or dead).
  - HR perms with NO FE control: `hrm.attendance.approve` (BE has approve-correction, FE has NO approve UI — corrections can be requested and never approved: real functional gap), `hrm.leave-requests.cancel`, `hrm.loans.cancel`, `hrm.punch-types.*` (full BE CRUD, no FE screen), `hrm.employees.import`. Orphan catalog perms (checked by NO controller): `hrm.performance-reviews.manage`, `hrm.attendance.update`, `hrm.applications.stage`.

- 2026-07-13: WP4g ✅ LIS/CORE/POS/CLINIC (final module pass). 76 files. **App-wide verification: 523 gate strings, ZERO dead** (my first check reported 6 false positives — my regex truncated 4-segment perms like `lis.compliance.checklists.delete`; the TOOLING was wrong, not the gates. Re-verified with full-string + prefix/namespace-aware matching).
  - **PRIVILEGE LEAK fixed:** dept-worklist Re-run button was guarded on `lis.results.enter` but `retestRow()` actually requires `lis.results.retest` — an entry-level tech could trigger a retest.
  - **WRONG GATE fixed:** lis-packages B2B Partners button gated on `lis.packages.update` but the dialog hits LabPackagePartnerController → `lis.external-labs.packages.manage` (403 for users who saw it).
  - Trap avoided: there is NO `lis.cashier.*` permission — cashier shift open/close run on `lis.payments.create`. The "obvious" name would have been a dead gate hiding the shift widget from everyone.
  - CORE: gallery image delete traced to AttachmentController → `core.attachments.delete` (not products). Setup ensureDefaultAccounts → `*appCanAll` (BE stacks TWO middleware = AND) — matches the WP5 fix.
  - POS: pay/cart-pay → `sales.invoices.create` (a POS sale posts a Sales invoice). No refund/void endpoint exists in POS at all.
  - **No unprotected BE endpoint found** in these modules (all 201 Clinic routes carry `permission:`).
- 2026-07-13: 🔒 **Clinic pricing-matrix fixed (`e42ca635e`).** The price cells are <input>s that debounce-save and had NO gate — a `clinic.pricing.view`-only user saw a fully EDITABLE price grid and every keystroke 403'd. Gated with `[readonly]` (NOT *appCan — hiding the cells would destroy the read view that .view legitimately grants), split by backing permission: contract cells → `clinic.payer.manage`, natural-axis → `clinic.pricing.manage`, + a save-handler guard (defence in depth). NOTE: my first guard keyed off `contract:` — the real key format is `contract_<id>` (buildColumns); caught before commit.

## Phase C — whole-branch review (2026-07-13)
**Verdict: BLOCK** (2 CRITICAL, 7 IMPORTANT, 4 MINOR). The reviewer's core insight: **the gates are correct wherever a button makes ONE backend call, and wrong wherever it makes MORE THAN ONE.** Every finding is an instance of that.
- **C1 CRITICAL — Setup wizard silently swallows setting-write 403s** (`setup-wizard.component.ts:899` `catchError(() => of(null))` + advances anyway). A role without `accounting.settings.manage` runs Setup → all 9 GL-key writes 403 → **wizard reports SUCCESS with zero GL mappings saved** → every later journal entry posts to the wrong account. (This is the deferral I logged at WP5 — the BE backfill mitigated it only for roles existing AT upgrade time; it cannot cover custom roles created later, nor an owner who deliberately narrows the permission, which is the feature's whole selling point.)
- **C2 CRITICAL — "Record payment" gated `.create` but handler chains create→post** (purchases bills + sales invoices, row + dialog = 4 controls). A clerk without `.post` creates an ORPHAN DRAFT payment, gets an error, retries → a SECOND orphan. The inline-pay section 4 lines below in the same file already had the `appCanAll` treatment — the dialog/row controls were missed.
- **I1** Sales inline-pay section completely UNGATED (Purchases' identical one is gated).
- **I2** HR schedules: ANY-OF gate over a non-atomic create+update+**delete** forkJoin fan-out → a `.create`-only user saves a roster edit and gets a PARTIALLY-APPLIED roster (no transaction).
- **I3** `profile-forms-manager` (shared component) gated 3 different + all wrong ways across its 3 hosts (any-of / core-settings-only / none). Needs ONE fix inside the shared component (requires BOTH `clinic.history.manage` AND `core.settings.manage`).
- **I4** Visit-queue primary action any-of over 3 perms; handler only ever calls `clinic.encounters.create` → a vitals-only nurse sees it and 403s on every click.
- **I5** `PermissionCatalog` emits `resource` as the **Arabic label**, and the FE uses it as the grouping KEY → the ENGLISH UI shows **Arabic row labels** for exactly the 4 cataloged modules.
- **I6** WP5's delegation goal is unreachable in the UI: the stock `accountant` holds no `core.settings.*` so it CANNOT OPEN /core/settings; and `accounting.settings` isn't in the catalog, so the keystone permission renders as a bare unlabelled key.
- **I7 (self-inflicted)** **15 of the 16 `<module>.settings.*` permissions I seeded are enforced by NOTHING** — grantable, visible, and inert. That is precisely the disease this feature exists to cure. Must be wired to real enforcement or removed.
- MINOR: dead HR controls (routes don't exist), roles Save button any-of across create/update modes, updateBulk all-or-nothing, bill/invoice auto-post chain toast.
**CLEAN (verified, nothing found):** appCanAll over-restriction (none — no gate hides a control from someone entitled); 15+ gate/BE spot-checks across all modules ALL correct (incl. every "the name lies" trap); the roles-screen selection invariant (Set is sound, save payload byte-identical); PermissionCatalog↔seeder zero drift; `GET /core/permissions` backward compatible; the WP5 upgrade path (GrantRolePermissionsSeeder is in moonstack.updater.seeders → runs on every fleet update); no over-broad prefix gates.
→ Two fix agents dispatched (BE: F1 dead settings perms, F2 resource_key, F3 accountant access + catalog entry, F4 bulk comment · FE: C1, C2, I1-I5, MINOR).

## Phase C — Fable [FIN] whole-feature review (2026-07-13) — VERDICT: CHANGES-NEEDED
Fable scanned all 1788 routed methods x their `middleware()` across 15 modules. It reviewed the LIVE repos (it correctly noted my diffs were stale). **All P0s independently re-verified by me.**
- **P0-1 [THE METHOD'S BLIND SPOT] — I made a money hole WIDER.** My standing rule was "trust the BE middleware, never the button name." But **the BE middleware ITSELF can be wrong**: `PayrollAdjustmentController.php:29` gates `payOut` (records that CASH LEFT the company) under `hrm.payroll.create` — the DATA-ENTRY permission. `hrm.payroll.pay` exists and every other pay action uses it. In WP4f I dutifully changed the FE buttons FROM `.pay` TO `.create` to match. Result: a deliberately-built "payroll data entry" role (view+create, no `.pay`) can create an adjustment AND mark it paid out — the exact separation of duties this feature exists to enforce. **Fix: correct the BE middleware (payOut → hrm.payroll.pay) AND revert the two FE buttons.** LESSON: the BE middleware is the source of truth for WHAT a button calls, but it is NOT automatically CORRECT — a money action under a data-entry permission is a BE bug, not a naming quirk.
- **P0-2 [CRITICAL, verified on this host] — `POST /api/hr/dev/reset-data` has NO permission middleware at all.** `HrDevController` doesn't even implement `HasMiddleware`. It truncates payrolls, payroll_items, payroll_adjustments, employee_loans, loan_installments, eos_settlements. Its ONLY guard is `app()->environment('production')` — **and `/home/moonui2/moon-erp-be/.env` has `APP_ENV=local`**, so on this host ANY authenticated user (an intern, a lab tech) can destroy every payroll record. LIS's equivalent rides `lis.requests.force_delete`; Core's `admin/reset-database` rides `core.admin.manage`. HRM's is the only naked one. Fix: `->middleware('permission:core.admin.manage')`.
- **P0-3 [MONEY LOSS] — two cashier lockouts, one loses cash.** (a) POS Return button now gated `sales.returns.create`, which the stock `cashier` role does NOT hold → the till cannot issue a refund at all. (b) **POS offline sync strands money:** `pos-offline.service.ts:316-329` replays a queued sale as `POST /sales/payments` then `POST /sales/payments/{id}/post`, but cashier lacks `sales.payments.post` → when the terminal reconnects, the payment row is created and **the post 403s: cash was collected at the till, no journal entry exists, the invoice still reads unpaid.** (The ONLINE path is fine — it uses the atomic `POST /pos/sales`.) Verified: cashier holds only sales.invoices.view/create + sales.payments.view/create.
- **P1-4** `POST /lis/requests/{r}/regenerate-invoice` ungated — cancels the live lab invoice and rebuilds it (wiping applied insurance/discounts). Sibling invoice-mutating actions all ride `lis.requests.update`.
- **P1-5** `LabSampleController::cancel/sendOut/recollect` ungated. **`sendOut` outsources tests to an external lab — it CREATES A PAYABLE.** And `lis.samples.send-external` already exists in the seeder and is used NOWHERE.
- **P1-6** `LabExternalLabReferralController::send/addTest/removeTest` ungated — addTest/removeTest change what the external lab BILLS YOU.
- **P2-7** `SalesCommissionRuleController::show` ungated (any authenticated user can read commission rates/tiers).
- **P2-8** My WP4a accounts import-template "fix" was WRONG in the other direction: I nested create+delete (ALL-OF) but the BE requires ONLY `accounting.accounts.delete` → a delete-only role is wrongly hidden from it.
**Fable verified CORRECT (no action):** Clinic is clean (every money route carries inline permission middleware); all Accounting money screens; the Setup wizard's appCanAll mirrors the BE's two-entry AND, with no fresh-install or fleet-update lockout; LIS cashier shift + commission settle/void; **no money chain is left ANY-OF**; all 533 FE gate strings resolve to real permissions (8 non-matches are intentional segment-aware prefix gates).

## ⚠️ BASELINE CORRECTION (honesty, 2026-07-13)
My Phase-A baseline sampled only 6 test files, so it under-counted pre-existing failures. The full Core suite surfaced **3 MORE pre-existing failures I had not recorded**: `ProductApiTest` barcode ×3 (`auto-generates barcode when setting is enabled`, `does not overwrite provided barcode`, `skips barcode generation when setting is disabled`).
**Verified NOT a regression from this feature:** the failure is `UniqueConstraintViolationException` on `setting_definitions.setting_key` for `core.barcode_auto_generate` — the SAME root cause as the 7 SettingApiTest failures WP5 fixed (the moonstack bridge migration seeds SettingDefinitionSeeder during `migrate`, so the test's `SettingDefinition::create()` hits a UNIQUE violation). Proven pre-existing: BOTH the seeder definition AND the test's `create()` calls exist at `0a587a82~1` (the commit before this feature began).
→ Will fix (same one-line family: create() → updateOrCreate()) in the round-2 fix pass.
**Corrected baseline = 7 pre-existing failures:** tenant-scoping ×3 (RoleLifecycleTest::C2, RoleTenantScopingTest ×2), AdminApiTest import-test-data ×1, ProductApiTest barcode ×3.
**Suite results so far:** HRM **239 passed / 0 failed** (incl. my 4 new payroll-security tests). Core: only the 4 known pre-existing (AdminApiTest ×1 + ProductApiTest ×3) in the portion that ran.

## Phase C — fix round 1 (2026-07-13)
- **FE `0631f1864`** — C1: Setup wizard no longer swallows setting-write 403s (collects failing keys, toasts them, does NOT advance; the step now disables the account selects + shows an up-front warning when the user lacks the permission, and offers Skip instead of a save-that-silently-fails). C2/I1: all 4 record-payment controls + the sales inline-pay section → `*appCanAll` [create, post]. I2: HR schedules → appCanAll [create, update, delete] (the forkJoin fan-out included a DELETE that wasn't even in the gate). I3: profile-forms-manager gated INSIDE the shared component (house rule) — with a reasoned pushback I accepted: it gates the two sections INDEPENDENTLY rather than all-of, because they call separate endpoints, so no control can 403 either way and a core-settings admin keeps the field-config they're entitled to. I4: visit-queue → `clinic.encounters.create` alone. I5: matrix now groups by `resource_key`, never by a label. MINOR: roles Save button gate is now mode-dependent.
- **BE `b53e8be26`** — F1 (the self-inflicted one): the 15 dead `<module>.settings.*` perms are now REAL. Mechanism is a per-MODULE rule (`SettingDefinitionSeeder::MODULE_SETTING_PERMISSIONS` + `permissionFor()`), not a hand-listed key map, so a setting added tomorrow is gated automatically instead of silently shipping under the broad core gate; `SettingDefinition::booted()` stamps the permission column for ALL SEVEN seeders that write that table (only the Core one knew the column existed) — a shared-layer fix. `authorizeSettingRead()` added so the `.view` half gates something too. **KEPT** (wired to real settings): accounting 14 defs, inventory 15, pos 6, production 26, clinic 22, nphies 1, einvoicing 1. **REMOVED**: `crm.settings.*` — CRM ships ZERO setting definitions, so rather than ship a dead permission it is deleted from the catalog + a forward-only migration drops the stale rows. No-regression on BOTH paths (additive backfill for upgrades/custom roles; explicit grant in createRoles for fresh installs). F2: catalog emits `resource_key`/`resource_ar`/`resource_en`. F3: stock `accountant` gets `core.settings.view` (WP5's delegation goal was unreachable without it); `accounting.settings` cataloged as danger. 52 passed / 2282 assertions.
  - Flagged pre-existing dead perms NOT touched (same disease, out of scope): `qms.settings.manage`, `cmms.settings.manage`, `webstore.settings.view` — no definitions, no enforcement site. → Deferral.

## Phase C — fix round 2, FE (`72c41b9`)
- **F1** payroll pay-out buttons reverted to `hrm.payroll.pay` (separation of duties restored; add/delete stay on `.create`, which is correct).
- **F2 [MONEY] POS offline sync fixed structurally.** The replay now calls the SAME `PosService.processSale()` the ONLINE path calls (atomic `POST /pos/sales`, gated `sales.invoices.create` which the cashier HAS) — so it cannot drift again. No new permission needed. The old hand-rolled replay (`POST /sales/payments` + `/post`) is gone.
  **It also killed 3 LATENT BUGS that were silently corrupting every offline sale:** (1) it dropped the `credit_card → card` method mapping, and the BE validates `in:cash,card,credit,check` — so **EVERY offline CARD sale would have 422'd**; (2) it dropped `notes`/`discount_percent`/`tax_rate`; (3) it defaulted `customer_id` to 1 instead of the online path's `pos_default_customer_id` (2). These were pre-existing and unrelated to permissions — found only because the permission gate forced us to read the replay path.
- **F3** CoA import-template: my WP4a "fix" had over-gated it (nested create+delete = ALL-OF) but the BE requires ONLY `accounting.accounts.delete` → collapsed. A delete-only role was wrongly hidden from it.
- Dead-gate check: 517 FE gates vs 1041 BE perms → empty except the intentional `clinic.payments.` segment-prefix gate. **Tooling trap recorded:** the naive BE-side regex `'[a-z0-9_]+\.[a-z0-9_.]+'` EXCLUDES HYPHENS and reports ~170 false dead gates (`core.unit-groups.create`). Any automated check must include `-`.

## Phase C — fix round 2, BE (`f617b3c53`) [FIN]
The round-2 BE agent completed the work but DIED before committing (it stalled waiting on its own test run). I verified its staged work myself, ran the tests (24/24 passed), reverted 18 files of unrelated Pint churn I had accidentally caused by running Pint on whole directories, and committed.
- **S1 [separation of duties] `payOut` → `hrm.payroll.pay`.** It rode `hrm.payroll.create` (data entry) while RECORDING THAT CASH LEFT THE COMPANY.
- **S2 [CRITICAL] `POST /hr/dev/reset-data` → `core.admin.manage`.** `HrDevController` didn't implement `HasMiddleware` at all; it truncates payrolls/items/adjustments/loans/installments/EOS-settlements, guarded ONLY by `env != production` — and this host is `APP_ENV=local`. Any authenticated user could destroy every payroll record.
- **S3 [money] `LabSampleController::sendOut` → `lis.samples.send-external`** — it outsources tests to an external lab, i.e. CREATES A PAYABLE. **[CORRECTION]** I first claimed this permission 'existed and was used NOWHERE'. That is WRONG — it IS used: `LabExternalSampleController:27` gates 5 methods on it (sendExternal/markSent/sendOut/splitSendOut/resultSendOut). The hole was still real (`LabSampleController::sendOut` was genuinely ungated), and gating it here makes it CONSISTENT with its sibling controller rather than novel. I repeated Fable's claim without verifying it. `cancel`/`recollect` → `lis.samples.create`.
- **S4 [money] `LabExternalLabReferralController::send/addTest/removeTest` → `lis.referrals.create`** — addTest/removeTest change what the external lab BILLS YOU.
- **S5 `LabRequestController::regenerateInvoice` → `lis.requests.update`** — it cancels the live lab invoice and rebuilds it, wiping applied insurance/discounts.
- **S6 `SalesCommissionRuleController::show` → `sales.commissions.view`** (commission rates/tiers were readable by any authenticated user).
- **S7 [cashier lockout] cashier granted `sales.returns.create` + `.view`** — enough to RAISE a till refund, deliberately NOT `.approve`/`.post` (separation of duties: cashier raises, supervisor approves). Fresh installs via `createRoles()`; existing installs via an additive backfill. Tests assert both paths.
- **S8** ProductApiTest barcode ×3 → `updateOrCreate` (the pre-existing UNIQUE violation). Green.
All 7 new gate strings verified present in the catalog. CHANGELOG: 22 bullets, all bilingual.

## Phase C — FINAL TEST RESULTS (clean tree, no agents running)
- **Core suite: 435 passed / 7 failed.** **HRM suite: 239 passed / 0 failed.** FE `ng build`: green. `/app` deployed.
- **ZERO new failures.** All 7 Core failures are PRE-EXISTING, each with a root cause that predates this feature (my Phase-A baseline sampled only 6 files and so under-counted them — corrected here):
  1. `AdminApiTest > import test data auto-populates sales account settings by code` (50 !== 5) — account-code/ID issue.
  2-4. Tenant-scoping family ×3 (`RoleLifecycleTest::C2`, `RoleTenantScopingTest` ×2).
  5. `SettingsServiceTest > set method validates allowed values for enum type` — `SettingsService::set()` was deliberately changed to throw `ValidationException` (documented `@throws`) by an earlier commit; the test still expects `InvalidArgumentException`. **PROVEN pre-existing: it fails on pre-feature source too** (verified by reverting Modules/ to `0a587a82~1` and re-running).
  6. `UserApiTest > can create a user with role` — `branch_ids` became `required` in commit `d60c2783a` (multi-branch users); the test never sends it. My feature never touched `StoreUserRequest`/`UserController`.
  7. `SessionApiTest > login captures ip and user_agent` — the test creates its user with NO `company_id`, and `AuthController:82` filters logins to users whose company is active → 403. A test-data bug.
  **Proof of non-involvement:** `git log --name-only 0a587a82~1..HEAD | grep -iE "auth|user|settingsservice"` → EMPTY. This feature touched none of those files.
- **FIXED by this feature: `ProductApiTest` barcode ×3** (the pre-existing UNIQUE violation) — they are now green.
- ⚠️ Process note: while proving #5 I ran `git checkout <old> -- Modules/` and the command TIMED OUT before its restore step, briefly leaving the tree reverted. Restored immediately (`git checkout HEAD -- Modules/`, 0 dirty files, all fixes verified back in place, HEAD `f617b3c53`). Lesson: never put a destructive revert and its restore in one time-boxed command.
- Recommendation (NOT done — out of the approved scope): the 3 stale tests (#5, #6, #7) are trivially fixable and would take the Core suite to green. Offer to the owner rather than silently expand scope.

## ⚠️ CORRECTION + additional pre-existing failures (2026-07-13, post-hoc)
- **CORRECTION:** I stated (in the ledger, the RESUME and to the owner) that `lis.samples.send-external` "existed in the catalog and was used nowhere". **That is false.** It is used by `LabExternalSampleController:27` (5 methods) and appears in `LisPermissionDependencies`. The security hole was still real — `LabSampleController::sendOut` was ungated — but my characterisation was wrong. I took Fable's claim at face value instead of grepping. Verified myself now.
- **15 MORE pre-existing failures exist on `hazemdev2`**, outside Core/HRM, proven pre-existing by the implementer stashing the ENTIRE changeset and re-running to get the identical set: `LabReportPdfParityTest` ×1, `LabRequestApiTest` ×3, `LabResultApiTest` ×2, `LisCatalogReplaceTest` ×2, `CogsAtDeliveryCancelTest` ×6 (RuntimeException), `SalesReturnApiTest` ×1 (validation). NOT caused by this feature; recorded so nobody attributes them to it later.
- Incidental: Pint's `no_unused_imports` stripped 6 genuinely-unused imports from `LabRequestController.php` (PostLabInvoice, RequestPriority, RequestSource, LabExternalLabPricing, LabInvestigation, LabPackage). Full LIS suite passes identically before/after — nothing referenced them.
- S7 detail confirmed: the POS refund path makes exactly ONE write call (`POST /sales/returns` → files a DRAFT credit note). Cashier got `sales.returns.create` + `.view` only; `.approve`/`.post` deliberately withheld and asserted false by test.