Merge remote-tracking branch 'origin/031-refactor-page-colocation' into 031-refactor-page-colocation

Author: matejvasek

.claude/commands/init-session.md

@@ -1,12 +1,10 @@
 ---
 allowed-tools: Bash(git log:*), Bash(pwd), Bash(./init.sh), Bash(yarn test*), Bash(cat .dev-env.json), Read
-description: Run startup sequence (steps 1-6 from docs/WORKFLOW.md)
+description: Run startup sequence and pick a story from the Jira epic
 ---
 
 # Session Onboard
 
-Execute the startup sequence from `docs/WORKFLOW.md`. AGENTS.md is always in context — no need to read it explicitly.
-
 ## Steps
 
 1. **Confirm working directory** — run `pwd`.
@@ -17,8 +15,10 @@ Execute the startup sequence from `docs/WORKFLOW.md`. AGENTS.md is always in con
    ```
 
 3. **Check struggles** — read `docs/agent-struggles.json`. If unresolved entries exist, present to user.
-4. **Pick feature** — read `docs/features.json`, find first `"passes": false` entry.
-5. **Start dev env** — run `./init.sh`.
-6. **Read ports** — read `.dev-env.json` and note the backend, plugin, and console ports.
-7. **Run tests** — run `yarn test` and verify app is healthy.
-8. **Wait** — tell the user you're oriented, report the picked feature and which step of the Feature Development Sequence you'd start at. When the user says to proceed, follow the Feature Development Sequence in `docs/WORKFLOW.md` step by step. Do NOT start any work autonomously.
+4. **CI check** — run `yarn ci` (lint, test, build) and verify the project is healthy.
+5. **Start dev env** — run `./init.sh`. If it fails (e.g. nono sandbox blocks `oc`), tell the user to start it manually from their terminal.
+6. **Read ports** — read `.dev-env.json` and note the backend, plugin, and console ports. If init.sh failed, skip this step.
+7. **Pick story** — tell the user you're oriented and propose picking a story from the PoC epic: <https://redhat.atlassian.net/browse/SRVOCF-810>. Ask the user to provide the story description (title, acceptance criteria, or a Jira link). The user may pick one story or a few small ones.
+8. **Create feature entry** — once the user provides a story, create a new entry in `docs/features.json` for it (append to the array, `"passes": false`).
+9. **Branch** — create a feature branch per [Branching](docs/WORKFLOW.md#branching) convention and open a draft PR (`gh pr create --draft`).
+10. **Propose planning** — tell the user the branch is ready and propose to start planning (step 2 of the Feature Development Sequence in `docs/WORKFLOW.md`). Do NOT start any work autonomously.

docs/ARCHITECTURE.md

@@ -11,8 +11,8 @@ flowchart TB
     TYPES[Types] ---|cross-cutting| UTILS[Utils]
     SERVICES[Services] ---|cross-cutting| UTILS
     COMPONENTS[Components] ---|cross-cutting| UTILS
-    VIEWS[Views] --> COMPONENTS[Components]
-    VIEWS --> HOOKS[Hooks]
+    PAGES[Pages] --> COMPONENTS[Components]
+    PAGES --> HOOKS[Hooks]
     COMPONENTS --> HOOKS
     COMPONENTS --> TYPES
     HOOKS --> SERVICES[Services]
@@ -24,34 +24,47 @@ Arrows mean "imports / depends on."
 
 | Layer | Maps to | Depends on |
 |-------|---------|------------|
-| **Types** | `services/types.ts` | nothing |
-| **Services** | `services/*/Service.ts` + implementations | Types, Utils |
-| **Hooks** | `services/*/use*.ts` — wiring layer | Services, Types, Utils |
-| **Components** | `components/` — FunctionTable, CreateForm, etc. | Hooks, Types, Utils |
-| **Views** | `views/` — page-level components | Components, Hooks, Utils |
-| **Utils** | `utils/` — constants, helpers | nothing (cross-cutting) |
+| **Types** | `common/services/types.ts` | nothing |
+| **Services** | `common/services/*/Service.ts` + implementations | Types, Utils |
+| **Hooks** | `common/services/*/use*.ts`, `common/hooks/`, `pages/<name>/hooks/` | Services, Types, Utils |
+| **Components** | `common/components/` (shared), `pages/<name>/components/` (page-specific) | Hooks, Types, Utils |
+| **Pages** | `pages/<name>/` | Components, Hooks, Utils |
+| **Utils** | `common/utils/` | nothing (cross-cutting) |
 
 ### Dependency Rules
 
-- Unidirectional: Types <- Services <- Hooks <- Components <- Views
+- Unidirectional: Types <- Services <- Hooks <- Components <- Pages
 - Utils can be imported by any layer
-- Views never import Services directly (always through Hooks)
-- Services never import Components or Views
+- Pages never import Services directly (always through Hooks)
+- Services never import Components or Pages
 - No circular dependencies
 
+### Co-location Convention
+
+- `src/pages/<name>/` contains the page component, its test, and a `components/` subdir
+- `src/pages/<name>/components/` contains components used only by that page
+- `src/common/` contains everything shared across pages (components, services, utils, context)
+- **Ownership rule:** if a component is imported by only one page (test imports don't count), it lives in `pages/<name>/components/`. If imported by multiple pages, it lives in `common/components/`.
+
 ## React
 
-### Page / Component / Hook Rules
+### Pages
+
+- **Smart for page-specific data** — pages use central hooks (e.g. `useClusterService`, `useSourceControl`) to fetch, prepare, and transform all data needed for downstream components.
+
+### Components
 
-**Components are simple by default** — they receive data via props, render it, and call callbacks. No logic at the top of a component.
+- **Simple by default** — they receive data via props, render it, and call callbacks. No logic at the top of a component.
+- **May own data when self-contained** — a component may own its own data and state when it encapsulates a self-contained capability that is not specific to any one page (e.g., forge connection, auth flows, notification subscriptions). The component becomes the single owner of that concern. Pages consume it without orchestrating its internals.
+- **Sub-components** — if a sub-component is only used by one parent, keep it in the parent's file, unexported. Extract to its own file only when the sub-component is used by multiple siblings.
 
-**A component may own its own data and state when it encapsulates a self-contained capability that is not specific to any one page** (e.g., forge connection, auth flows, notification subscriptions). The component becomes the single owner of that concern. Pages consume it without orchestrating its internals.
+### Hooks
 
-**Pages are smart for page-specific data** — they use central hooks (e.g. `useClusterService`, `useSourceControl`) to fetch, prepare, and transform all data needed for downstream components.
+- **Extract logic into hooks** — if a page or component has any logic (state management, data transformation, side effects), extract it into a custom hook. If the hook is only used by one component, keep it in the same file, do not export it. If the hook is reused by multiple components within one page, put it in `src/pages/<name>/hooks/`. If reused across pages, put it in `src/common/hooks/`. If there is no logic, no hook is needed.
 
-**Extract logic into hooks** — if a page or component has any logic (state management, data transformation, side effects), extract it into a custom hook. If the hook is reused by multiple components, put it in a separate file: `useFunctionTable.ts`. If the hook is only used by one component, keep it in the same file, do not export it. If there is no logic, no hook is needed.
+### File Ordering
 
-**File ordering** — within a file, put the exported component at the top, then its hook below, then helper functions at the bottom. Readers see the main thing first and can drill down.
+Within a file, put the exported component at the top, then its hook below, then sub-components, then helper functions at the bottom. Readers see the main thing first and can drill down.
 
 ### Performance
 
@@ -60,6 +73,7 @@ Arrows mean "imports / depends on."
 ## Architectural Guidance
 
 - PatternFly components preferred over custom HTML
+- PatternFly styling and styling rules over custom CSS
 - Error handling through ErrorProvider/addError pattern
-- Shared utilities in `utils/`, not hand-rolled per component
+- Shared utilities in `common/utils/`, not hand-rolled per component
 - Services consumed through hooks, never imported directly

docs/TESTING.md

@@ -15,23 +15,31 @@ Do NOT write all test cases first and then implement everything at once.
 
 | Layer | Tool | Scope |
 |-------|------|-------|
-| Unit / Component | Jest + React Testing Library | Hooks, services, component rendering, form logic |
+| Unit / Component | Vitest + React Testing Library | Hooks, services, component rendering, form logic |
 | E2e / Feature validation | Cypress | Validate features.json entries in real browser |
 | API mocking | MSW (Mock Service Worker) | GitHub API + K8s API — mock everything first, real cluster later |
 
 ## Mock Strategy
 
-Use MSW for all API mocking (GitHub + K8s). If SDK hooks require module-level mocks in unit tests (because they depend on Console runtime internals), fall back to Jest mocks — but try MSW first.
+MSW is the primary mocking strategy for anything that hits the network (GitHub API, K8s API, Go backend). K8s API mocking uses MSW WebSocket capability.
 
-- **Start:** Mock everything via MSW (GitHub API, K8s API)
-- **Later:** Real cluster for e2e — SDK hooks work natively, GitHub API remains mocked
-- **Fallback:** If SDK hooks can't be driven by MSW alone in unit tests, use Jest module mocks
+`vi.mock` is only for framework and library internals that have no external service:
+
+- `react-i18next` (translation hook)
+- `@openshift-console/dynamic-plugin-sdk` (console shell runtime components like DocumentTitle, ListPageHeader, consoleFetchJSON)
+- `@patternfly/react-icons` (UI library)
+- `react-router-dom-v5-compat` (framework routing)
+- `libsodium-wrappers` (WASM crypto library)
+
+If it makes an HTTP or WebSocket call, mock it with MSW, not `vi.mock`.
 
 ## File Conventions
 
 | Type | Location |
 |------|----------|
-| Unit / Component tests | `src/**/*.test.ts\|tsx` |
+| Component tests | `src/pages/<name>/components/*.test.ts\|tsx`, `src/common/components/*.test.ts\|tsx` |
+| Page tests | `src/pages/<name>/*.test.ts\|tsx` |
+| Service / Hook / Util tests | `src/common/**/*.test.ts\|tsx` |
 | E2e specs | `e2e/<feature-name>/*.test.ts` |
 | MSW handlers | `testing/msw/handlers.ts` |
 
@@ -42,9 +50,28 @@ Use MSW for all API mocking (GitHub + K8s). If SDK hooks require module-level mo
 | Service interfaces | Unit | `FunctionService.generateFunction()` returns expected files |
 | React hooks | Unit | `useFunctionService()` returns service instance |
 | Components | Component | `CreateForm` renders all fields, validates input |
-| Views | Component + E2e | `FunctionListPage` shows empty state, table |
+| Pages | Component + E2e | `FunctionsListPage` shows empty state, table |
 | User flows | E2e | Create form → submit → list shows new function |
 
+## Component vs. Page Tests
+
+Every component gets its own exhaustive test file. Every page gets its own test file that tests the page's orchestration and integration with its components.
+
+**Component tests** cover:
+
+- Rendering based on props (all states and variants)
+- User interactions that trigger callbacks (clicks, input, form validation)
+- Internal state (expand/collapse, selection)
+
+**Page tests** cover:
+
+- Component is present on the page and wired correctly
+- Data flows from hooks/services to components (correct props)
+- User actions that trigger cross-component effects or service calls (e.g., form submit calls service, then navigates)
+- Page-level states: loading, error, empty
+
+Overlap between component tests and page tests is expected and acceptable. They test at different levels: component tests verify the component works in isolation, page tests verify the page's orchestration logic works correctly.
+
 ## Testing Best Practices
 
 1. **User-Centric Testing** — Test what users see and interact with.
@@ -61,50 +88,54 @@ Use MSW for all API mocking (GitHub + K8s). If SDK hooks require module-level mo
    - **Act:** Perform user actions
    - **Assert:** Verify expected state
 
+6. **Scoping** — Place beforeEach, afterEach, and afterAll inside describe blocks.
+
 ## Mocking Patterns
 
+MSW is the primary approach. `vi.mock` is rare (see Mock Strategy above).
+
 Use ESM `import` at top of file. Never use `require('react')` or `React.createElement()` in mocks.
-Prefer `jest.mock()` for modules, `jest.fn()` for components. Keep mocks simple.
+Keep mocks simple.
 
-**Correct patterns:**
+**Correct patterns (for the rare `vi.mock` cases):**
 
 ```typescript
 // Return null
-jest.mock('../MyComponent', () => () => null);
+vi.mock('../MyComponent', () => () => null);
 
 // Return string
-jest.mock('../LoadingSpinner', () => () => 'Loading...');
+vi.mock('../LoadingSpinner', () => () => 'Loading...');
 
 // Return children directly
-jest.mock('../Wrapper', () => ({ children }) => children);
+vi.mock('../Wrapper', () => ({ children }) => children);
 
-// Track calls with jest.fn
-jest.mock('../ButtonBar', () => jest.fn(({ children }) => children));
+// Track calls with vi.fn
+vi.mock('../ButtonBar', () => vi.fn(({ children }) => children));
 
-// Mock custom hooks
-jest.mock('../useCustomHook', () => ({
-  useCustomHook: jest.fn(() => [/* mock data */]),
+// Mock framework hooks
+vi.mock('react-i18next', () => ({
+  useTranslation: () => ({ t: (key: string) => key }),
 }));
 ```
 
 **Forbidden patterns:**
 
 ```typescript
 // NEVER - require() in mocks
