docs(14): re-verify phase 14 as passed after permission-split fix
This commit is contained in:
parent
00f196c115
commit
1dcceecfd9
2 changed files with 32 additions and 13 deletions
|
|
@ -1,36 +1,38 @@
|
|||
---
|
||||
status: partial
|
||||
status: resolved
|
||||
phase: 14-pax8-ui-surface
|
||||
source: [14-VERIFICATION.md]
|
||||
started: 2026-07-11T19:20:00Z
|
||||
updated: 2026-07-11T19:20:00Z
|
||||
updated: 2026-07-12T22:20:00Z
|
||||
---
|
||||
|
||||
## Current Test
|
||||
|
||||
[awaiting human testing]
|
||||
None — both items resolved.
|
||||
|
||||
## Tests
|
||||
|
||||
### 1. Re-confirm SC#4 (resolve + persist-through-sync) against the live dev DB, now that CR-01 is fixed
|
||||
expected: As an admin, resolve one ambiguous review via a candidate "Link company" button, and one zero-candidate review via the manual-search combobox. In psql, confirm both `pax8_companies.match_method='manual'` (with `autotask_company_id` set) and both `pax8_company_match_review.resolved_at` are set. Trigger a PAX8 sync and confirm neither resolution is reverted or re-flagged.
|
||||
result: [pending]
|
||||
result: passed — user resolved 10 reviews via the live UI (candidate + manual-search paths); DB confirmed `match_method='manual'` + `resolved_at` set for all 10, `autotask_company_id` matching `resolved_to_company_id`. A real PAX8 full sync ran (2026-07-12T15:41:34Z–15:43:13Z, status=completed) after the resolutions; all 10 remained intact afterward (still 10 manual/resolved, 28 open — untouched by the sync).
|
||||
|
||||
why flagged: 14-06-SUMMARY.md claimed this was already confirmed, but its file timestamp (15:09) precedes both the code review (15:15) and the CR-01 fix commit d56db02 (15:16:38). CR-01 was the bug where the manual-search resolve path 400s on every attempt — so that sub-step of the checkpoint could not have succeeded as described. The live dev DB currently shows 0 resolved matches (38 still open), consistent with the checkpoint not actually having exercised this path successfully. Code is now correct; the live E2E needs to be redone.
|
||||
why flagged: 14-06-SUMMARY.md claimed this was already confirmed, but its file timestamp (15:09) precedes both the code review (15:15) and the CR-01 fix commit d56db02 (15:16:38). CR-01 was the bug where the manual-search resolve path 400s on every attempt — so that sub-step of the checkpoint could not have succeeded as described. Redone for real after the fix and independently confirmed via direct DB queries, not just user claim.
|
||||
|
||||
### 2. Re-confirm the D-07/D-08 view/resolve permission split at runtime
|
||||
expected: As a non-admin authenticated user, `/pax8` and both tabs load, but a resolve attempt (candidate button or manual search) is rejected with a 403 surfaced as an error toast. GET routes succeed for any authenticated session; POST `.../resolve` returns 403 for role='user' and 200 for admin/super-admin.
|
||||
result: [pending]
|
||||
result: passed — user (colleen@wulfconsulting.com, role='user') attempted to link a company from `/pax8`'s Needs Review tab; first attempt hit a real bug (see below), and after the fix, the attempt was correctly rejected with a "Forbidden" 403.
|
||||
|
||||
why flagged: Part of the same contested checkpoint above; the underlying role-check logic is code-verified correct but was not independently re-confirmed live after CR-01.
|
||||
why flagged: Part of the same contested checkpoint above. Uncovered a real, previously-unknown, app-wide bug in the process: `hasPermission()` in `lib/permissions.ts` had a function parameter named `userRole` that shadowed the module-level `userRole` role-object constant, so any permission check for a "user"-role session threw a 500 instead of returning `false`/403. This predates phase 14 (latent since the permissions module was written) but had never been exercised end-to-end by a non-admin session before this checkpoint. Fixed in commit `00f196c` (parameter renamed to `roleName`, added `lib/permissions.test.ts` — previously zero coverage on this file), rebuilt, and re-confirmed live: non-admin now gets a clean 403.
|
||||
|
||||
## Summary
|
||||
|
||||
total: 2
|
||||
passed: 0
|
||||
passed: 2
|
||||
issues: 0
|
||||
pending: 2
|
||||
pending: 0
|
||||
skipped: 0
|
||||
blocked: 0
|
||||
|
||||
## Gaps
|
||||
|
||||
None — both items resolved and independently confirmed (DB queries for item 1, live retest for item 2). One out-of-scope but real bug (`hasPermission` shadowing) was found and fixed as part of closing item 2; tracked in the fix commit and covered by a new regression test.
|
||||
|
|
|
|||
|
|
@ -1,23 +1,27 @@
|
|||
---
|
||||
phase: 14-pax8-ui-surface
|
||||
verified: 2026-07-11T19:20:00Z
|
||||
status: human_needed
|
||||
score: 4/4 code-level truths verified; 1 live-environment claim contested by evidence
|
||||
reverified: 2026-07-12T22:20:00Z
|
||||
status: passed
|
||||
score: 4/4 truths fully code-and-live verified
|
||||
overrides_applied: 0
|
||||
human_verification:
|
||||
- test: "Re-run Plan 14-06 Task 2 Step 6 (resolve one candidate-based review + one manual-search review as admin; trigger a PAX8 sync; confirm both resolutions persist and are not overwritten) against the live dev DB, now that CR-01 is fixed"
|
||||
expected: "Two pax8_companies rows show match_method='manual' with autotask_company_id set; two pax8_company_match_review rows show resolved_at/resolved_by_user_id/resolved_to_company_id set; a subsequent PAX8 sync leaves both untouched (match_method still 'manual', review still resolved)"
|
||||
why_human: "Requires live UI interaction (candidate button + Command/Popover manual search) and triggering a real PAX8 sync against the dev DB — not verifiable by static code inspection alone"
|
||||
result: "PASSED — user resolved 10 reviews live; DB confirmed match_method='manual' + resolved_at set for all 10; a real PAX8 full sync (2026-07-12T15:41:34Z-15:43:13Z) ran afterward and left all 10 untouched (still 10 resolved / 28 open)"
|
||||
- test: "Re-confirm the D-07/D-08 view/resolve permission split against the running app: a non-admin user can view /pax8 and both tabs, but a resolve attempt returns 403"
|
||||
expected: "GET routes succeed for any authenticated user; POST .../resolve returns 403 for a 'user'-role session and 200 for 'admin'/'super-admin'"
|
||||
why_human: "Requires two distinct authenticated browser sessions to exercise both branches at runtime"
|
||||
result: "PASSED (after fix) — non-admin resolve attempt initially 500'd due to a real, pre-existing, app-wide bug in lib/permissions.ts:hasPermission() (parameter name shadowed the module-level userRole role object, so every requirePermission() check for a 'user'-role session crashed instead of returning 403). Fixed in commit 00f196c, covered by new lib/permissions.test.ts, rebuilt, and re-confirmed: non-admin now gets a clean 403 Forbidden."
|
||||
---
|
||||
|
||||
# Phase 14: /pax8 UI Surface Verification Report
|
||||
|
||||
**Phase Goal:** A manager can open `/pax8` and see PAX8 companies with their subscriptions and a cost breakdown, and an admin can resolve any flagged/ambiguous company match directly from that page — no psql required.
|
||||
**Verified:** 2026-07-11T19:20:00Z
|
||||
**Status:** human_needed
|
||||
**Re-verified:** 2026-07-12T22:20:00Z
|
||||
**Status:** passed
|
||||
**Re-verification:** No — initial verification
|
||||
|
||||
## Goal Achievement
|
||||
|
|
@ -119,5 +123,18 @@ The phase's real open question is process, not code: the "checkpoint:human-verif
|
|||
|
||||
---
|
||||
|
||||
## Re-verification (2026-07-12T22:20:00Z)
|
||||
|
||||
Both human verification items above were redone for real and independently confirmed:
|
||||
|
||||
1. **SC#4 resolve + persist-through-sync:** User resolved 10 reviews live (both candidate-button and manual-search paths). Confirmed directly via DB query: 10 `pax8_companies` rows with `match_method='manual'` + `autotask_company_id` set, 10 `pax8_company_match_review` rows with `resolved_at` set, `autotask_company_id` matching `resolved_to_company_id`. A real PAX8 full sync then ran (`sync_history`: started 2026-07-12T15:41:34Z, completed 15:43:13Z, status=completed) and left all 10 resolutions untouched afterward (re-queried: still 10 manual/resolved, 28 open).
|
||||
|
||||
2. **D-07/D-08 permission split:** Confirmed a second, previously-unknown bug in the process — `hasPermission()` in `lib/permissions.ts` had a function parameter named `userRole: string` that shadowed the module-level `const userRole = ac.newRole(...)` role object. Any permission check for a `role='user'` session hit `.statements` on the shadowed string parameter instead of the real role object and **threw a 500** instead of returning `false`/403. This bug is app-wide and predates Phase 14 (latent in `lib/permissions.ts` since it was written) — it had simply never been exercised end-to-end by a non-admin session hitting a `requirePermission()`-gated route before this checkpoint. Fixed in commit `00f196c` (parameter renamed to `roleName`), covered by a new `lib/permissions.test.ts` (5 tests, previously zero coverage on this file). Rebuilt and re-confirmed live: non-admin resolve attempt now correctly returns 403 Forbidden.
|
||||
|
||||
Both items are now genuinely resolved with independent evidence (direct DB queries, not just user claims). **Status updated to `passed`.**
|
||||
|
||||
---
|
||||
|
||||
_Verified: 2026-07-11T19:20:00Z_
|
||||
_Verifier: Claude (gsd-verifier)_
|
||||
_Re-verified: 2026-07-12T22:20:00Z_
|
||||
_Verifier: Claude (gsd-verifier / orchestrator re-verification)_
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue