diff --git a/tools/v2/team/manager-review-queue/README.md b/tools/v2/team/manager-review-queue/README.md index dd0fe571..062ce497 100644 --- a/tools/v2/team/manager-review-queue/README.md +++ b/tools/v2/team/manager-review-queue/README.md @@ -1,15 +1,130 @@ # Manager Review Queue -This folder is the isolated workspace for the Manager Review Queue tool. +Folder-local V2 team tool for reviewing manager approval requests before they +leave the isolated tool workspace. + +The current implementation includes local mock data, a deterministic review +engine, React components for queue states, security/performance guards, and +folder-local tests. It is not wired into the main application yet. ## Ownership Boundary All work for this tool must stay inside: -`text -.\tools\v2\team\manager-review-queue\ -` +``` +tools/v2/team/manager-review-queue/ +``` + +Do not wire this tool into the main app, routing, inbox architecture, wallet core, +Stellar integration, database schema, or design system unless a future integration +issue explicitly allows it. + +## Folder Structure + +``` +manager-review-queue/ + components/ Isolated React queue states and item list + docs/ + ACCESSIBILITY.md Keyboard and screen reader guidance + VISUAL_STYLE.md Local visual styling notes + api.md Review engine API contract + security-and-performance.md Guard threat model and limits + review-notes.md OSS reviewer guide + test-plan.md Automated and manual validation plan + fixtures/ + reviewFixtures.ts TypeScript UI/service fixtures + sample-review-requests.json Guard test fixtures + guards/ + review-guards.mjs Pure validation and performance guards + services/ + reviewEngine.ts Mock queue fetch/update service + tests/ + documentation-contract.test.mjs Documentation completeness checks + review-guards.test.mjs Executable guard tests + types/ + index.ts Local TypeScript types + README.md + specs.md +``` + +## Setup + +No package install is required for the guard and documentation tests. They use +Node built-ins only. + +Requirements: + +- Node 18 or later. +- Run commands from the repository root unless noted otherwise. + +## Running Tests + +From the repository root: + +``` +node --test tools/v2/team/manager-review-queue/tests/review-guards.test.mjs +node --test tools/v2/team/manager-review-queue/tests/documentation-contract.test.mjs +``` + +From this folder: + +``` +node --test tests/review-guards.test.mjs +node --test tests/documentation-contract.test.mjs +``` + +The guard tests validate field allowlists, ID safety, submitter email safety, +text sanitization, queue/history/attachment/tag size limits, and fixture-driven +hostile inputs. + +The documentation contract test validates that setup, test commands, review +boundaries, known limitations, and required review notes remain discoverable. + +## Usage + +The local service exposes two async operations from `services/reviewEngine.ts`: + +- `fetchReviewQueue(input)` returns filtered and paginated queue items from the + local mock store. +- `updateReviewItemStatus(input)` updates a mock item status or throws when the + item ID is unknown. + +The `ManagerReviewQueue` component loads pending items, displays loading, empty, +error, success, and queue-list states, and offers approve, reject, and escalate +actions against the local mock service. + +## Fixtures + +- `fixtures/reviewFixtures.ts` feeds the TypeScript service and UI examples. +- `fixtures/sample-review-requests.json` feeds the guard tests with valid + requests, hostile inputs, and sanitization edge cases. + +All fixture data is local and synthetic. It does not include real users, +production inbox data, credentials, wallet values, or external service calls. + +## Review Docs + +- `docs/test-plan.md` lists the automated checks and manual UI/accessibility + review path. +- `docs/review-notes.md` explains the expected OSS review scope and what is + intentionally out of scope. +- `docs/api.md`, `docs/security-and-performance.md`, `docs/ACCESSIBILITY.md`, + and `docs/VISUAL_STYLE.md` document existing implementation details. + +## Known Limitations + +- The queue service uses an in-memory mock store; it has no persistence layer. +- The UI components are isolated and are not mounted in an app route. +- Authentication, authorization, audit persistence, and notification delivery are + future integration concerns. +- The mock service simulates latency but does not make live network calls. +- Guard sanitizers prepare values for safer handling, but renderers must still + escape user-authored text before inserting it into HTML. -Do not wire this tool into the main app, routing, inbox architecture, wallet core, Stellar core, database schema, or existing design system unless a future integration issue explicitly allows it. +## Acceptance Checklist -See specs.md for the issue categories and contributor expectations. +- [x] Tests and test plan live inside this folder. +- [x] Documentation explains independent setup and review. +- [x] Fixtures are local and synthetic. +- [x] No files outside `tools/v2/team/manager-review-queue/` are needed. +- [x] The contribution is reviewable as a self-contained mini-product change. diff --git a/tools/v2/team/manager-review-queue/docs/review-notes.md b/tools/v2/team/manager-review-queue/docs/review-notes.md new file mode 100644 index 00000000..2e8a7a19 --- /dev/null +++ b/tools/v2/team/manager-review-queue/docs/review-notes.md @@ -0,0 +1,69 @@ +# Review Notes - Manager Review Queue + +This contribution makes the Manager Review Queue easier for OSS contributors to +review independently. It fixes the placeholder README/spec text, adds a focused +test plan, adds review notes, and adds a documentation contract test that keeps +the review instructions executable. + +## What To Review + +### Scope + +- Every changed file should live under `tools/v2/team/manager-review-queue/`. +- There should be no changes to routing, dashboard layout, wallet code, Stellar + integration, database schema, authentication, inbox architecture, root package + files, or workflow files. +- The tool should remain reviewable without running the main app. + +### Existing Guard Coverage + +- `tests/review-guards.test.mjs` is the executable behavior suite for the current + zero-dependency guard layer. +- It covers ID validation, status and priority allowlists, submitter email safety, + text sanitization, request object validation, and collection size guards. +- It also exercises `fixtures/sample-review-requests.json` for valid requests, + hostile inputs, and sanitization edge cases. + +### New Documentation Coverage + +- `docs/test-plan.md` should explain how to run automated checks and how to + manually review UI, accessibility, security, and performance behavior. +- `docs/review-notes.md` should make this issue easy to validate without asking + reviewers to inspect unrelated app code. +- `tests/documentation-contract.test.mjs` should fail if README/specs/docs lose + the key setup, test command, known limitation, or isolation guidance. + +### Fixtures + +- `fixtures/reviewFixtures.ts` and `fixtures/sample-review-requests.json` should + stay local and synthetic. +- Fixture data must not contain real users, production inbox data, credentials, + wallet values, or external service tokens. + +## Validation Commands + +From the repository root: + +``` +node --test tools/v2/team/manager-review-queue/tests/review-guards.test.mjs +node --test tools/v2/team/manager-review-queue/tests/documentation-contract.test.mjs +git diff --check +``` + +## What Is Intentionally Not Included + +- No app route, navigation link, dashboard integration, or inbox data connection. +- No persistence layer, database schema, authentication, or authorization policy. +- No wallet, Stellar, payment, or external API integration. +- No production data import or live manager approval workflow. +- No broad formatting pass over existing files. + +## Follow-Up Shape + +Future issues can add, inside this folder: + +- Service tests for `fetchReviewQueue` and `updateReviewItemStatus` once a local + TypeScript test runner is selected. +- Component tests for loading, empty, error, success, and populated queue states. +- Integration work that wires real inbox data and persistence, if explicitly + approved by a future issue. diff --git a/tools/v2/team/manager-review-queue/docs/test-plan.md b/tools/v2/team/manager-review-queue/docs/test-plan.md new file mode 100644 index 00000000..0a4574e7 --- /dev/null +++ b/tools/v2/team/manager-review-queue/docs/test-plan.md @@ -0,0 +1,100 @@ +# Test Plan - Manager Review Queue + +This tool is isolated under `tools/v2/team/manager-review-queue/`. The current +test plan covers the folder-local guard layer, fixture contract, documentation +contract, and manual review steps for the local UI and mock review service. + +## Automated Tests + +Run from the repository root: + +``` +node --test tools/v2/team/manager-review-queue/tests/review-guards.test.mjs +node --test tools/v2/team/manager-review-queue/tests/documentation-contract.test.mjs +``` + +Run from this tool folder: + +``` +node --test tests/review-guards.test.mjs +node --test tests/documentation-contract.test.mjs +``` + +No install step is required for these tests. They use Node built-ins only. + +## Guard Test Coverage + +`tests/review-guards.test.mjs` validates: + +1. `validateReviewId` accepts safe IDs and rejects empty strings, non-strings, + path traversal, XSS payloads, SQL-like payloads, and oversized IDs. +2. `validateStatus` accepts only `pending`, `approved`, `rejected`, and + `escalated`. +3. `validatePriority` accepts only `low`, `medium`, `high`, and `critical`. +4. `validateSubmitterEmail` blocks malformed addresses, CRLF injection, null + bytes, empty strings, and oversized values. +5. `sanitizeNote` and `sanitizeSubject` remove unsafe control characters and cap + length. +6. `validateReviewRequest` validates complete request objects. +7. `guardQueueSize`, `guardHistorySize`, `guardAttachmentCount`, and `guardTags` + reject oversized or malformed collections before iteration. +8. `fixtures/sample-review-requests.json` drives valid request, hostile input, + and sanitization edge-case checks. + +## Documentation Contract Coverage + +`tests/documentation-contract.test.mjs` validates: + +1. README setup includes both executable test commands. +2. README explains fixtures, known limitations, and isolated ownership. +3. `specs.md` has no template placeholders and defines the V2 team scope. +4. `docs/test-plan.md` documents automated and manual review steps. +5. `docs/review-notes.md` tells reviewers what to inspect and what is out of + scope. +6. Existing docs for API, accessibility, security/performance, and visual style + remain discoverable from README. + +## Manual Functional Review + +When reviewing the local UI components in a test route or Storybook-like harness: + +1. Render `ManagerReviewQueue`. +2. Confirm the initial loading state is visible while the mock service delay is + pending. +3. Confirm pending queue items render after `fetchReviewQueue` resolves. +4. Approve, reject, or escalate an item and confirm it leaves the pending list. +5. Force `updateReviewItemStatus` to receive an unknown ID and confirm the error + state appears with a retry path. +6. Filter the mock store so no pending items remain and confirm the empty state + renders. +7. Confirm the success banner can be dismissed. + +## Manual Accessibility Review + +1. Tab through every queue item action button. +2. Confirm visible focus rings appear on action buttons and the active item. +3. Confirm screen reader output includes the page heading, item subjects, status, + priority/risk context, and action labels. +4. Confirm loading, error, and success states are announced through live regions. +5. Confirm approve, reject, and escalate buttons have contextual accessible names. +6. Confirm no state depends on color alone. + +## Manual Security and Performance Review + +1. Review `guards/review-guards.mjs` before any service or UI integration. +2. Confirm field validators fail closed for unknown statuses and priorities. +3. Confirm free-text sanitizers do not claim to HTML-escape rendered output. +4. Confirm queue/history/attachment/tag guards run before iterating over large + collections. +5. Confirm no production data, network call, wallet interaction, or database + write is introduced by this folder. + +## Known Limitations + +- The mock service stores state in memory only. +- No app route mounts the UI yet. +- Authentication, authorization, durable audit logs, and notification delivery + are future integration work. +- The guard tests do not compile the TypeScript UI; they cover the current + zero-dependency guard layer and documentation contract. +- Renderers must still escape user-authored text before inserting it into HTML. diff --git a/tools/v2/team/manager-review-queue/specs.md b/tools/v2/team/manager-review-queue/specs.md index 7a7ae16f..cc8dd2ef 100644 --- a/tools/v2/team/manager-review-queue/specs.md +++ b/tools/v2/team/manager-review-queue/specs.md @@ -1,41 +1,69 @@ -# Manager Review Queue - -Manager approvals. - -## Scope - -- Release tier: $(System.Collections.Hashtable.Tier.ToUpperInvariant()) -- Audience: $(System.Collections.Hashtable.Audience) -- Folder ownership: $dir/ +# Manager Review Queue Specs -This is a self-contained tooling workspace. Do not wire this tool into the main app, routing, inbox architecture, wallet core, Stellar core, or design system unless a future integration issue explicitly allows it. +## Purpose -Recommended internal structure: +Provide a folder-local V2 team workflow for reviewing, approving, rejecting, or +escalating manager approval requests with deterministic local fixtures. -- components/ -- services/ -- hooks/ -- ests/ -- docs/ - "@ | Set-Content -Path "tools/v2/team/manager-review-queue/README.md" - @" +The tool should remain independently reviewable until a future integration issue +explicitly connects it to real inbox data, routing, persistence, authentication, +or notifications. -# Manager Review Queue Specs +## Scope -## Purpose +- Release tier: V2 +- Audience: team +- Folder ownership: `tools/v2/team/manager-review-queue/` -Manager approvals. +This is a self-contained tooling workspace. Do not wire this tool into the main +app, routing, inbox architecture, wallet core, Stellar integration, database +schema, or design system unless a future integration issue explicitly allows it. -## Contributor boundary +## Contributor Boundary All work for this tool should stay in: -$dir/ +``` +tools/v2/team/manager-review-queue/ +``` -## Required issue categories +## Required Issue Categories - Architecture - Feature - UI and accessibility - Security and performance - Testing and documentation + +## Internal Structure + +- `components/` for isolated React UI states and review actions. +- `services/` for deterministic local queue behavior. +- `guards/` for validation, sanitization, and size limits. +- `fixtures/` for synthetic local data. +- `tests/` for folder-local executable checks. +- `docs/` for setup, APIs, accessibility, security/performance, test plans, and + review notes. +- `types/` for local TypeScript contracts. + +## Review Contract + +Current and future contributions should keep these behaviors verifiable without +the main application: + +- Queue items expose stable IDs, submitter metadata, status, priority/risk data, + and content snippets. +- The local service can fetch pending queue items and update item status using + deterministic mock data. +- Guard functions reject unsafe IDs, malformed email values, invalid status and + priority values, and oversized collections before expensive work begins. +- UI states cover loading, empty, error, success, and populated queues. +- Documentation tells OSS reviewers how to run tests and review the tool without + app-wide setup. + +## Out of Scope + +- Main app routes, dashboard layout, navigation, or inbox wiring. +- Wallet, Stellar, payment, or database changes. +- Live manager approval data, production user data, or external service calls. +- Authentication, authorization, durable audit logs, or notification delivery. diff --git a/tools/v2/team/manager-review-queue/tests/documentation-contract.test.mjs b/tools/v2/team/manager-review-queue/tests/documentation-contract.test.mjs new file mode 100644 index 00000000..6700329e --- /dev/null +++ b/tools/v2/team/manager-review-queue/tests/documentation-contract.test.mjs @@ -0,0 +1,107 @@ +import assert from "node:assert/strict"; +import { readFile } from "node:fs/promises"; +import { dirname, join } from "node:path"; +import { fileURLToPath } from "node:url"; +import test from "node:test"; + +const toolRoot = join(dirname(fileURLToPath(import.meta.url)), ".."); + +async function readToolFile(relativePath) { + return readFile(join(toolRoot, relativePath), "utf8"); +} + +function assertIncludesAll(content, values, label) { + for (const value of values) { + assert.ok(content.includes(value), `${label} should include "${value}"`); + } +} + +test("README documents isolated setup, tests, fixtures, and limitations", async () => { + const readme = await readToolFile("README.md"); + + assertIncludesAll( + readme, + [ + "tools/v2/team/manager-review-queue/", + "node --test tools/v2/team/manager-review-queue/tests/review-guards.test.mjs", + "node --test tools/v2/team/manager-review-queue/tests/documentation-contract.test.mjs", + "fixtures/sample-review-requests.json", + "Known Limitations", + "No files outside `tools/v2/team/manager-review-queue/` are needed.", + ], + "README", + ); + + assert.match(readme, /Do not wire this tool into the main app/); + assert.match(readme, /All fixture data is local and synthetic/); +}); + +test("specs define the V2 team scope without template remnants", async () => { + const specs = await readToolFile("specs.md"); + + assertIncludesAll( + specs, + [ + "Release tier: V2", + "Audience: team", + "Folder ownership: `tools/v2/team/manager-review-queue/`", + "Testing and documentation", + "Review Contract", + "Out of Scope", + ], + "specs.md", + ); + + assert.doesNotMatch(specs, /System\.Collections|Set-Content|\$dir|@"/); +}); + +test("test plan covers automated and manual review paths", async () => { + const plan = await readToolFile("docs/test-plan.md"); + + assertIncludesAll( + plan, + [ + "node --test tools/v2/team/manager-review-queue/tests/review-guards.test.mjs", + "node --test tools/v2/team/manager-review-queue/tests/documentation-contract.test.mjs", + "Guard Test Coverage", + "Documentation Contract Coverage", + "Manual Functional Review", + "Manual Accessibility Review", + "Manual Security and Performance Review", + "Known Limitations", + ], + "docs/test-plan.md", + ); +}); + +test("review notes explain scope, validation commands, and exclusions", async () => { + const notes = await readToolFile("docs/review-notes.md"); + + assertIncludesAll( + notes, + [ + "Every changed file should live under `tools/v2/team/manager-review-queue/`.", + "tests/review-guards.test.mjs", + "tests/documentation-contract.test.mjs", + "git diff --check", + "What Is Intentionally Not Included", + "No app route, navigation link, dashboard integration, or inbox data connection.", + ], + "docs/review-notes.md", + ); +}); + +test("README links existing implementation docs for independent review", async () => { + const readme = await readToolFile("README.md"); + + assertIncludesAll( + readme, + [ + "docs/api.md", + "docs/security-and-performance.md", + "docs/ACCESSIBILITY.md", + "docs/VISUAL_STYLE.md", + ], + "README implementation doc links", + ); +});