-jest.mock('../Component', () => {
+vi.mock('../Component', () => {
   const React = require('react');
   return () => React.createElement('div');
 });
 
 // NEVER - JSX in mocks
-jest.mock('../Component', () => () => <div>Mock</div>);
+vi.mock('../Component', () => () => <div>Mock</div>);
 ```
 
 **Clean up mocks:**
 
 ```typescript
 afterEach(() => {
-  jest.restoreAllMocks();
+  vi.restoreAllMocks();
 });
 ```
 

docs/WORKFLOW.md

@@ -2,23 +2,14 @@
 
 ## Startup Sequence
 
-Every session, before doing any work:
-
-1. `pwd` — confirm working directory
-2. Read `docs/claude-progress.txt` + `git log --oneline -10` — orient
-3. Read `docs/agent-struggles.json` — if unresolved entries exist, present to user
-4. Read `docs/features.json` — pick first `"passes": false` entry
-5. Run `./init.sh` — start dev env
-6. Read `.dev-env.json` — note the dev server ports (backend, plugin, console)
-7. Run tests — verify app is healthy
-8. If broken → fix first. If clean → start [Feature Development Sequence](#feature-development-sequence).
+Handled by the `init-session` command (`.claude/commands/init-session.md`).
 
 ## Feature Development Sequence
 
 After [Startup Sequence](#startup-sequence), work through the picked feature:
 
-1. **Plan** — read `docs/ARCHITECTURE.md` + `docs/STYLEGUIDE.md` + `docs/TESTING.md`, then use `/brainstorming` to design the chosen feature from `docs/features.json`, then use `/writing-plans` to create implementation plan → `docs/plans/active/<NNN>-<type>-<short-name>.md`
-2. **Branch** — create feature branch per [Branching](#branching) convention. Immediately push and open a **draft PR** (`gh pr create --draft`) to reserve the PR number for other contributors' branch numbering.
+1. **Branch** — create feature branch per [Branching](#branching) convention. Immediately push and open a **draft PR** (`gh pr create --draft`) to reserve the PR number for other contributors' branch numbering.
+2. **Plan** — read `docs/ARCHITECTURE.md` + `docs/STYLEGUIDE.md` + `docs/TESTING.md`, then use `/brainstorming` to design the chosen feature from `docs/features.json`, then use `/writing-plans` to create implementation plan → `docs/plans/active/<NNN>-<type>-<short-name>.md`
 3. **Implement** — using `/executing-plans` skill
 4. **Review** — code review using `/requesting-code-review` skill, fix found issues
 5. **Manual Test** — use browser automation and validate it works in the browser

docs/claude-progress.txt

@@ -1,6 +1,42 @@
 # Claude Progress Log
 # Newest entries first. Agents: append your entry at the top after the header.
 
+---
+## 2026-05-21 | Session: Page co-location restructure (continued)
+Worked on: Follow-up fixes from review, test cleanup, docs
+Completed:
+- Folded LeaveModal back into EditToolbar.tsx (sub-component rule: single consumer = inline unexported)
+- Documented sub-component rule in ARCHITECTURE.md
+- Restructured ARCHITECTURE.md Page/Component/Hook rules into subsections (Pages, Components, Hooks, File Ordering)
+- Added hooks directory convention to ARCHITECTURE.md (common/hooks/, pages/<name>/hooks/)
+- Added EditToolbar tests (9 tests: render, disabled state, save success with auto-dismiss, save failure, back navigation, leave modal flow)
+- Moved all beforeEach/afterEach/afterAll into describe blocks across all test files
+- Merged duplicate beforeEach blocks in OcpClusterService.test.ts
+- Added scoping rule to TESTING.md (beforeEach/afterEach inside describe, vi.mock outside)
+- Fixed useClusterService.test.tsx afterEach scoping
+- Added yarn ci script to package.json (lint + test + build)
+- Updated init-session.md: use yarn ci, handle init.sh failure in sandbox
+- Fixed Monaco editor error: replaced runtime import of @patternfly/react-code-editor with import type (prevents bundling Monaco workers)
+- 14 suites, 121 tests passing, zero lint errors, webpack builds clean
+Left off: PR #31 ready for review.
+Blockers: Monaco "Unexpected usage" error in OCP console editor page (pre-existing, deferred to separate session)
+
+---
+## 2026-05-20 | Session: Page co-location restructure + workflow updates
+Worked on: Restructure src/ into pages/ and common/, update workflow and docs
+Completed:
+- Updated .pi/prompts/init-session.md and docs/WORKFLOW.md: dropped old step 4 (pick from features.json), added Jira epic story selection flow, swapped Feature Dev steps 1 and 2 (Branch before Plan), removed duplicate startup sequence from WORKFLOW.md
+- Created features.json entry for SRVOCF-845, branch 031-refactor-page-colocation, draft PR #31
+- Moved all shared code to src/common/ (services, utils, context, UserAvatar)
+- Moved pages to src/pages/function-list/, function-create/, function-edit/ with components/ subdirs
+- Extracted EditToolbar and LeaveModal from FunctionEditPage into separate files
+- Updated package.json exposedModules paths
+- Updated ARCHITECTURE.md with co-location convention and ownership rule
+- Updated TESTING.md: component vs. page test rules, Vitest/MSW migration, updated file conventions
+- 13 suites, 112 tests passing, zero lint errors, webpack builds clean
+Left off: Three follow-up items for next day.
+Blockers: oc login not accessible from agent sandbox (permission denied on ~/.kube/config)
+
 ---
 ## 2026-05-18 | Session: Error server and JSON error responses
 Worked on: Errserver for failed Go compilation, JSON error responses for backend