chore: archive 11 completed plan documents
Move completed design/implementation docs from docs/plans/ to docs/archive/ to keep the plans folder focused on active and future work. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
39
docs/archive/2026-02-03-continuation-modal-ux.md
Normal file
39
docs/archive/2026-02-03-continuation-modal-ux.md
Normal file
@@ -0,0 +1,39 @@
|
||||
# Continuation Modal UX Improvements
|
||||
|
||||
**Date:** 2026-02-03
|
||||
**Status:** Approved
|
||||
|
||||
## Changes
|
||||
|
||||
### 1. Parent option labels: hover tooltips instead of static headers
|
||||
|
||||
The ContinuationModal currently groups descendant nodes under uppercase headers like "FROM: PRINTER WON'T PRINT AT ALL -> NOTHING HAPPENS / NO RESPONSE". These clutter the screen.
|
||||
|
||||
**Change:** Move `parentOptionLabel` from static section headers to a `title` attribute on each descendant button. The descendants become a flat list. Hovering shows the path context as a native browser tooltip.
|
||||
|
||||
The "Or" divider and "Build Custom Branch" button at the bottom remain unchanged.
|
||||
|
||||
**Files:** `frontend/src/components/session/ContinuationModal.tsx`
|
||||
|
||||
### 2. Land on custom step before navigating to selected descendant
|
||||
|
||||
Currently, selecting a descendant in the ContinuationModal navigates directly to that node. The user should land on their custom step first to write notes, then proceed.
|
||||
|
||||
**Change:** Add `pendingContinuationNodeId` state to `TreeNavigationPage`. When the user selects a descendant:
|
||||
|
||||
1. Store the selected node ID in `pendingContinuationNodeId`
|
||||
2. Close the ContinuationModal (user is now viewing their custom step)
|
||||
3. Render a "Continue to: [node name]" button on the custom step view
|
||||
4. Clicking it navigates to the descendant and clears `pendingContinuationNodeId`
|
||||
|
||||
The "Continue to" button and custom branch controls are mutually exclusive. If the user chose "Build Custom Branch", they get the existing Add Another Step / This Solves My Issue controls instead.
|
||||
|
||||
**Files:** `frontend/src/pages/TreeNavigationPage.tsx`
|
||||
|
||||
## Summary of state changes
|
||||
|
||||
| State | Purpose |
|
||||
|-------|---------|
|
||||
| `pendingContinuationNodeId` | Stores which descendant the user selected, so the custom step can show a "Continue to" button |
|
||||
|
||||
## No backend changes required
|
||||
346
docs/archive/2026-02-04-complete-rebrand.md
Normal file
346
docs/archive/2026-02-04-complete-rebrand.md
Normal file
@@ -0,0 +1,346 @@
|
||||
# Complete Rebrand: Patherly → ResolutionFlow
|
||||
|
||||
> **For Claude:** REQUIRED SUB-SKILL: Use superpowers:executing-plans to implement this plan task-by-task.
|
||||
|
||||
**Goal:** Replace every remaining "Patherly" reference in the codebase with "ResolutionFlow" so the brand is consistent across frontend, backend, docs, and comments.
|
||||
|
||||
**Architecture:** The frontend rebrand is already complete (PR #26). This plan covers the backend (config, API metadata, log messages, model docstrings, seed scripts), the root README, and CLAUDE.md. No database schema changes — the database name `patherly` and Docker container `patherly_postgres` stay as-is (infrastructure identifiers, not user-facing brand).
|
||||
|
||||
**Tech Stack:** Python/FastAPI backend, Markdown docs
|
||||
|
||||
---
|
||||
|
||||
## Scope Summary
|
||||
|
||||
| Area | Files | Type of Change |
|
||||
|------|-------|---------------|
|
||||
| Backend config | `config.py` | APP_NAME value |
|
||||
| Backend API | `main.py` | OpenAPI metadata, log messages, root endpoint |
|
||||
| Backend models | `user.py`, `category.py`, `step_category.py` | Docstring/comment text |
|
||||
| Seed scripts | `seed_data.py`, `seed_trees.py` | Comments and print output |
|
||||
| Root docs | `README.md` | Full rewrite of heading and tagline |
|
||||
| Project context | `CLAUDE.md` | Update branding table and env var example |
|
||||
|
||||
**NOT changed** (infrastructure identifiers):
|
||||
- Database name `patherly`, Docker container `patherly_postgres`
|
||||
- Repository directory name `patherly/`
|
||||
- Production URLs `app.patherly.com` / `api.patherly.com`
|
||||
- Railway service name `patherly`
|
||||
- File paths like `C:\Dev\Projects\patherly`
|
||||
|
||||
---
|
||||
|
||||
### Task 1: Backend Config — APP_NAME
|
||||
|
||||
**Files:**
|
||||
- Modify: `backend/app/core/config.py:8`
|
||||
|
||||
**Step 1: Update APP_NAME**
|
||||
|
||||
Change line 8 from:
|
||||
```python
|
||||
APP_NAME: str = "Patherly"
|
||||
```
|
||||
to:
|
||||
```python
|
||||
APP_NAME: str = "ResolutionFlow"
|
||||
```
|
||||
|
||||
**Step 2: Verify no other references in config.py**
|
||||
|
||||
Search `config.py` for "Patherly" — should return 0 matches after the edit.
|
||||
|
||||
---
|
||||
|
||||
### Task 2: Backend API — main.py
|
||||
|
||||
**Files:**
|
||||
- Modify: `backend/app/main.py:21,28,33,47,74`
|
||||
|
||||
**Step 1: Update startup log (line 21)**
|
||||
|
||||
```python
|
||||
# FROM:
|
||||
logger.info("Starting Patherly API server...")
|
||||
# TO:
|
||||
logger.info("Starting ResolutionFlow API server...")
|
||||
```
|
||||
|
||||
**Step 2: Update shutdown log (line 28)**
|
||||
|
||||
```python
|
||||
# FROM:
|
||||
logger.info("Shutting down Patherly API server...")
|
||||
# TO:
|
||||
logger.info("Shutting down ResolutionFlow API server...")
|
||||
```
|
||||
|
||||
**Step 3: Update OpenAPI description (line 33)**
|
||||
|
||||
```python
|
||||
# FROM:
|
||||
description="Patherly - Take the path MOST traveled. Guided troubleshooting with automatic documentation.",
|
||||
# TO:
|
||||
description="ResolutionFlow - Take the path MOST traveled. Guided troubleshooting with automatic documentation.",
|
||||
```
|
||||
|
||||
**Step 4: Update CORS comment (line 47)**
|
||||
|
||||
```python
|
||||
# FROM:
|
||||
# PLUS the explicit allowed_origins list (for custom domains like app.patherly.com)
|
||||
# TO:
|
||||
# PLUS the explicit allowed_origins list (for custom domains like app.resolutionflow.com)
|
||||
```
|
||||
|
||||
Note: The actual CORS origin values in `config.py` are correct as-is (they use the real domain). This is just a comment.
|
||||
|
||||
**Step 5: Update root endpoint response (line 74)**
|
||||
|
||||
```python
|
||||
# FROM:
|
||||
"message": "Patherly API",
|
||||
# TO:
|
||||
"message": "ResolutionFlow API",
|
||||
```
|
||||
|
||||
**Step 6: Run backend tests**
|
||||
|
||||
Run: `cd backend && python -m pytest -x -q`
|
||||
Expected: All tests pass (no test references "Patherly" in assertions)
|
||||
|
||||
**Step 7: Commit**
|
||||
|
||||
```bash
|
||||
git add backend/app/core/config.py backend/app/main.py
|
||||
git commit -m "feat: Rebrand backend config and API metadata to ResolutionFlow"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Task 3: Backend Model Docstrings
|
||||
|
||||
**Files:**
|
||||
- Modify: `backend/app/models/user.py:53`
|
||||
- Modify: `backend/app/models/category.py:19`
|
||||
- Modify: `backend/app/models/step_category.py:18`
|
||||
|
||||
**Step 1: Update user.py docstring (line 53)**
|
||||
|
||||
```python
|
||||
# FROM:
|
||||
"""Returns True if user is a global (Patherly) admin."""
|
||||
# TO:
|
||||
"""Returns True if user is a global (ResolutionFlow) admin."""
|
||||
```
|
||||
|
||||
**Step 2: Update category.py docstring (line 19)**
|
||||
|
||||
```python
|
||||
# FROM:
|
||||
- Global (team_id=NULL): Created by Patherly admins, visible to all
|
||||
# TO:
|
||||
- Global (team_id=NULL): Created by ResolutionFlow admins, visible to all
|
||||
```
|
||||
|
||||
**Step 3: Update step_category.py docstring (line 18)**
|
||||
|
||||
```python
|
||||
# FROM:
|
||||
- Global (team_id=NULL): Created by Patherly admins, visible to all
|
||||
# TO:
|
||||
- Global (team_id=NULL): Created by ResolutionFlow admins, visible to all
|
||||
```
|
||||
|
||||
**Step 4: Commit**
|
||||
|
||||
```bash
|
||||
git add backend/app/models/user.py backend/app/models/category.py backend/app/models/step_category.py
|
||||
git commit -m "docs: Update backend model docstrings from Patherly to ResolutionFlow"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Task 4: Seed Scripts
|
||||
|
||||
**Files:**
|
||||
- Modify: `backend/scripts/seed_data.py:3,409,461`
|
||||
- Modify: `backend/scripts/seed_trees.py:5,3437`
|
||||
|
||||
**Step 1: Update seed_data.py docstring (line 3)**
|
||||
|
||||
```python
|
||||
# FROM:
|
||||
Seed data script for Patherly decision trees.
|
||||
# TO:
|
||||
Seed data script for ResolutionFlow decision trees.
|
||||
```
|
||||
|
||||
**Step 2: Update seed_data.py print output (line 409)**
|
||||
|
||||
```python
|
||||
# FROM:
|
||||
print("\n[*] Patherly Database Seeder")
|
||||
# TO:
|
||||
print("\n[*] ResolutionFlow Database Seeder")
|
||||
```
|
||||
|
||||
**Step 3: Update seed_data.py argparse description (line 461)**
|
||||
|
||||
```python
|
||||
# FROM:
|
||||
parser = argparse.ArgumentParser(description="Seed the Patherly database with example trees")
|
||||
# TO:
|
||||
parser = argparse.ArgumentParser(description="Seed the ResolutionFlow database with example trees")
|
||||
```
|
||||
|
||||
**Step 4: Update seed_trees.py docstring (line 5)**
|
||||
|
||||
```python
|
||||
# FROM:
|
||||
This script populates Patherly with realistic troubleshooting decision trees
|
||||
# TO:
|
||||
This script populates ResolutionFlow with realistic troubleshooting decision trees
|
||||
```
|
||||
|
||||
**Step 5: Update seed_trees.py argparse description (line 3437)**
|
||||
|
||||
```python
|
||||
# FROM:
|
||||
description="Seed the Patherly database with MSP/SMB troubleshooting trees"
|
||||
# TO:
|
||||
description="Seed the ResolutionFlow database with MSP/SMB troubleshooting trees"
|
||||
```
|
||||
|
||||
**Step 6: Commit**
|
||||
|
||||
```bash
|
||||
git add backend/scripts/seed_data.py backend/scripts/seed_trees.py
|
||||
git commit -m "docs: Update seed script references from Patherly to ResolutionFlow"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Task 5: README.md
|
||||
|
||||
**Files:**
|
||||
- Modify: `README.md` (full update)
|
||||
|
||||
**Step 1: Update README heading and tagline (lines 1-2)**
|
||||
|
||||
```markdown
|
||||
# FROM:
|
||||
# Patherly
|
||||
|
||||
> Take the path MOST traveled.
|
||||
|
||||
# TO:
|
||||
# ResolutionFlow
|
||||
|
||||
> Take the path MOST traveled.
|
||||
```
|
||||
|
||||
**Step 2: Search the rest of README for "Patherly"**
|
||||
|
||||
The README is a legacy planning doc from early in the project. It doesn't reference "Patherly" elsewhere in the body text. Only the heading needs updating.
|
||||
|
||||
**Step 3: Commit**
|
||||
|
||||
```bash
|
||||
git add README.md
|
||||
git commit -m "docs: Update README heading from Patherly to ResolutionFlow"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Task 6: CLAUDE.md — Branding Table and Env Var
|
||||
|
||||
**Files:**
|
||||
- Modify: `CLAUDE.md:25,236`
|
||||
|
||||
**Step 1: Update branding table (line 25)**
|
||||
|
||||
The branding table row for Backend APP_NAME should now reflect the change:
|
||||
|
||||
```markdown
|
||||
# FROM:
|
||||
| Backend (FastAPI, env vars, APP_NAME) | Patherly |
|
||||
# TO:
|
||||
| Backend (FastAPI, env vars, APP_NAME) | ResolutionFlow |
|
||||
```
|
||||
|
||||
**Step 2: Update env var example (line 236)**
|
||||
|
||||
```bash
|
||||
# FROM:
|
||||
APP_NAME=Patherly
|
||||
# TO:
|
||||
APP_NAME=ResolutionFlow
|
||||
```
|
||||
|
||||
**Step 3: Update CORS comment references (lines 471, 476)**
|
||||
|
||||
These reference `app.patherly.com` which is the current production URL. Keep as-is — they're accurate infrastructure references, not brand names.
|
||||
|
||||
**Step 4: Commit**
|
||||
|
||||
```bash
|
||||
git add CLAUDE.md
|
||||
git commit -m "docs: Update CLAUDE.md to reflect backend rebrand to ResolutionFlow"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Task 7: Final Verification
|
||||
|
||||
**Step 1: Search entire codebase for remaining "Patherly" references**
|
||||
|
||||
Run: `grep -r "Patherly" --include="*.py" --include="*.ts" --include="*.tsx" --include="*.md" --include="*.html" --include="*.css" --include="*.json" .`
|
||||
|
||||
**Expected remaining matches (all legitimate):**
|
||||
- `CLAUDE.md` — only in the branding table context rows (repo name, database, URLs) and historical rebrand description
|
||||
- `REBRAND-IMPLEMENTATION-GUIDE.md` — historical guide documenting the rebrand process
|
||||
- `docs/plans/2026-02-04-complete-rebrand.md` — this plan file itself
|
||||
|
||||
**No matches should appear in:**
|
||||
- `backend/app/**/*.py`
|
||||
- `frontend/src/**/*`
|
||||
- `README.md`
|
||||
|
||||
**Step 2: Run frontend build**
|
||||
|
||||
Run: `cd frontend && npm run build`
|
||||
Expected: Build succeeds with no errors
|
||||
|
||||
**Step 3: Run backend tests**
|
||||
|
||||
Run: `cd backend && python -m pytest -x -q`
|
||||
Expected: All tests pass
|
||||
|
||||
**Step 4: Push**
|
||||
|
||||
```bash
|
||||
git push origin main
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Risk Assessment
|
||||
|
||||
**Risk: Backend tests break after APP_NAME change**
|
||||
- Likelihood: Low — no tests assert on APP_NAME or the root endpoint message
|
||||
- Mitigation: Run `pytest` after Task 2
|
||||
|
||||
**Risk: Production deployment affected**
|
||||
- Likelihood: None — APP_NAME only affects OpenAPI metadata and log messages
|
||||
- Mitigation: Railway auto-deploys on merge, no config change needed (APP_NAME is a code default, not an env var override)
|
||||
|
||||
**Risk: `.env` file has `APP_NAME=Patherly` hardcoded**
|
||||
- Note: The `.env` file may override the code default. If so, update `.env` to `APP_NAME=ResolutionFlow` locally. Railway env vars may also need updating in the dashboard.
|
||||
- This plan updates the code default. The `.env` file and Railway dashboard are manual steps flagged below.
|
||||
|
||||
## Manual Steps (Not in Git)
|
||||
|
||||
1. **Local `.env`**: If `backend/.env` has `APP_NAME=Patherly`, update to `APP_NAME=ResolutionFlow`
|
||||
2. **Railway Dashboard**: If `APP_NAME` is set as an env var in Railway, update it there too
|
||||
3. **Domain rename** (future): If/when `patherly.com` → `resolutionflow.com`, update production URLs in CLAUDE.md, config.py CORS, and Railway settings
|
||||
63
docs/archive/2026-02-04-scratchpad-overlay-design.md
Normal file
63
docs/archive/2026-02-04-scratchpad-overlay-design.md
Normal file
@@ -0,0 +1,63 @@
|
||||
# Scratchpad Floating Overlay Design
|
||||
|
||||
> **Date:** 2026-02-04
|
||||
> **Status:** Approved
|
||||
|
||||
---
|
||||
|
||||
## Problem
|
||||
|
||||
The scratchpad is a full-height sidebar that pushes main content left when open, reducing the available width for tree navigation. On smaller screens this is particularly disruptive. It should be a floating overlay that doesn't affect layout.
|
||||
|
||||
## Design
|
||||
|
||||
### Component: ScratchpadSidebar.tsx
|
||||
|
||||
Single component with two visual states, both using `position: fixed`.
|
||||
|
||||
**Closed — floating button:**
|
||||
- Fixed to viewport right edge, vertically centered
|
||||
- `right: 0; top: 50%; transform: translateY(-50%); z-index: 40`
|
||||
- Rounded left corners, flat right edge (`rounded-l-md`)
|
||||
- StickyNote icon
|
||||
- Amber dot (absolute positioned) when unsaved changes exist
|
||||
|
||||
**Open — overlay panel:**
|
||||
- Fixed to viewport right edge, vertically centered
|
||||
- `right: 0; top: 50%; transform: translateY(-50%); z-index: 40`
|
||||
- Width: 420px, height: 55vh
|
||||
- Rounded left corners, flat right edge (`rounded-l-lg`)
|
||||
- Shadow (`shadow-xl`) and left border for depth
|
||||
- Slide-in animation: `translateX(100%)` → `translateX(0)`, 200ms ease-out
|
||||
- No backdrop — clicking outside does NOT close it
|
||||
|
||||
**Panel contents (unchanged):**
|
||||
- Header: title, preview toggle (Eye/Pencil), close button (X)
|
||||
- Body: textarea or markdown preview, scrollable
|
||||
- Footer: save status indicator
|
||||
|
||||
### Keyboard Shortcut
|
||||
|
||||
`Ctrl+/` toggles open/closed. Handled via `useEffect` keydown listener inside the component.
|
||||
|
||||
### TreeNavigationPage.tsx
|
||||
|
||||
- Outer wrapper no longer uses `flex` for sidebar layout
|
||||
- Main content takes full width
|
||||
- ScratchpadSidebar renders in same DOM position (fixed positioning makes it layout-independent)
|
||||
|
||||
### Preserved Behavior
|
||||
|
||||
- Auto-save with 1000ms debounce
|
||||
- Markdown preview toggle
|
||||
- Save status indicators (Saving.../Saved/Unsaved changes)
|
||||
- localStorage persistence for open/closed state (key: `scratchpad-collapsed`)
|
||||
- beforeunload warning for unsaved changes
|
||||
- Session ID reset on navigation
|
||||
|
||||
## Files Changed
|
||||
|
||||
| File | Change |
|
||||
|------|--------|
|
||||
| `frontend/src/components/session/ScratchpadSidebar.tsx` | Refactor to fixed-position floating overlay |
|
||||
| `frontend/src/pages/TreeNavigationPage.tsx` | Remove flex sidebar layout |
|
||||
276
docs/archive/2026-02-04-session-scratchpad-design.md
Normal file
276
docs/archive/2026-02-04-session-scratchpad-design.md
Normal file
@@ -0,0 +1,276 @@
|
||||
# Session Scratchpad Design
|
||||
|
||||
> **Date:** February 4, 2026
|
||||
> **Source:** Feature Ideas Brainstorm - Idea 6
|
||||
> **Priority:** Must-have (per Michael)
|
||||
> **Category:** Context capture
|
||||
|
||||
---
|
||||
|
||||
## Overview
|
||||
|
||||
A collapsible right sidebar during active sessions for capturing ambient data — IP addresses, error codes, server names, usernames — anything that doesn't fit a specific decision node's notes field.
|
||||
|
||||
**Core principle:** Engineers already accumulate scraps of data during troubleshooting (from `ipconfig`, Event Viewer, phone conversations) that live on sticky notes or in their head. This gives it a home and includes it automatically in the export.
|
||||
|
||||
**Data model:** Single freeform `Text` column on the `sessions` table with markdown formatting support. Not JSONB — we're starting simple with a plain text field that the engineer writes in freely.
|
||||
|
||||
**Save behavior:** Auto-save with 1000ms debounce. No manual save button.
|
||||
|
||||
**Layout:** Collapsible right sidebar in `TreeNavigationPage`, 300px when open, 48px toggle strip when collapsed.
|
||||
|
||||
---
|
||||
|
||||
## Section 1: Data Model
|
||||
|
||||
### Database
|
||||
|
||||
New `scratchpad` column on `sessions` table:
|
||||
|
||||
```python
|
||||
# SQLAlchemy model (session.py)
|
||||
scratchpad: Mapped[Optional[str]] = mapped_column(Text, nullable=True, server_default=sa.text("''"))
|
||||
```
|
||||
|
||||
### Migration
|
||||
|
||||
```python
|
||||
# alembic revision -m "add scratchpad to sessions"
|
||||
def upgrade():
|
||||
op.add_column('sessions', sa.Column('scratchpad', sa.Text(), nullable=True, server_default=sa.text("''")))
|
||||
# Backfill existing rows
|
||||
op.execute("UPDATE sessions SET scratchpad = '' WHERE scratchpad IS NULL")
|
||||
|
||||
def downgrade():
|
||||
op.drop_column('sessions', 'scratchpad')
|
||||
```
|
||||
|
||||
- `server_default=sa.text("''")` ensures new rows get `""` at the database level
|
||||
- Backfill normalizes existing rows so no code path sees `NULL`
|
||||
|
||||
### Schemas
|
||||
|
||||
**SessionUpdate** — add field:
|
||||
```python
|
||||
scratchpad: Optional[str] = None
|
||||
```
|
||||
|
||||
**SessionResponse** — add field with validator:
|
||||
```python
|
||||
scratchpad: str = ""
|
||||
|
||||
@validator('scratchpad', pre=True, always=True)
|
||||
def normalize_scratchpad(cls, v):
|
||||
return v or ""
|
||||
```
|
||||
|
||||
**New schema — ScratchpadUpdate:**
|
||||
```python
|
||||
class ScratchpadUpdate(BaseModel):
|
||||
scratchpad: str
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Section 2: Frontend Component Design
|
||||
|
||||
### Layout Restructuring
|
||||
|
||||
Current `TreeNavigationPage` is a single-column `container mx-auto` div. Restructure to:
|
||||
|
||||
```
|
||||
┌─────────────────────────────────────┬──────────────┐
|
||||
│ Existing tree navigation content │ Scratchpad │
|
||||
│ (header, breadcrumb, node card, │ sidebar │
|
||||
│ notes, back button, shortcuts) │ (300px) │
|
||||
│ │ │
|
||||
└─────────────────────────────────────┴──────────────┘
|
||||
```
|
||||
|
||||
- Outer wrapper: `flex` layout
|
||||
- Main content: `flex-1 min-w-0` (prevents overflow)
|
||||
- Sidebar: Fixed 300px open, 48px collapsed
|
||||
- Collapse state persisted in `localStorage` as `scratchpad-collapsed`
|
||||
|
||||
### ScratchpadSidebar Component
|
||||
|
||||
**File:** `frontend/src/components/session/ScratchpadSidebar.tsx`
|
||||
|
||||
**Props:**
|
||||
```typescript
|
||||
interface ScratchpadSidebarProps {
|
||||
sessionId: string
|
||||
initialContent: string
|
||||
onSave: (content: string) => Promise<void>
|
||||
}
|
||||
```
|
||||
|
||||
**Internal state:**
|
||||
- `content: string` — textarea value, initialized from `initialContent`, **only reset when `sessionId` changes** (via `useEffect` with `[sessionId]` dependency)
|
||||
- `isCollapsed: boolean` — sidebar open/closed, persisted in localStorage
|
||||
- `isSaving: boolean` — network request in flight
|
||||
- `hasUnsavedChanges: boolean` — `content !== lastSaved`
|
||||
- `lastSaved: string` — last successfully saved content
|
||||
- `showPreview: boolean` — toggle between edit and markdown preview
|
||||
|
||||
### Save Flow
|
||||
|
||||
1. User types → `content` updates → `hasUnsavedChanges = (content !== lastSaved)`
|
||||
2. After 1000ms of no typing (debounce), if `hasUnsavedChanges`, fire `onSave(content)`
|
||||
3. Set `isSaving = true` during request
|
||||
4. On success: `lastSaved = content`, `isSaving = false`, `hasUnsavedChanges = false`
|
||||
5. On blur: trigger immediate save if `hasUnsavedChanges` (cancel pending debounce)
|
||||
6. On error: keep `hasUnsavedChanges = true`, show error indicator
|
||||
|
||||
### Save Indicator (bottom of sidebar)
|
||||
|
||||
| State | Display |
|
||||
|-------|---------|
|
||||
| Idle, no changes | Nothing shown |
|
||||
| Typing, unsaved | "Unsaved changes" (muted text) |
|
||||
| Saving | "Saving..." with subtle spinner |
|
||||
| Just saved | "Saved" with checkmark (green, fades after 2s) |
|
||||
|
||||
### beforeunload Warning
|
||||
|
||||
- Register `window.addEventListener('beforeunload', handler)` only when `hasUnsavedChanges === true`
|
||||
- Handler calls `e.preventDefault()` to trigger browser's "unsaved changes" dialog
|
||||
- Clean up listener on unmount
|
||||
|
||||
### Markdown Preview
|
||||
|
||||
- Small toggle link below textarea: "Preview" / "Edit"
|
||||
- Preview renders current `content` using existing `MarkdownContent` component
|
||||
- No new dependencies required
|
||||
|
||||
### Dimensions & Animation
|
||||
|
||||
- Open width: 300px
|
||||
- Collapsed width: 48px (notepad icon button)
|
||||
- Transition: `width 200ms ease`
|
||||
- Toggle icon: Notepad/pencil icon from Lucide
|
||||
|
||||
---
|
||||
|
||||
## Section 3: Backend & API
|
||||
|
||||
### New Endpoint
|
||||
|
||||
```
|
||||
PATCH /api/v1/sessions/{id}/scratchpad
|
||||
```
|
||||
|
||||
**Request body:**
|
||||
```json
|
||||
{ "scratchpad": "- Server IP: 192.168.1.50\n- Error: 0x80070005" }
|
||||
```
|
||||
|
||||
**Response:** `SessionResponse` (full session object)
|
||||
|
||||
**Handler logic:**
|
||||
1. Load session by ID
|
||||
2. Verify `session.user_id == current_user.id` (ownership check)
|
||||
3. Set `session.scratchpad = data.scratchpad`
|
||||
4. Commit and return updated session
|
||||
|
||||
This is intentionally separate from the existing PUT endpoint. Auto-saving the scratchpad every second should not send the full session payload (`path_taken`, `decisions`, `custom_steps`) — that risks overwriting concurrent changes from decision tracking in the same session.
|
||||
|
||||
### Frontend API Client
|
||||
|
||||
```typescript
|
||||
// sessions.ts
|
||||
updateScratchpad: (id: string, content: string) =>
|
||||
api.patch<Session>(`/api/v1/sessions/${id}/scratchpad`, { scratchpad: content })
|
||||
```
|
||||
|
||||
### Router Registration
|
||||
|
||||
Add to `backend/app/api/router.py` — the endpoint lives in the existing `sessions.py` endpoints file.
|
||||
|
||||
---
|
||||
|
||||
## Section 4: Export Integration
|
||||
|
||||
Add a "Evidence / Reference" section to all three export generators, inserted **between the header block and "Troubleshooting Steps"**. Only rendered when `scratchpad.strip()` is non-empty.
|
||||
|
||||
### Markdown Export
|
||||
|
||||
```markdown
|
||||
## Evidence / Reference
|
||||
|
||||
- Server IP: 192.168.1.50
|
||||
- Error code: 0x80070005
|
||||
- Affected user: jsmith@contoso.com
|
||||
|
||||
---
|
||||
|
||||
## Troubleshooting Steps
|
||||
```
|
||||
|
||||
Scratchpad content rendered as-is (user writes markdown, it appears as markdown).
|
||||
|
||||
### Text Export
|
||||
|
||||
```
|
||||
EVIDENCE / REFERENCE
|
||||
--------------------
|
||||
- Server IP: 192.168.1.50
|
||||
- Error code: 0x80070005
|
||||
- Affected user: jsmith@contoso.com
|
||||
|
||||
TROUBLESHOOTING STEPS
|
||||
---------------------
|
||||
```
|
||||
|
||||
### HTML Export
|
||||
|
||||
```html
|
||||
<h2>Evidence / Reference</h2>
|
||||
<div class="scratchpad" style="white-space: pre-wrap; background: #f9f9f9; padding: 12px; border-radius: 4px; margin-bottom: 20px;">
|
||||
- Server IP: 192.168.1.50
|
||||
- Error code: 0x80070005
|
||||
</div>
|
||||
<h2>Troubleshooting Steps</h2>
|
||||
```
|
||||
|
||||
Start with `pre-wrap` for HTML export. Future enhancement: render scratchpad markdown to HTML.
|
||||
|
||||
---
|
||||
|
||||
## Files Changed
|
||||
|
||||
| File | Action | Description |
|
||||
|------|--------|-------------|
|
||||
| `backend/app/models/session.py` | Modify | Add `scratchpad` field |
|
||||
| `backend/app/schemas/session.py` | Modify | Add to `SessionUpdate`, `SessionResponse` + new `ScratchpadUpdate` |
|
||||
| `backend/app/api/endpoints/sessions.py` | Modify | Add PATCH endpoint + scratchpad in all 3 export generators |
|
||||
| `backend/app/api/router.py` | Modify | Register new PATCH route |
|
||||
| `backend/alembic/versions/xxx_add_scratchpad.py` | Create | Migration with backfill |
|
||||
| `frontend/src/components/session/ScratchpadSidebar.tsx` | Create | New sidebar component |
|
||||
| `frontend/src/pages/TreeNavigationPage.tsx` | Modify | Add flex layout + render sidebar |
|
||||
| `frontend/src/api/sessions.ts` | Modify | Add `updateScratchpad` method |
|
||||
| `frontend/src/types/session.ts` | Modify | Add `scratchpad` to Session type |
|
||||
|
||||
---
|
||||
|
||||
## Design Decisions
|
||||
|
||||
| Decision | Choice | Rationale |
|
||||
|----------|--------|-----------|
|
||||
| Storage type | `Text` column (not JSONB) | Freeform text is simpler; structured entries are a future enhancement |
|
||||
| Save mechanism | Auto-save with 1000ms debounce | Reduces friction — engineer never thinks about saving |
|
||||
| API endpoint | Dedicated PATCH (not reuse PUT) | Prevents scratchpad auto-saves from overwriting concurrent decision updates |
|
||||
| Conflict detection | None (last-write-wins) | Sessions are single-user; multi-user requires broader solution |
|
||||
| Markdown rendering | Preview toggle (not live) | Keeps the editing experience simple; preview is optional |
|
||||
| Collapse persistence | localStorage | Survives page refreshes, per-browser preference |
|
||||
|
||||
---
|
||||
|
||||
## Pairs With (Future)
|
||||
|
||||
- **Command Output Capture** (Idea 3): Structured output at nodes + freeform notes in scratchpad = complete evidence
|
||||
- **Share Progress** (Idea 2): Scratchpad content included in shared summary
|
||||
|
||||
---
|
||||
|
||||
*Design finalized during brainstorming session, February 4, 2026*
|
||||
1069
docs/archive/2026-02-04-session-scratchpad-implementation.md
Normal file
1069
docs/archive/2026-02-04-session-scratchpad-implementation.md
Normal file
File diff suppressed because it is too large
Load Diff
81
docs/archive/2026-02-04-token-refresh-fix-design.md
Normal file
81
docs/archive/2026-02-04-token-refresh-fix-design.md
Normal file
@@ -0,0 +1,81 @@
|
||||
# Token Refresh Fix Design
|
||||
|
||||
> **Date:** 2026-02-04
|
||||
> **Status:** Approved
|
||||
> **Issue:** After idle period, app shows "Failed to load trees" with 401 cascade
|
||||
|
||||
---
|
||||
|
||||
## Problem
|
||||
|
||||
When the access token expires (15 minutes), the Axios response interceptor attempts to refresh it by calling `POST /api/v1/auth/refresh`. This call fails because of a request/response mismatch:
|
||||
|
||||
- **Frontend** sends the refresh token in the `Authorization: Bearer <token>` header
|
||||
- **Backend** expects `refresh_token` as a bare `str` query/body parameter
|
||||
|
||||
FastAPI cannot extract a bare string parameter from the Authorization header, so the refresh call returns 422 (missing required parameter). The interceptor catches this as a generic failure, but the error propagates to the calling component rather than redirecting to login — leaving the user stuck on a broken page.
|
||||
|
||||
Additionally, when multiple API calls (trees, folders, categories) all return 401 simultaneously, each independently triggers its own refresh attempt, creating a race condition.
|
||||
|
||||
## Root Cause
|
||||
|
||||
`backend/app/api/endpoints/auth.py:156-158`:
|
||||
```python
|
||||
async def refresh_token(
|
||||
refresh_token: str, # FastAPI expects query/body param
|
||||
db: ...
|
||||
)
|
||||
```
|
||||
|
||||
`frontend/src/api/client.ts:37-41`:
|
||||
```typescript
|
||||
await axios.post(url, null, {
|
||||
headers: { Authorization: `Bearer ${refreshToken}` } // Sent as header
|
||||
})
|
||||
```
|
||||
|
||||
## Solution
|
||||
|
||||
### 1. Backend: Add `get_refresh_token_payload` dependency
|
||||
|
||||
Create a proper FastAPI dependency in `deps.py` that extracts and validates a refresh token from the Authorization header, mirroring the existing `get_current_user` pattern:
|
||||
|
||||
```python
|
||||
async def get_refresh_token_payload(
|
||||
token: Annotated[str, Depends(oauth2_scheme)]
|
||||
) -> dict:
|
||||
payload = decode_token(token)
|
||||
if payload is None or payload.get("type") != "refresh":
|
||||
raise HTTPException(status_code=401, detail="Invalid refresh token")
|
||||
return payload
|
||||
```
|
||||
|
||||
### 2. Backend: Refactor refresh endpoint
|
||||
|
||||
Change `refresh_token()` in `auth.py` to use the new dependency instead of a bare string parameter.
|
||||
|
||||
### 3. Frontend: Add refresh queue (single-flight pattern)
|
||||
|
||||
When multiple requests hit 401 simultaneously, only the first triggers the actual refresh call. Others queue up and retry with the new token once the refresh completes.
|
||||
|
||||
### 4. Frontend: Sync auth store after refresh
|
||||
|
||||
Add a `setTokens` action to `authStore.ts`. Call it from the interceptor after a successful refresh so the Zustand store stays consistent with localStorage.
|
||||
|
||||
## Files Changed
|
||||
|
||||
| File | Change |
|
||||
|------|--------|
|
||||
| `backend/app/api/deps.py` | Add `get_refresh_token_payload` dependency |
|
||||
| `backend/app/api/endpoints/auth.py` | Use new dependency in refresh endpoint |
|
||||
| `frontend/src/api/client.ts` | Refresh queue + auth store sync |
|
||||
| `frontend/src/store/authStore.ts` | Add `setTokens` action |
|
||||
|
||||
## Testing
|
||||
|
||||
- Set `ACCESS_TOKEN_EXPIRE_MINUTES=1` temporarily to reproduce quickly
|
||||
- Verify silent refresh: login, wait >1 min, interact — no errors visible
|
||||
- Verify concurrent requests: page load after expiry triggers one refresh, all requests succeed
|
||||
- Verify refresh token expiry: after 7 days, clean redirect to login
|
||||
- Run existing backend tests (`pytest`) to confirm no regressions
|
||||
- Run frontend build (`npm run build`) to confirm no compile errors
|
||||
188
docs/archive/2026-02-05-permissions-audit-design.md
Normal file
188
docs/archive/2026-02-05-permissions-audit-design.md
Normal file
@@ -0,0 +1,188 @@
|
||||
# Permissions & RBAC Audit — Design Document
|
||||
|
||||
> **Date:** 2026-02-05
|
||||
> **Status:** Updated after re-audit
|
||||
> **Scope:** Full-stack permissions audit across trees, steps, sessions, teams, and admin features
|
||||
|
||||
---
|
||||
|
||||
## Background
|
||||
|
||||
ResolutionFlow has a role-based access control system with a role hierarchy (`super_admin` > `team_admin` > `engineer` > `viewer`), backed by `is_super_admin` and `is_team_admin` boolean flags on the User model, plus team-scoped resources.
|
||||
|
||||
Two rounds of audit were performed:
|
||||
1. **Initial audit** — three independent reviews (UX, architecture, adversarial)
|
||||
2. **Re-audit** — after RBAC implementation commit (`34daa26`) added `is_super_admin`, `permissions.py`, `usePermissions.ts`, and updated endpoint permission checks
|
||||
|
||||
**Audit methodology:** Each round used three specialized reviewers:
|
||||
- Frontend UX audit — every page, component, API client, store, and router
|
||||
- Backend architecture audit — every endpoint, dependency, model, schema, and test
|
||||
- Gap analysis / devil's advocate — security gaps, edge cases, challenged assumptions
|
||||
|
||||
---
|
||||
|
||||
## Current Architecture (Post-RBAC Commit)
|
||||
|
||||
### What Was Added in Commit `34daa26`
|
||||
|
||||
- `is_super_admin` boolean on User model (migration 010)
|
||||
- `backend/app/core/permissions.py` — centralized permission functions (role hierarchy, content guards)
|
||||
- `frontend/src/hooks/usePermissions.ts` — permission hook with role hierarchy
|
||||
- `require_admin` dependency now checks `is_super_admin` instead of `role == "admin"`
|
||||
- `require_engineer_or_admin` dependency blocks viewers from tree/step creation
|
||||
- `usePermissions` hook used in TreeLibraryPage (edit/create gating) and TreeEditorPage (access guard)
|
||||
- Role badge display in AppLayout header
|
||||
- `TreeListItem` type includes `team_id` for frontend permission checks
|
||||
|
||||
### What Works
|
||||
|
||||
- **Session ownership** is properly enforced — users can only see/modify their own sessions
|
||||
- **Folder ownership** is properly scoped to the creating user
|
||||
- **Tree CRUD** has backend permission checks (engineer+ to create, author/admin/team-admin to edit, super-admin-only delete)
|
||||
- **Step Library** enforces visibility levels (private/team/public) and ownership for edit/delete
|
||||
- **Category/Tag management** is properly gated to super admin and team admin roles
|
||||
- **Invite code management** is super-admin-only
|
||||
- **Frontend gating (partial):** Create Tree button hidden from viewers, Edit button checks ownership/team/admin
|
||||
- **`is_super_admin` flag** prevents the old admin role escalation (flag has `server_default=false`)
|
||||
|
||||
### What's Still Broken
|
||||
|
||||
The permission system still has critical vulnerabilities, a dead-code centralized module, and significant gaps.
|
||||
|
||||
---
|
||||
|
||||
## Findings — Combined Status Table
|
||||
|
||||
### Legend
|
||||
- **FIXED** — Issue fully resolved
|
||||
- **PARTIAL** — Infrastructure exists but incomplete or not fully wired
|
||||
- **OPEN** — No change from original audit
|
||||
|
||||
| # | Severity | Finding | Status | Evidence |
|
||||
|---|----------|---------|--------|----------|
|
||||
| 1 | CRITICAL | Self-assignable role at registration | **OPEN** | `UserCreate` still has `role` field (`schemas/user.py:14`). `auth.py:78` passes `user_data.role` to User. No enum validation — arbitrary strings accepted. The `is_super_admin` escalation is blocked (server default false), but `role` field is unvalidated. |
|
||||
| 2 | CRITICAL | XSS in HTML export | **OPEN** | `sessions.py:349-405` — raw f-string interpolation without `html.escape()`. All user content (tree_name, ticket_number, client_name, question, answer, notes, scratchpad) injected unescaped. |
|
||||
| 3 | CRITICAL | Default secret key in source code | **OPEN** | `config.py:29` — default string with no production validation. |
|
||||
| 4 | HIGH | No access control on `start_session` | **OPEN** | `sessions.py:70-109` — checks tree existence and active status only. No tree access check. Any authenticated user can read any tree's full structure via session snapshot. |
|
||||
| 5 | HIGH | No token revocation | **OPEN** | Logout (`auth.py:188-193`) is still a no-op. JWTs valid until expiry. |
|
||||
| 6 | HIGH | No account deactivation (`is_active`) | **OPEN** | No `is_active` field. `get_current_active_user` (`deps.py:66-72`) is a no-op. |
|
||||
| 7 | HIGH | No rate limiting | **OPEN** | No rate limiting on any endpoint. |
|
||||
| 8 | HIGH | Admin flags cannot be set via API | **PARTIAL** | `is_super_admin` field added with migration 010. But no API endpoint exists to set `is_super_admin` or `is_team_admin`. Must be set via direct DB access. |
|
||||
| 9 | MEDIUM | Frontend role awareness | **PARTIAL** | `usePermissions` hook exists, used in 3 files (TreeLibraryPage, TreeEditorPage, AppLayout). But: `ProtectedRoute` still auth-only, no route-level role guards, no 403 handling in Axios, no admin pages. |
|
||||
| 10 | MEDIUM | `None == None` team visibility bug | **PARTIAL** | Fixed in `trees.py:33` (`if current_user.team_id else False`). Still present in local helpers: `steps.py:36`, `categories.py:31`, `step_categories.py:28`. |
|
||||
| 11 | MEDIUM | Admin list vs get tree inconsistency | **OPEN** | `build_tree_access_filter` has no admin bypass. `get_tree` checks `is_super_admin`. |
|
||||
| 12 | MEDIUM | No audit logging | **OPEN** | No change. |
|
||||
| 13 | MEDIUM | No delete UI for trees or steps | **OPEN** | API methods exist, no buttons wired. `canDeleteTree` exists in hook but unused. |
|
||||
| 14 | MEDIUM | Node deletion without confirmation | **OPEN** | No change. |
|
||||
| 15 | LOW | Viewer role barely enforced | **PARTIAL** | `require_engineer_or_admin` blocks tree/step creation. Viewers can still create folders, start sessions, etc. |
|
||||
| 16 | LOW | No password complexity | **OPEN** | No change. |
|
||||
| 17 | LOW | Soft delete cascade cleanup | **OPEN** | No change. |
|
||||
| 18 | LOW | Debug endpoint in production | **OPEN** | No change. |
|
||||
| 19 | LOW | Tag search wildcard escaping | **OPEN** | No change. |
|
||||
| 20 | LOW | No team existence validation | **OPEN** | No change. |
|
||||
|
||||
**Summary: 0 FIXED / 4 PARTIAL / 16 OPEN**
|
||||
|
||||
---
|
||||
|
||||
## New Issues Found in Re-Audit
|
||||
|
||||
### N1. `permissions.py` is Dead Code (HIGH)
|
||||
|
||||
**Location:** `backend/app/core/permissions.py`
|
||||
|
||||
The centralized permissions module defines `can_edit_tree`, `can_delete_tree`, `can_edit_step`, `can_manage_category`, `can_manage_tree_tags`, `has_minimum_role`, `get_effective_role`, and `can_create_content`.
|
||||
|
||||
**None of these functions are imported or called by any endpoint.** Zero imports across the entire backend.
|
||||
|
||||
Each endpoint file (`trees.py`, `steps.py`, `categories.py`, `tags.py`, `step_categories.py`, `folders.py`) defines its own inline permission helpers that are similar but not identical. Key differences:
|
||||
|
||||
| Function in `permissions.py` | Local equivalent | Difference |
|
||||
|------------------------------|-----------------|------------|
|
||||
| `can_edit_tree` | `trees.py:403-407` inline | Local version doesn't call `can_create_content` |
|
||||
| `can_manage_category` | `categories.py:25-33` inline | Local version lacks `team_id is not None` guard |
|
||||
| `can_manage_step_category` | `step_categories.py:22-41` inline | Local version lacks `team_id is not None` guard |
|
||||
| `has_minimum_role` / `get_effective_role` | Not used anywhere | Entirely dead code |
|
||||
|
||||
**Risk:** False confidence that permissions are centralized. Maintaining two diverging copies.
|
||||
|
||||
### N2. `require_engineer_or_admin` Blocks Team Admins with Viewer Role (BUG)
|
||||
|
||||
**Location:** `backend/app/api/deps.py:87-98`
|
||||
|
||||
```python
|
||||
async def require_engineer_or_admin(current_user):
|
||||
if current_user.is_super_admin:
|
||||
return current_user
|
||||
if current_user.role not in ("engineer",):
|
||||
raise HTTPException(403)
|
||||
```
|
||||
|
||||
Checks `is_super_admin` but NOT `is_team_admin`. A user with `role="viewer"` + `is_team_admin=True` would be blocked from creating trees — contradicting the role hierarchy where team_admin > engineer.
|
||||
|
||||
### N3. 49 Endpoints Bypass `get_current_active_user` (HIGH)
|
||||
|
||||
**Location:** All endpoint files
|
||||
|
||||
Most endpoints depend directly on `get_current_user`, bypassing `get_current_active_user`. If `is_active` were implemented, deactivated users could still: list/view sessions, export, update scratchpads, manage folders, list/view categories, tags, steps, rate steps, etc.
|
||||
|
||||
### N4. `role` Field Accepts Arbitrary Strings (MEDIUM)
|
||||
|
||||
**Location:** `backend/app/schemas/user.py:14`, `backend/app/models/user.py:27`
|
||||
|
||||
The `role` field is `String(50)` with no DB constraint or schema validation. Users could register with `role="superadmin"`, `role="root"`, etc. While these don't grant permissions (only `is_super_admin` flag does), it creates inconsistent data. The `UserCreate` schema describes "engineer or viewer" but doesn't enforce it.
|
||||
|
||||
### N5. TreeEditorPage Checks `canCreateTrees` Not `canEditTree` (MEDIUM)
|
||||
|
||||
**Location:** `frontend/src/pages/TreeEditorPage.tsx:81-85`
|
||||
|
||||
The editor page uses `canCreateTrees` (a blanket role check) instead of `canEditTree(tree)` (ownership/team check). A non-author engineer can navigate to `/trees/{someone-else's-id}/edit` and the frontend won't redirect them — the backend will block the save, but the UX is poor (loads editor, then fails on save).
|
||||
|
||||
---
|
||||
|
||||
## MSP-Specific Concerns
|
||||
|
||||
1. **Weak multi-tenancy:** Super admin transcends all teams. Team A's super admin could access Team B's client troubleshooting data.
|
||||
2. **No data retention/purge:** Session scratchpads and notes contain sensitive client information with no cleanup mechanism.
|
||||
3. **No team-level session visibility:** Supervisors can't review their team's sessions.
|
||||
4. **Step Library IP leakage:** Public steps expose team-specific troubleshooting procedures to all teams.
|
||||
|
||||
---
|
||||
|
||||
## Challenged Assumptions
|
||||
|
||||
| Assumption | Reality |
|
||||
|-----------|---------|
|
||||
| "Invite codes protect registration" | They don't prevent arbitrary role strings — the role field is unvalidated |
|
||||
| "Team scoping = tenant isolation" | Super admin transcends all teams with no boundary |
|
||||
| "`permissions.py` centralizes permissions" | It's dead code — no endpoint imports it |
|
||||
| "JWT statelessness is acceptable" | For an MSP tool with client data, inability to immediately revoke access is a liability |
|
||||
| "`usePermissions` hook covers the frontend" | Only used in 3 of ~10 components that need it |
|
||||
|
||||
---
|
||||
|
||||
## Test Coverage Gaps
|
||||
|
||||
The existing test suite does NOT cover:
|
||||
- Cross-user resource access (user A accessing user B's sessions/folders/steps)
|
||||
- Non-admin attempting tree deletion
|
||||
- Viewer role restrictions
|
||||
- Arbitrary role string at registration
|
||||
- Team-based access controls (no team fixtures exist)
|
||||
- Step library permission checks (no step tests at all)
|
||||
- Category/tag/folder cross-user access
|
||||
- `start_session` tree access bypass
|
||||
- Concurrent session access across users
|
||||
- Deactivated user access (blocked by no `is_active` field)
|
||||
|
||||
---
|
||||
|
||||
## References
|
||||
|
||||
- Centralized permissions (dead code): `backend/app/core/permissions.py`
|
||||
- Frontend permissions hook: `frontend/src/hooks/usePermissions.ts`
|
||||
- Auth dependencies: `backend/app/api/deps.py`
|
||||
- RBAC questions doc: `docs/RBAC-IMPLEMENTATION-QUESTIONS.md`
|
||||
- Backend endpoints: `backend/app/api/endpoints/`
|
||||
- Models: `backend/app/models/`
|
||||
- Tests: `backend/tests/`
|
||||
501
docs/archive/2026-02-05-permissions-audit-implementation.md
Normal file
501
docs/archive/2026-02-05-permissions-audit-implementation.md
Normal file
@@ -0,0 +1,501 @@
|
||||
# Permissions & RBAC Fixes — Implementation Plan
|
||||
|
||||
> **Date:** 2026-02-05
|
||||
> **Status:** Updated after re-audit
|
||||
> **Depends on:** [2026-02-05-permissions-audit-design.md](2026-02-05-permissions-audit-design.md)
|
||||
> **Scope:** Phased fix of all permission issues identified in audit and re-audit
|
||||
|
||||
---
|
||||
|
||||
## Phasing Strategy
|
||||
|
||||
Fixes are grouped into four phases by severity and dependency:
|
||||
- **Phase A:** Critical security fixes (must-do before any multi-user use)
|
||||
- **Phase B:** High-severity gaps (needed for MSP readiness)
|
||||
- **Phase C:** Medium-severity improvements (better UX and consistency)
|
||||
- **Phase D:** Low-severity cleanup (nice-to-haves)
|
||||
|
||||
Each phase is independently deployable. Phase A should be done as a single PR.
|
||||
|
||||
### What Changed Since Initial Plan
|
||||
|
||||
The RBAC commit (`34daa26`) added infrastructure (`permissions.py`, `usePermissions.ts`, `is_super_admin`), but the re-audit revealed:
|
||||
- `permissions.py` is dead code — no endpoint imports it
|
||||
- `require_engineer_or_admin` has a bug blocking team admins with viewer role
|
||||
- 49 endpoints bypass `get_current_active_user`
|
||||
- The `role` field still accepts arbitrary strings
|
||||
- TreeEditorPage uses wrong permission check for edit mode
|
||||
|
||||
These new findings are integrated into the phases below.
|
||||
|
||||
---
|
||||
|
||||
## Phase A: Critical Security Fixes
|
||||
|
||||
**Branch:** `fix/critical-security`
|
||||
**Priority:** Immediate — blocks all other work
|
||||
|
||||
### A1. Lock Down Registration Role Field
|
||||
|
||||
**Files to modify:**
|
||||
- `backend/app/schemas/user.py` — Constrain `role` to `Literal["engineer", "viewer"]` or remove entirely
|
||||
- `backend/app/api/endpoints/auth.py` — Hardcode `role="engineer"` in registration
|
||||
|
||||
**Changes:**
|
||||
```python
|
||||
# schemas/user.py — Option A: Remove role from UserCreate entirely
|
||||
class UserCreate(UserBase):
|
||||
password: str = Field(..., min_length=10)
|
||||
# role field REMOVED — always defaults to "engineer"
|
||||
|
||||
# schemas/user.py — Option B: Constrain with Literal (if viewer self-registration is desired)
|
||||
from typing import Literal
|
||||
class UserCreate(UserBase):
|
||||
role: Literal["engineer", "viewer"] = "engineer"
|
||||
password: str = Field(..., min_length=10)
|
||||
|
||||
# auth.py — If using Option A, hardcode:
|
||||
new_user = User(
|
||||
...
|
||||
role="engineer", # Always engineer; changed only via admin endpoint
|
||||
...
|
||||
)
|
||||
```
|
||||
|
||||
**Tests to add:**
|
||||
- Register with `role: "admin"` → should still be engineer (or 422 with Literal)
|
||||
- Register with `role: "super_admin"` → rejected
|
||||
- Register with arbitrary string → rejected
|
||||
- Register with default → engineer
|
||||
|
||||
### A2. Escape HTML Export Output
|
||||
|
||||
**Files to modify:**
|
||||
- `backend/app/api/endpoints/sessions.py` — HTML export function
|
||||
|
||||
**Changes:**
|
||||
```python
|
||||
from html import escape
|
||||
|
||||
# Escape ALL user-provided content in _generate_html_export()
|
||||
html.append(f'<title>{escape(tree_name)}</title>')
|
||||
html.append(f'<h1>{escape(tree_name)}</h1>')
|
||||
html.append(f'<p><strong>Ticket:</strong> {escape(session.ticket_number or "")}</p>')
|
||||
html.append(f'<p><strong>Client:</strong> {escape(session.client_name or "")}</p>')
|
||||
html.append(f'<div class="scratchpad">{escape(scratchpad)}</div>')
|
||||
html.append(f'<h3>Step {i}: {escape(question)}</h3>')
|
||||
html.append(f'<p class="answer">Answer: {escape(answer)}</p>')
|
||||
html.append(f'<p class="notes">Notes: {escape(notes)}</p>')
|
||||
```
|
||||
|
||||
**Tests to add:**
|
||||
- Export session with `<script>alert('xss')</script>` in ticket_number → verify `<script>` in output
|
||||
- Export session with `<img onerror=...>` in notes → verify escaped
|
||||
- Export session with `&`, `"`, `'` in fields → verify entities
|
||||
|
||||
### A3. Fail on Default Secret Key in Production
|
||||
|
||||
**Files to modify:**
|
||||
- `backend/app/core/config.py`
|
||||
|
||||
**Changes:**
|
||||
```python
|
||||
@validator("SECRET_KEY")
|
||||
def validate_secret_key(cls, v):
|
||||
default = "your-secret-key-change-in-production-use-openssl-rand-hex-32"
|
||||
if not v or v == default:
|
||||
import os
|
||||
if os.getenv("DEBUG", "true").lower() != "true":
|
||||
raise ValueError("SECRET_KEY must be set to a secure value in production")
|
||||
return v
|
||||
```
|
||||
|
||||
### A4. Add Role Enum Constraint to Database
|
||||
|
||||
**Files to modify:**
|
||||
- `backend/app/models/user.py` — Add check constraint
|
||||
- New migration
|
||||
|
||||
**Changes:**
|
||||
```python
|
||||
from sqlalchemy import CheckConstraint
|
||||
|
||||
class User(Base):
|
||||
__table_args__ = (
|
||||
CheckConstraint("role IN ('engineer', 'viewer')", name="ck_user_role"),
|
||||
)
|
||||
```
|
||||
|
||||
Note: Role is now only `engineer` or `viewer`. The old `admin` role is fully replaced by `is_super_admin` flag (migration 010 already converted existing admin users).
|
||||
|
||||
**Migration:** `alembic revision --autogenerate -m "Add role check constraint"`
|
||||
|
||||
---
|
||||
|
||||
## Phase B: High-Severity Gaps
|
||||
|
||||
**Branch:** `fix/high-security`
|
||||
**Priority:** Before production MSP use
|
||||
|
||||
### B1. Add Tree Access Check to `start_session`
|
||||
|
||||
**Files to modify:**
|
||||
- `backend/app/api/endpoints/sessions.py`
|
||||
|
||||
**Changes:** After fetching the tree, add access check matching `get_tree`:
|
||||
```python
|
||||
can_access = (
|
||||
tree.is_default or tree.is_public or
|
||||
tree.author_id == current_user.id or
|
||||
(tree.team_id == current_user.team_id and current_user.team_id is not None) or
|
||||
current_user.is_super_admin
|
||||
)
|
||||
if not can_access:
|
||||
raise HTTPException(status_code=403, detail="You don't have access to this tree")
|
||||
```
|
||||
|
||||
**Tests to add:**
|
||||
- User starts session on own tree → success
|
||||
- User starts session on public tree → success
|
||||
- User starts session on private team tree they don't belong to → 403
|
||||
- Super admin starts session on any tree → success
|
||||
|
||||
### B2. Wire `permissions.py` Into Endpoints (or Delete It)
|
||||
|
||||
**Decision needed:** Wire the centralized module into all endpoints, or delete it and keep local helpers.
|
||||
|
||||
**Recommended: Wire it in.** Replace all 6 local permission helpers with imports from `permissions.py`.
|
||||
|
||||
**Files to modify:**
|
||||
- `backend/app/api/endpoints/trees.py` — Import and use `can_edit_tree`, `can_delete_tree`
|
||||
- `backend/app/api/endpoints/steps.py` — Import and use `can_edit_step`, `can_view_step`
|
||||
- `backend/app/api/endpoints/tags.py` — Import and use `can_manage_tree_tags`, `can_create_tag`
|
||||
- `backend/app/api/endpoints/categories.py` — Import and use `can_manage_category`, `can_create_category`
|
||||
- `backend/app/api/endpoints/step_categories.py` — Import and use `can_manage_step_category`, `can_create_step_category`
|
||||
- `backend/app/api/endpoints/folders.py` — Import and use `can_access_tree`
|
||||
- `backend/app/core/permissions.py` — Add any missing functions (`can_view_step`, `can_create_tag`, `can_access_tree`)
|
||||
|
||||
**This also fixes:**
|
||||
- N1 (dead code)
|
||||
- Finding #10 partial (`None` guards from centralized module applied everywhere)
|
||||
|
||||
### B3. Fix `require_engineer_or_admin` Team Admin Bug
|
||||
|
||||
**Files to modify:**
|
||||
- `backend/app/api/deps.py`
|
||||
|
||||
**Changes:**
|
||||
```python
|
||||
async def require_engineer_or_admin(
|
||||
current_user: Annotated[User, Depends(get_current_active_user)]
|
||||
) -> User:
|
||||
if current_user.is_super_admin:
|
||||
return current_user
|
||||
if current_user.is_team_admin and current_user.team_id is not None:
|
||||
return current_user
|
||||
if current_user.role not in ("engineer",):
|
||||
raise HTTPException(status_code=403, detail="Requires engineer or higher role")
|
||||
return current_user
|
||||
```
|
||||
|
||||
### B4. Add `is_active` Field and Update All Dependencies
|
||||
|
||||
**Files to modify:**
|
||||
- `backend/app/models/user.py` — Add `is_active = Column(Boolean, default=True, server_default="true")`
|
||||
- `backend/app/api/deps.py` — Implement `get_current_active_user` check
|
||||
- **All endpoint files** — Change `Depends(get_current_user)` to `Depends(get_current_active_user)` (49 usages)
|
||||
- New migration
|
||||
|
||||
**Changes:**
|
||||
```python
|
||||
# deps.py
|
||||
async def get_current_active_user(
|
||||
current_user: Annotated[User, Depends(get_current_user)]
|
||||
) -> User:
|
||||
if not current_user.is_active:
|
||||
raise HTTPException(status_code=403, detail="Account has been deactivated")
|
||||
return current_user
|
||||
```
|
||||
|
||||
### B5. Create Admin User Management Endpoints
|
||||
|
||||
**New file:** `backend/app/api/endpoints/admin.py`
|
||||
|
||||
**Endpoints:**
|
||||
| Method | Path | Description |
|
||||
|--------|------|-------------|
|
||||
| GET | `/api/v1/admin/users` | List all users (super admin only) |
|
||||
| GET | `/api/v1/admin/users/{id}` | Get user details (super admin only) |
|
||||
| PUT | `/api/v1/admin/users/{id}/role` | Change user role (super admin only) |
|
||||
| PUT | `/api/v1/admin/users/{id}/team-admin` | Toggle is_team_admin (super admin only) |
|
||||
| PUT | `/api/v1/admin/users/{id}/deactivate` | Deactivate account (super admin only) |
|
||||
| PUT | `/api/v1/admin/users/{id}/activate` | Reactivate account (super admin only) |
|
||||
|
||||
**Files to modify:**
|
||||
- `backend/app/api/router.py` — Register admin router
|
||||
- `backend/app/schemas/user.py` — Add admin response/update schemas
|
||||
|
||||
### B6. Add Rate Limiting
|
||||
|
||||
**New dependency:** `slowapi`
|
||||
|
||||
**Files to modify:**
|
||||
- `backend/requirements.txt` — Add `slowapi`
|
||||
- `backend/app/main.py` — Add rate limiter middleware
|
||||
- `backend/app/api/endpoints/auth.py` — Apply rate limits
|
||||
|
||||
**Rate limits:**
|
||||
| Endpoint | Limit |
|
||||
|----------|-------|
|
||||
| `POST /auth/login` | 5/minute per IP |
|
||||
| `POST /auth/register` | 3/minute per IP |
|
||||
| `POST /auth/refresh` | 10/minute per IP |
|
||||
| `GET /invites/validate/{code}` | 5/minute per IP |
|
||||
|
||||
### B7. Token Revocation (Short-Term Fix)
|
||||
|
||||
**Approach:** Store refresh tokens in DB so they can be revoked. Reduce access token TTL.
|
||||
|
||||
**Files to modify:**
|
||||
- `backend/app/models/` — New `RefreshToken` model (token hash, user_id, expires_at, revoked_at)
|
||||
- `backend/app/api/endpoints/auth.py` — Store on login, validate on refresh, revoke on logout
|
||||
- `backend/app/core/config.py` — Reduce `ACCESS_TOKEN_EXPIRE_MINUTES` to 5
|
||||
- New migration
|
||||
|
||||
**Logout becomes meaningful:**
|
||||
```python
|
||||
@router.post("/logout")
|
||||
async def logout(payload = Depends(get_refresh_token_payload), db = Depends(get_db)):
|
||||
await db.execute(
|
||||
update(RefreshToken)
|
||||
.where(RefreshToken.token_hash == hash_token(payload["jti"]))
|
||||
.values(revoked_at=datetime.now(timezone.utc))
|
||||
)
|
||||
return {"message": "Successfully logged out"}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Phase C: Medium-Severity Improvements
|
||||
|
||||
**Branch:** `feat/permissions-ux`
|
||||
**Priority:** Better UX and consistency
|
||||
|
||||
### C1. Frontend: Add 403 Handling and Route Guards
|
||||
|
||||
**Files to modify:**
|
||||
- `frontend/src/api/client.ts` — Add 403 interceptor
|
||||
- `frontend/src/components/layout/ProtectedRoute.tsx` — Add `requiredRole` prop
|
||||
- `frontend/src/router.tsx` — Apply role guards to admin routes
|
||||
|
||||
**403 interceptor:**
|
||||
```typescript
|
||||
if (error.response?.status === 403) {
|
||||
toast.error("You don't have permission to perform this action")
|
||||
return Promise.reject(error)
|
||||
}
|
||||
```
|
||||
|
||||
**ProtectedRoute enhancement:**
|
||||
```typescript
|
||||
interface ProtectedRouteProps {
|
||||
requiredRole?: UserRole | UserRole[]
|
||||
requireTeamAdmin?: boolean
|
||||
children: React.ReactNode
|
||||
}
|
||||
```
|
||||
|
||||
### C2. Frontend: Fix TreeEditorPage Permission Check
|
||||
|
||||
**Files to modify:**
|
||||
- `frontend/src/pages/TreeEditorPage.tsx`
|
||||
|
||||
**Change:** After loading the tree in edit mode, check `canEditTree(tree)` instead of just `canCreateTrees`:
|
||||
```typescript
|
||||
useEffect(() => {
|
||||
if (tree && id && !canEditTree({ author_id: tree.author_id, team_id: tree.team_id })) {
|
||||
navigate('/trees')
|
||||
}
|
||||
}, [tree, id])
|
||||
```
|
||||
|
||||
### C3. Frontend: Add Delete UI for Trees and Steps
|
||||
|
||||
**Files to modify:**
|
||||
- `frontend/src/pages/TreeLibraryPage.tsx` — Add delete button gated by `canDeleteTree`
|
||||
- `frontend/src/components/step-library/StepCard.tsx` — Add delete button gated by `canEditStep`
|
||||
- `frontend/src/components/common/ConfirmDialog.tsx` — New reusable confirmation modal
|
||||
|
||||
### C4. Frontend: Wire `usePermissions` Into All Relevant Components
|
||||
|
||||
**Files to modify:**
|
||||
- `frontend/src/components/step-library/StepLibraryBrowser.tsx` — Use `canCreateSteps` for create button
|
||||
- `frontend/src/components/step-library/CustomStepModal.tsx` — Check `canCreateSteps`
|
||||
- `frontend/src/components/tree-editor/NodeList.tsx` — Add confirmation dialog for node deletion
|
||||
|
||||
### C5. Backend: Fix `None == None` in All Local Helpers
|
||||
|
||||
**Files to modify (if not already fixed by B2):**
|
||||
- `backend/app/api/endpoints/steps.py` — `can_view_step()` null guard
|
||||
- `backend/app/api/endpoints/categories.py` — `can_manage_category()` null guard
|
||||
- `backend/app/api/endpoints/step_categories.py` — `can_manage_step_category()` null guard
|
||||
|
||||
### C6. Backend: Fix Admin Tree List Inconsistency
|
||||
|
||||
**Files to modify:**
|
||||
- `backend/app/api/endpoints/trees.py`
|
||||
|
||||
**Change:** Add super admin bypass to `build_tree_access_filter`:
|
||||
```python
|
||||
def build_tree_access_filter(current_user: User):
|
||||
conditions = [
|
||||
Tree.is_default == True,
|
||||
Tree.is_public == True,
|
||||
Tree.author_id == current_user.id,
|
||||
]
|
||||
if current_user.team_id:
|
||||
conditions.append(Tree.team_id == current_user.team_id)
|
||||
if current_user.is_super_admin:
|
||||
conditions.append(sqlalchemy.true())
|
||||
return or_(*conditions)
|
||||
```
|
||||
|
||||
### C7. Backend: Add Audit Log Table
|
||||
|
||||
**New file:** `backend/app/models/audit_log.py`
|
||||
|
||||
```python
|
||||
class AuditLog(Base):
|
||||
__tablename__ = "audit_logs"
|
||||
id = Column(UUID, primary_key=True, default=uuid4)
|
||||
user_id = Column(UUID, ForeignKey("users.id"))
|
||||
action = Column(String(50)) # "tree.delete", "user.role_change", etc.
|
||||
resource_type = Column(String(50)) # "tree", "step", "user", etc.
|
||||
resource_id = Column(UUID)
|
||||
details = Column(JSONB) # Before/after values
|
||||
created_at = Column(DateTime(timezone=True), default=lambda: datetime.now(timezone.utc))
|
||||
```
|
||||
|
||||
Integrate into admin endpoints and destructive actions.
|
||||
|
||||
---
|
||||
|
||||
## Phase D: Low-Severity Cleanup
|
||||
|
||||
**Branch:** `chore/permissions-cleanup`
|
||||
**Priority:** When convenient
|
||||
|
||||
### D1. Define and Enforce Viewer Role Permissions
|
||||
|
||||
Decide what viewers can/cannot do and enforce consistently:
|
||||
- **Proposed:** Viewers can browse trees, start sessions, view steps, and rate steps. Cannot create trees, steps, folders, or manage tags/categories.
|
||||
- Add `require_engineer_or_admin` to folder creation and tag creation endpoints.
|
||||
|
||||
### D2. Add Password Complexity Validation
|
||||
|
||||
**Files to modify:** `backend/app/schemas/user.py`
|
||||
|
||||
Add regex validator: at least one uppercase, one lowercase, one digit, 10+ characters.
|
||||
|
||||
### D3. Soft Delete Cascade Cleanup
|
||||
|
||||
When soft-deleting a tree, also:
|
||||
- Remove from all folder assignments
|
||||
- Remove tag assignments
|
||||
- Keep sessions intact (they have the snapshot)
|
||||
|
||||
### D4. Remove Debug Endpoint in Production
|
||||
|
||||
**Files to modify:** `backend/app/main.py`
|
||||
|
||||
Conditionally register `/debug/cors` only when `DEBUG=true`.
|
||||
|
||||
### D5. Escape Wildcards in Tag Search
|
||||
|
||||
**Files to modify:** `backend/app/api/endpoints/tags.py`
|
||||
|
||||
```python
|
||||
q_escaped = q.replace("%", "\\%").replace("_", "\\_")
|
||||
query = select(TreeTag).where(TreeTag.name.ilike(f"%{q_escaped}%"))
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Execution Order Summary
|
||||
|
||||
```
|
||||
Phase A (Critical) ──── Single PR, immediate
|
||||
A1. Lock down registration role field
|
||||
A2. Escape HTML export
|
||||
A3. Fail on default secret key
|
||||
A4. Add role DB constraint
|
||||
|
||||
Phase B (High) ──── 3-4 PRs
|
||||
B1. Tree access check on start_session
|
||||
B2. Wire permissions.py into endpoints (fixes dead code + None guards)
|
||||
B3. Fix require_engineer_or_admin team admin bug
|
||||
B4. is_active field + update all 49 endpoint dependencies
|
||||
B5. Admin user management endpoints
|
||||
B6. Rate limiting
|
||||
B7. Token revocation
|
||||
|
||||
Phase C (Medium) ──── 3-4 PRs
|
||||
C1. Frontend 403 handling + route guards
|
||||
C2. Fix TreeEditorPage canEditTree check
|
||||
C3. Delete UI for trees and steps
|
||||
C4. Wire usePermissions into all components
|
||||
C5. Fix None==None in all local helpers (if not done in B2)
|
||||
C6. Fix admin tree list inconsistency
|
||||
C7. Audit log table
|
||||
|
||||
Phase D (Low) ──── As convenient
|
||||
D1. Viewer role enforcement
|
||||
D2. Password complexity
|
||||
D3. Soft delete cascade cleanup
|
||||
D4. Remove debug endpoint in prod
|
||||
D5. Escape wildcards in tag search
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Estimated Scope
|
||||
|
||||
| Phase | Backend Changes | Frontend Changes | Migrations | New Tests |
|
||||
|-------|----------------|-----------------|------------|-----------|
|
||||
| A | 4 files modified | 0 | 1 | ~8 tests |
|
||||
| B | 10+ files modified, 2 new | 0 | 2 | ~20 tests |
|
||||
| C | 3 files modified, 2 new | 6 files modified, 1 new | 1 | ~10 tests |
|
||||
| D | 4 files modified | 0 | 0 | ~5 tests |
|
||||
|
||||
---
|
||||
|
||||
## Testing Strategy
|
||||
|
||||
Each phase should include:
|
||||
1. **Unit tests** for new/changed permission checks
|
||||
2. **Integration tests** for cross-user access scenarios
|
||||
3. **Negative tests** — verify unauthorized access returns 403, not 500
|
||||
4. Manual verification of frontend behavior per role
|
||||
|
||||
**Priority test scenarios:**
|
||||
- Register with `role: "admin"` → rejected or ignored
|
||||
- Register with arbitrary role string → rejected
|
||||
- User A accesses User B's session → 403
|
||||
- Viewer creates a tree → 403
|
||||
- Team admin with viewer role creates a tree → success (B3 fix)
|
||||
- Start session on private tree without access → 403
|
||||
- Login with deactivated account → 403
|
||||
- Exported HTML with `<script>` tag → escaped
|
||||
- Brute force login → rate limited after 5 attempts
|
||||
- `permissions.py` functions match endpoint behavior (after B2)
|
||||
|
||||
---
|
||||
|
||||
## Notes
|
||||
|
||||
- Phase A changes are backward-compatible — no frontend changes needed
|
||||
- Phase B is larger than originally planned due to dead code wiring (B2) and dependency chain fix (B4)
|
||||
- B2 (wiring permissions.py) and B4 (is_active dependency chain) touch many files — plan for careful review
|
||||
- Token revocation (B7) is the most complex change — consider deferring to a dedicated PR
|
||||
- Phase C partially started: `usePermissions` hook exists, TreeLibraryPage and TreeEditorPage use it
|
||||
- All `require_admin` checks should use `is_super_admin` (never `role == "admin"`)
|
||||
1315
docs/archive/2026-02-06-subscription-tier-implementation.md
Normal file
1315
docs/archive/2026-02-06-subscription-tier-implementation.md
Normal file
File diff suppressed because it is too large
Load Diff
834
docs/archive/2026-02-07-foundational-schema-design.md
Normal file
834
docs/archive/2026-02-07-foundational-schema-design.md
Normal file
@@ -0,0 +1,834 @@
|
||||
# Foundational Domain Model: Step Library, User Trees, and Session Sharing
|
||||
|
||||
## Context
|
||||
|
||||
This design establishes the foundational schema for three critical ResolutionFlow features that define the durable domain model. All future functionality depends on these tables, fields, and constraints being correctly designed from the start.
|
||||
|
||||
### Features Covered
|
||||
|
||||
1. **Step Library Core Schema** (Issues #4-#7): Reusable troubleshooting steps with categories, ratings, and usage tracking
|
||||
2. **User Trees & Forking** (Issue #11): Personal/forked tree data model for engineer customization
|
||||
3. **Session Sharing** (Issue #15): Read-only share links with configurable access control
|
||||
|
||||
### Why These First
|
||||
|
||||
These features establish:
|
||||
- **Step Library**: Canonical data model for reusable steps before any UI or session integrations
|
||||
- **User Trees**: Fork relationships and ownership model before implementing fork/share workflows
|
||||
- **Sharing Schema**: Token mechanics and access control before APIs/UI solidify
|
||||
|
||||
Getting these schemas right now prevents costly migrations and refactoring later.
|
||||
|
||||
---
|
||||
|
||||
## Part 1: Tree Forking Model
|
||||
|
||||
### Design Principle
|
||||
|
||||
**Forked trees are regular trees with added fork metadata.** They use the same ownership/visibility model as existing trees:
|
||||
- `author_id`: Engineer who created the fork
|
||||
- `account_id`: Account that owns the fork
|
||||
- `is_public` / visibility: Controls who can see/use it
|
||||
|
||||
### Schema Changes: New Fields on `trees` Table
|
||||
|
||||
Add three fields to track fork relationships:
|
||||
|
||||
```python
|
||||
# Fork relationship tracking
|
||||
parent_tree_id: Mapped[Optional[uuid.UUID]] = mapped_column(
|
||||
UUID(as_uuid=True),
|
||||
ForeignKey("trees.id", ondelete="SET NULL"), # Orphan forks on parent delete
|
||||
nullable=True,
|
||||
index=True
|
||||
)
|
||||
|
||||
fork_reason: Mapped[Optional[str]] = mapped_column(
|
||||
String(255),
|
||||
nullable=True,
|
||||
comment="Brief reason: 'Added Cisco Meraki steps for our network'"
|
||||
)
|
||||
|
||||
parent_updated_at: Mapped[Optional[datetime]] = mapped_column(
|
||||
DateTime(timezone=True),
|
||||
nullable=True,
|
||||
comment="Snapshot of parent's updated_at when fork created. Compare to detect parent updates."
|
||||
)
|
||||
```
|
||||
|
||||
### New Relationships on `Tree` Model
|
||||
|
||||
```python
|
||||
# In Tree class:
|
||||
parent: Mapped[Optional["Tree"]] = relationship(
|
||||
"Tree",
|
||||
remote_side=[id],
|
||||
foreign_keys=[parent_tree_id],
|
||||
back_populates="forks"
|
||||
)
|
||||
|
||||
forks: Mapped[list["Tree"]] = relationship(
|
||||
"Tree",
|
||||
foreign_keys=[parent_tree_id],
|
||||
back_populates="parent"
|
||||
# No cascade - ondelete="SET NULL" handles orphaning at DB level
|
||||
)
|
||||
```
|
||||
|
||||
### Field Explanations
|
||||
|
||||
| Field | Type | Purpose |
|
||||
|-------|------|---------|
|
||||
| `parent_tree_id` | UUID nullable | NULL = root tree, not-NULL = forked tree. Points to original tree. |
|
||||
| `fork_reason` | String(255) | Optional engineer note: "Added wireless troubleshooting for our Meraki APs" |
|
||||
| `parent_updated_at` | Datetime nullable | Timestamp snapshot. Compare to parent's actual `updated_at` to detect changes. |
|
||||
|
||||
### Key Behaviors
|
||||
|
||||
**Fork Creation:**
|
||||
1. Copy parent's `tree_structure` JSONB
|
||||
2. Set `parent_tree_id` to parent's ID
|
||||
3. Set `fork_reason` from user input
|
||||
4. Snapshot `parent_updated_at = parent.updated_at`
|
||||
5. Set `author_id = current_user.id`
|
||||
6. Set `account_id = current_user.account_id`
|
||||
7. Default `is_public = False` (private fork)
|
||||
8. Reset `version = 1` for fork
|
||||
|
||||
**Fork Updates:**
|
||||
- Engineer edits fork → `version` increments
|
||||
- Parent tree unaffected
|
||||
- `parent_updated_at` remains frozen at fork time
|
||||
|
||||
**Parent Deletion:**
|
||||
- Hard delete → `parent_tree_id` becomes NULL (orphaned)
|
||||
- Fork survives as independent tree
|
||||
- Soft delete → `parent_tree_id` preserved, fork still references parent
|
||||
|
||||
**Update Notifications:**
|
||||
```python
|
||||
def has_parent_updates(fork: Tree) -> bool:
|
||||
"""Check if parent tree has updates since fork."""
|
||||
if not fork.parent_tree_id or not fork.parent_updated_at:
|
||||
return False
|
||||
|
||||
parent = db.query(Tree).get(fork.parent_tree_id)
|
||||
if not parent:
|
||||
return False # Parent deleted or soft-deleted
|
||||
|
||||
return parent.updated_at > fork.parent_updated_at
|
||||
```
|
||||
|
||||
### Migration 1: Tree Forking Fields
|
||||
|
||||
**File**: `alembic/versions/022_add_tree_forking.py`
|
||||
|
||||
```python
|
||||
def upgrade():
|
||||
# Add fork tracking columns
|
||||
op.add_column('trees', sa.Column('parent_tree_id', UUID, nullable=True))
|
||||
op.add_column('trees', sa.Column('fork_reason', sa.String(255), nullable=True))
|
||||
op.add_column('trees', sa.Column('parent_updated_at', sa.DateTime(timezone=True), nullable=True))
|
||||
|
||||
# Add foreign key
|
||||
op.create_foreign_key(
|
||||
'fk_trees_parent_tree_id',
|
||||
'trees', 'trees',
|
||||
['parent_tree_id'], ['id'],
|
||||
ondelete='SET NULL'
|
||||
)
|
||||
|
||||
# Add index for fork queries
|
||||
op.create_index('ix_trees_parent_tree_id', 'trees', ['parent_tree_id'])
|
||||
|
||||
def downgrade():
|
||||
op.drop_index('ix_trees_parent_tree_id')
|
||||
op.drop_constraint('fk_trees_parent_tree_id', 'trees')
|
||||
op.drop_column('trees', 'parent_updated_at')
|
||||
op.drop_column('trees', 'fork_reason')
|
||||
op.drop_column('trees', 'parent_tree_id')
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Part 2: Session Custom Steps Enhancement
|
||||
|
||||
### Current State
|
||||
|
||||
`sessions.custom_steps` already exists as JSONB field. Current structure:
|
||||
|
||||
```json
|
||||
{
|
||||
"custom_steps": [
|
||||
{
|
||||
"type": "action",
|
||||
"content": "Check Meraki dashboard for AP status",
|
||||
"notes": "Found AP offline"
|
||||
}
|
||||
]
|
||||
}
|
||||
```
|
||||
|
||||
### Enhanced Structure (Backward Compatible)
|
||||
|
||||
**No migration needed** - pure JSONB enhancement:
|
||||
|
||||
```json
|
||||
{
|
||||
"custom_steps": [
|
||||
{
|
||||
"type": "action",
|
||||
"content": "Check Meraki dashboard for AP status",
|
||||
"notes": "Found AP offline",
|
||||
|
||||
// NEW FIELDS (optional, added to new sessions only):
|
||||
"source": "ad-hoc", // "ad-hoc" | "step-library" | "forked-tree"
|
||||
"source_step_id": null, // UUID string if from StepLibrary
|
||||
"inserted_at": "2026-02-07T15:30:00Z", // ISO datetime
|
||||
"inserted_after_node_id": "ad_verify_identity" // Node ID from tree_structure
|
||||
},
|
||||
{
|
||||
"type": "action",
|
||||
"content": "Verify DNS configuration",
|
||||
"notes": "DNS servers correct",
|
||||
"source": "step-library",
|
||||
"source_step_id": "123e4567-e89b-12d3-a456-426614174000",
|
||||
"inserted_at": "2026-02-07T15:32:00Z",
|
||||
"inserted_after_node_id": "network_check"
|
||||
}
|
||||
]
|
||||
}
|
||||
```
|
||||
|
||||
### New Field Definitions
|
||||
|
||||
| Field | Type | Purpose |
|
||||
|-------|------|---------|
|
||||
| `source` | string | Origin: `ad-hoc` (typed by engineer), `step-library` (from library), `forked-tree` (from fork) |
|
||||
| `source_step_id` | UUID string | If `source=step-library`, points to StepLibrary.id for usage tracking |
|
||||
| `inserted_at` | ISO datetime | When engineer added this step to session |
|
||||
| `inserted_after_node_id` | string | Node ID from tree_structure where step was inserted (e.g., `"ad_verify_identity"`) |
|
||||
|
||||
### Use Cases
|
||||
|
||||
**1. Step Library Usage Tracking**
|
||||
- Engineer inserts step from library → logs to `step_usage_log`
|
||||
- Increments `StepLibrary.usage_count`
|
||||
- Even if step later deleted, `source_step_id` preserved (orphaned reference for analytics)
|
||||
|
||||
**2. Identify Popular Ad-Hoc Steps**
|
||||
- Analytics query: "What ad-hoc steps appear across multiple sessions?"
|
||||
- Example: "Check Meraki dashboard" appears 15 times → suggest adding to library
|
||||
- Helps identify gaps in official step library
|
||||
|
||||
**3. Save Session as Tree (Future)**
|
||||
- Engineer completes session with custom steps
|
||||
- "Save as Tree" reconstructs tree structure
|
||||
- Uses `inserted_after_node_id` to place custom steps correctly
|
||||
|
||||
### Backward Compatibility Strategy
|
||||
|
||||
**Pydantic schema** (backend):
|
||||
```python
|
||||
class CustomStepSchema(BaseModel):
|
||||
type: str # "decision" | "action" | "solution"
|
||||
content: str
|
||||
notes: Optional[str] = None
|
||||
|
||||
# New fields with defaults for old sessions:
|
||||
source: str = "ad-hoc"
|
||||
source_step_id: Optional[UUID] = None
|
||||
inserted_at: Optional[datetime] = None
|
||||
inserted_after_node_id: Optional[str] = None
|
||||
```
|
||||
|
||||
**Result**: Old sessions load without error. New fields auto-populate with defaults in-memory. Database unchanged.
|
||||
|
||||
### Step Library Reference Handling
|
||||
|
||||
**When StepLibrary entry deleted:**
|
||||
- `source_step_id` remains in sessions (orphaned UUID)
|
||||
- No foreign key constraint (it's in JSONB)
|
||||
- Analytics can still count: "Deleted step XYZ was used 50 times historically"
|
||||
- Frontend shows: "From step library (no longer available)"
|
||||
|
||||
### Usage Tracking Implementation
|
||||
|
||||
**When engineer inserts library step into session:**
|
||||
|
||||
```python
|
||||
async def insert_step_from_library(
|
||||
session_id: UUID,
|
||||
step_id: UUID,
|
||||
current_node_id: str,
|
||||
current_user: User,
|
||||
db: AsyncSession
|
||||
):
|
||||
# 1. Load step from library
|
||||
step = await db.get(StepLibrary, step_id)
|
||||
|
||||
# 2. Add to session.custom_steps
|
||||
new_step = {
|
||||
"type": step.step_type,
|
||||
"content": step.content,
|
||||
"notes": "",
|
||||
"source": "step-library",
|
||||
"source_step_id": str(step.id),
|
||||
"inserted_at": datetime.now(timezone.utc).isoformat(),
|
||||
"inserted_after_node_id": current_node_id
|
||||
}
|
||||
|
||||
session.custom_steps.append(new_step)
|
||||
|
||||
# 3. Log usage immediately
|
||||
usage_log = StepUsageLog(
|
||||
step_id=step.id,
|
||||
user_id=current_user.id,
|
||||
session_id=session.id,
|
||||
used_at=datetime.now(timezone.utc)
|
||||
)
|
||||
db.add(usage_log)
|
||||
|
||||
# 4. Increment counter
|
||||
step.usage_count += 1
|
||||
|
||||
await db.commit()
|
||||
```
|
||||
|
||||
**Multiple insertions**: Same step inserted twice in one session → 2 `StepUsageLog` entries, 2 items in `custom_steps` array, `usage_count += 2`.
|
||||
|
||||
### No Migration Required
|
||||
|
||||
✅ This is pure application-layer enhancement. Existing sessions remain valid. New sessions use enhanced structure.
|
||||
|
||||
---
|
||||
|
||||
## Part 3: Session Share Tokens
|
||||
|
||||
### Overview
|
||||
|
||||
Enable engineers to share read-only session links with configurable access control:
|
||||
- **Public shares**: Anyone with link can view (no auth required)
|
||||
- **Account-only shares**: Requires login + account membership
|
||||
|
||||
Account owners control whether engineers can create public shares via policy setting.
|
||||
|
||||
### New Table 1: `session_shares`
|
||||
|
||||
```python
|
||||
class SessionShare(Base):
|
||||
__tablename__ = "session_shares"
|
||||
__table_args__ = (
|
||||
CheckConstraint(
|
||||
"visibility IN ('public', 'account')",
|
||||
name='ck_session_shares_visibility'
|
||||
),
|
||||
)
|
||||
|
||||
id: Mapped[uuid.UUID] = mapped_column(
|
||||
UUID(as_uuid=True),
|
||||
primary_key=True,
|
||||
default=uuid.uuid4
|
||||
)
|
||||
|
||||
session_id: Mapped[uuid.UUID] = mapped_column(
|
||||
UUID(as_uuid=True),
|
||||
ForeignKey("sessions.id", ondelete="CASCADE"),
|
||||
nullable=False,
|
||||
index=True
|
||||
)
|
||||
|
||||
share_token: Mapped[str] = mapped_column(
|
||||
String(64),
|
||||
unique=True,
|
||||
nullable=False,
|
||||
index=True,
|
||||
comment="URL-safe random token (48 bytes → 64 base64 chars)"
|
||||
)
|
||||
|
||||
share_name: Mapped[Optional[str]] = mapped_column(
|
||||
String(100),
|
||||
nullable=True,
|
||||
comment="Optional label: 'Training link', 'Customer escalation #1234'"
|
||||
)
|
||||
|
||||
visibility: Mapped[str] = mapped_column(
|
||||
String(20),
|
||||
nullable=False,
|
||||
default="public",
|
||||
comment="public = anyone with link, account = account members only"
|
||||
)
|
||||
|
||||
created_by: Mapped[uuid.UUID] = mapped_column(
|
||||
UUID(as_uuid=True),
|
||||
ForeignKey("users.id", ondelete="CASCADE"),
|
||||
nullable=False,
|
||||
index=True # For "My Shares" view performance
|
||||
)
|
||||
|
||||
created_at: Mapped[datetime] = mapped_column(
|
||||
DateTime(timezone=True),
|
||||
default=lambda: datetime.now(timezone.utc)
|
||||
)
|
||||
|
||||
updated_at: Mapped[datetime] = mapped_column(
|
||||
DateTime(timezone=True),
|
||||
default=lambda: datetime.now(timezone.utc),
|
||||
onupdate=lambda: datetime.now(timezone.utc)
|
||||
)
|
||||
|
||||
expires_at: Mapped[Optional[datetime]] = mapped_column(
|
||||
DateTime(timezone=True),
|
||||
nullable=True,
|
||||
index=True,
|
||||
comment="Optional expiration for time-limited shares"
|
||||
)
|
||||
|
||||
# Deprecated: Simple counting replaced by session_share_views table
|
||||
view_count: Mapped[int] = mapped_column(
|
||||
Integer,
|
||||
nullable=False,
|
||||
default=0
|
||||
)
|
||||
|
||||
last_viewed_at: Mapped[Optional[datetime]] = mapped_column(
|
||||
DateTime(timezone=True),
|
||||
nullable=True
|
||||
)
|
||||
|
||||
is_active: Mapped[bool] = mapped_column(
|
||||
Boolean,
|
||||
nullable=False,
|
||||
default=True,
|
||||
index=True
|
||||
)
|
||||
|
||||
# Relationships
|
||||
session: Mapped["Session"] = relationship("Session", back_populates="shares")
|
||||
creator: Mapped["User"] = relationship("User", foreign_keys=[created_by])
|
||||
views: Mapped[list["SessionShareView"]] = relationship(
|
||||
"SessionShareView",
|
||||
back_populates="share",
|
||||
cascade="all, delete-orphan"
|
||||
)
|
||||
```
|
||||
|
||||
### New Table 2: `session_share_views`
|
||||
|
||||
Track detailed view analytics, including WHO viewed account-only shares:
|
||||
|
||||
```python
|
||||
class SessionShareView(Base):
|
||||
__tablename__ = "session_share_views"
|
||||
|
||||
id: Mapped[uuid.UUID] = mapped_column(
|
||||
UUID(as_uuid=True),
|
||||
primary_key=True,
|
||||
default=uuid.uuid4
|
||||
)
|
||||
|
||||
share_id: Mapped[uuid.UUID] = mapped_column(
|
||||
UUID(as_uuid=True),
|
||||
ForeignKey("session_shares.id", ondelete="CASCADE"),
|
||||
nullable=False,
|
||||
index=True
|
||||
)
|
||||
|
||||
viewer_id: Mapped[Optional[uuid.UUID]] = mapped_column(
|
||||
UUID(as_uuid=True),
|
||||
ForeignKey("users.id", ondelete="SET NULL"),
|
||||
nullable=True,
|
||||
index=True,
|
||||
comment="NULL for public shares (unauthenticated views)"
|
||||
)
|
||||
|
||||
viewed_at: Mapped[datetime] = mapped_column(
|
||||
DateTime(timezone=True),
|
||||
default=lambda: datetime.now(timezone.utc),
|
||||
index=True
|
||||
)
|
||||
|
||||
viewer_ip: Mapped[Optional[str]] = mapped_column(
|
||||
String(45), # IPv6 max length
|
||||
nullable=True
|
||||
)
|
||||
|
||||
viewer_user_agent: Mapped[Optional[str]] = mapped_column(
|
||||
String(500),
|
||||
nullable=True
|
||||
)
|
||||
|
||||
# Relationships
|
||||
share: Mapped["SessionShare"] = relationship("SessionShare", back_populates="views")
|
||||
viewer: Mapped[Optional["User"]] = relationship("User")
|
||||
```
|
||||
|
||||
### Account-Level Sharing Policy
|
||||
|
||||
Add to `accounts` table:
|
||||
|
||||
```python
|
||||
# In Account model:
|
||||
allow_public_shares: Mapped[bool] = mapped_column(
|
||||
Boolean,
|
||||
nullable=False,
|
||||
default=True,
|
||||
comment="Policy: engineers can create public shares. Only affects NEW shares (grandfathered)."
|
||||
)
|
||||
```
|
||||
|
||||
**Policy enforcement** (at share creation time only):
|
||||
```python
|
||||
# In create_share endpoint:
|
||||
if visibility == "public" and not current_user.account.allow_public_shares:
|
||||
raise HTTPException(
|
||||
status_code=403,
|
||||
detail="Your organization does not allow public session sharing. Use account-only visibility."
|
||||
)
|
||||
```
|
||||
|
||||
**Existing shares**: If account owner toggles policy OFF, existing public shares remain active (grandfathered). Owner can manually revoke individual shares if needed.
|
||||
|
||||
### Share Token Generation
|
||||
|
||||
**Requirements:**
|
||||
- URL-safe (no special chars)
|
||||
- Cryptographically random (non-guessable)
|
||||
- Collision-resistant
|
||||
|
||||
**Implementation:**
|
||||
```python
|
||||
import secrets
|
||||
from sqlalchemy.exc import IntegrityError
|
||||
|
||||
async def create_session_share(
|
||||
db: AsyncSession,
|
||||
session_id: UUID,
|
||||
created_by: UUID,
|
||||
visibility: str,
|
||||
share_name: Optional[str] = None,
|
||||
expires_at: Optional[datetime] = None,
|
||||
max_retries: int = 3
|
||||
) -> SessionShare:
|
||||
"""Create share with automatic token collision retry."""
|
||||
|
||||
for attempt in range(max_retries):
|
||||
try:
|
||||
share_token = secrets.token_urlsafe(48) # 48 bytes → 64 chars
|
||||
|
||||
share = SessionShare(
|
||||
session_id=session_id,
|
||||
share_token=share_token,
|
||||
share_name=share_name,
|
||||
visibility=visibility,
|
||||
created_by=created_by,
|
||||
expires_at=expires_at
|
||||
)
|
||||
|
||||
db.add(share)
|
||||
await db.commit()
|
||||
await db.refresh(share)
|
||||
return share
|
||||
|
||||
except IntegrityError as e:
|
||||
if "session_shares_share_token_key" in str(e):
|
||||
# Token collision (extremely rare), retry
|
||||
await db.rollback()
|
||||
if attempt == max_retries - 1:
|
||||
raise
|
||||
continue
|
||||
else:
|
||||
raise
|
||||
```
|
||||
|
||||
**Share URL format:**
|
||||
```
|
||||
https://patherly.com/share/x3K9mN_2pQ7vR8sT4wZ1aB5cD6eF7gH8iJ9kL0mN
|
||||
```
|
||||
|
||||
### Access Control Flow
|
||||
|
||||
**When user visits share link:**
|
||||
|
||||
```python
|
||||
async def access_share(
|
||||
token: str,
|
||||
current_user: Optional[User],
|
||||
request: Request,
|
||||
db: AsyncSession
|
||||
) -> SessionResponse:
|
||||
# 1. Lookup share
|
||||
share = await db.execute(
|
||||
select(SessionShare)
|
||||
.where(SessionShare.share_token == token)
|
||||
.options(joinedload(SessionShare.session))
|
||||
)
|
||||
share = share.scalar_one_or_none()
|
||||
|
||||
# 2. Validate share
|
||||
if not share or not share.is_active:
|
||||
raise HTTPException(404, "Share not found or has been revoked")
|
||||
|
||||
if share.expires_at and share.expires_at < datetime.now(timezone.utc):
|
||||
raise HTTPException(410, "Share link has expired")
|
||||
|
||||
# 3. Check visibility
|
||||
if share.visibility == "account":
|
||||
if not current_user:
|
||||
raise HTTPException(401, "This share requires authentication")
|
||||
|
||||
# Check account membership
|
||||
session_owner = await db.get(User, share.session.user_id)
|
||||
if current_user.account_id != session_owner.account_id:
|
||||
raise HTTPException(403, "You don't have access to this session")
|
||||
|
||||
# 4. Record view
|
||||
view = SessionShareView(
|
||||
share_id=share.id,
|
||||
viewer_id=current_user.id if current_user else None,
|
||||
viewed_at=datetime.now(timezone.utc),
|
||||
viewer_ip=request.client.host,
|
||||
viewer_user_agent=request.headers.get("user-agent")
|
||||
)
|
||||
db.add(view)
|
||||
|
||||
share.last_viewed_at = datetime.now(timezone.utc)
|
||||
await db.commit()
|
||||
|
||||
# 5. Return read-only session view
|
||||
return SessionResponse.from_orm(share.session)
|
||||
```
|
||||
|
||||
### Share Management Permissions
|
||||
|
||||
**Create share**: `POST /api/v1/sessions/{session_id}/shares`
|
||||
- **Requires**: `session.user_id == current_user.id` (session owner only)
|
||||
- **Validates**: `account.allow_public_shares` if `visibility='public'`
|
||||
|
||||
**Revoke share**: `DELETE /api/v1/shares/{share_id}`
|
||||
- **Requires**: `share.created_by == current_user.id` (share creator only)
|
||||
- Sets `is_active = False` (soft revoke)
|
||||
|
||||
**List my shares**: `GET /api/v1/shares/my-shares`
|
||||
- Returns: `WHERE created_by == current_user.id`
|
||||
- Uses `created_by` index for performance
|
||||
|
||||
### Multiple Shares Per Session
|
||||
|
||||
✅ **Supported**: No unique constraint on `session_id`. Engineer can create:
|
||||
- Public link for customer
|
||||
- Account-only link for internal team
|
||||
- Time-limited link (24h) for contractor
|
||||
- Named links: "Training link for new hires", "Escalation to senior"
|
||||
|
||||
### Cleanup Job for Expired Shares
|
||||
|
||||
**Background task** (runs daily via cron/scheduler):
|
||||
|
||||
```python
|
||||
async def cleanup_expired_shares(
|
||||
db: AsyncSession,
|
||||
retention_days: int = 30
|
||||
):
|
||||
"""Hard-delete expired shares after retention period."""
|
||||
|
||||
cutoff = datetime.now(timezone.utc) - timedelta(days=retention_days)
|
||||
|
||||
# Find shares expired > retention_days ago
|
||||
expired_shares = await db.execute(
|
||||
select(SessionShare)
|
||||
.where(SessionShare.expires_at < cutoff)
|
||||
.where(SessionShare.is_active == False)
|
||||
)
|
||||
|
||||
for share in expired_shares.scalars():
|
||||
await db.delete(share) # Cascades to session_share_views
|
||||
|
||||
await db.commit()
|
||||
|
||||
logger.info(f"Cleaned up {len(list(expired_shares))} expired shares")
|
||||
```
|
||||
|
||||
### Migration 2: Session Sharing
|
||||
|
||||
**File**: `alembic/versions/023_add_session_sharing.py`
|
||||
|
||||
```python
|
||||
def upgrade():
|
||||
# Create session_shares table
|
||||
op.create_table(
|
||||
'session_shares',
|
||||
sa.Column('id', UUID, primary_key=True),
|
||||
sa.Column('session_id', UUID, nullable=False),
|
||||
sa.Column('share_token', sa.String(64), nullable=False, unique=True),
|
||||
sa.Column('share_name', sa.String(100), nullable=True),
|
||||
sa.Column('visibility', sa.String(20), nullable=False),
|
||||
sa.Column('created_by', UUID, nullable=False),
|
||||
sa.Column('created_at', sa.DateTime(timezone=True), nullable=False),
|
||||
sa.Column('updated_at', sa.DateTime(timezone=True), nullable=False),
|
||||
sa.Column('expires_at', sa.DateTime(timezone=True), nullable=True),
|
||||
sa.Column('view_count', sa.Integer, nullable=False, server_default='0'),
|
||||
sa.Column('last_viewed_at', sa.DateTime(timezone=True), nullable=True),
|
||||
sa.Column('is_active', sa.Boolean, nullable=False, server_default='true'),
|
||||
sa.ForeignKeyConstraint(['session_id'], ['sessions.id'], ondelete='CASCADE'),
|
||||
sa.ForeignKeyConstraint(['created_by'], ['users.id'], ondelete='CASCADE'),
|
||||
sa.CheckConstraint("visibility IN ('public', 'account')", name='ck_session_shares_visibility')
|
||||
)
|
||||
|
||||
# Create indexes
|
||||
op.create_index('ix_session_shares_session_id', 'session_shares', ['session_id'])
|
||||
op.create_index('ix_session_shares_share_token', 'session_shares', ['share_token'])
|
||||
op.create_index('ix_session_shares_created_by', 'session_shares', ['created_by'])
|
||||
op.create_index('ix_session_shares_expires_at', 'session_shares', ['expires_at'])
|
||||
op.create_index('ix_session_shares_is_active', 'session_shares', ['is_active'])
|
||||
|
||||
# Create session_share_views table
|
||||
op.create_table(
|
||||
'session_share_views',
|
||||
sa.Column('id', UUID, primary_key=True),
|
||||
sa.Column('share_id', UUID, nullable=False),
|
||||
sa.Column('viewer_id', UUID, nullable=True),
|
||||
sa.Column('viewed_at', sa.DateTime(timezone=True), nullable=False),
|
||||
sa.Column('viewer_ip', sa.String(45), nullable=True),
|
||||
sa.Column('viewer_user_agent', sa.String(500), nullable=True),
|
||||
sa.ForeignKeyConstraint(['share_id'], ['session_shares.id'], ondelete='CASCADE'),
|
||||
sa.ForeignKeyConstraint(['viewer_id'], ['users.id'], ondelete='SET NULL')
|
||||
)
|
||||
|
||||
# Create indexes
|
||||
op.create_index('ix_session_share_views_share_id', 'session_share_views', ['share_id'])
|
||||
op.create_index('ix_session_share_views_viewer_id', 'session_share_views', ['viewer_id'])
|
||||
op.create_index('ix_session_share_views_viewed_at', 'session_share_views', ['viewed_at'])
|
||||
|
||||
# Add account policy
|
||||
op.add_column('accounts', sa.Column('allow_public_shares', sa.Boolean, nullable=False, server_default='true'))
|
||||
|
||||
def downgrade():
|
||||
op.drop_column('accounts', 'allow_public_shares')
|
||||
op.drop_table('session_share_views')
|
||||
op.drop_table('session_shares')
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Summary of Schema Changes
|
||||
|
||||
### Migration 022: Tree Forking
|
||||
**New columns on `trees`:**
|
||||
- `parent_tree_id` (UUID nullable, FK to trees.id, SET NULL on delete)
|
||||
- `fork_reason` (String 255)
|
||||
- `parent_updated_at` (Datetime nullable)
|
||||
|
||||
**Indexes:**
|
||||
- `ix_trees_parent_tree_id`
|
||||
|
||||
### Migration 023: Session Sharing
|
||||
**New tables:**
|
||||
- `session_shares` (13 columns, 5 indexes)
|
||||
- `session_share_views` (6 columns, 3 indexes)
|
||||
|
||||
**New columns on `accounts`:**
|
||||
- `allow_public_shares` (Boolean, default true)
|
||||
|
||||
### No Migration: Custom Steps Enhancement
|
||||
Pure JSONB enhancement with backward compatibility. Existing sessions remain valid.
|
||||
|
||||
---
|
||||
|
||||
## Implementation Checklist
|
||||
|
||||
### Phase 1: Tree Forking (Issues #11)
|
||||
- [ ] Create migration 022 (tree forking fields)
|
||||
- [ ] Update Tree model with new fields and relationships
|
||||
- [ ] Add fork creation endpoint: `POST /api/v1/trees/{tree_id}/fork`
|
||||
- [ ] Add fork list endpoint: `GET /api/v1/trees/{tree_id}/forks`
|
||||
- [ ] Implement parent update detection logic
|
||||
- [ ] Frontend: Fork button on tree detail page
|
||||
- [ ] Frontend: "Parent updated" notification badge
|
||||
- [ ] Tests: Fork creation, orphaning, update detection
|
||||
|
||||
### Phase 2: Custom Steps Enhancement (Issues #4-#7 partial)
|
||||
- [ ] Update CustomStepSchema with new optional fields
|
||||
- [ ] Implement step insertion from library endpoint
|
||||
- [ ] Create StepUsageLog entry on insertion
|
||||
- [ ] Increment StepLibrary.usage_count
|
||||
- [ ] Analytics endpoint: Popular ad-hoc steps
|
||||
- [ ] Frontend: Insert from library button in session
|
||||
- [ ] Tests: Usage tracking, backward compatibility
|
||||
|
||||
### Phase 3: Session Sharing (Issue #15)
|
||||
- [ ] Create migration 023 (session sharing tables)
|
||||
- [ ] Update SessionShare and SessionShareView models
|
||||
- [ ] Add Account.allow_public_shares field
|
||||
- [ ] Implement share creation endpoint with token retry
|
||||
- [ ] Implement share access endpoint with view tracking
|
||||
- [ ] Implement share revocation endpoint
|
||||
- [ ] Setup cleanup job for expired shares
|
||||
- [ ] Frontend: Share button in session detail
|
||||
- [ ] Frontend: Share management modal
|
||||
- [ ] Frontend: Public share view page
|
||||
- [ ] Tests: Share creation, access control, expiration
|
||||
|
||||
---
|
||||
|
||||
## Verification Plan
|
||||
|
||||
### Tree Forking Tests
|
||||
- [ ] Create fork from tree → `parent_tree_id` set correctly
|
||||
- [ ] Update parent tree → fork unaffected
|
||||
- [ ] Delete parent tree (hard) → fork orphaned (`parent_tree_id = NULL`)
|
||||
- [ ] Soft delete parent → fork still references parent
|
||||
- [ ] `has_parent_updates()` returns true after parent updated
|
||||
- [ ] `has_parent_updates()` returns false if parent unchanged
|
||||
- [ ] Fork reason stored and retrieved correctly
|
||||
|
||||
### Custom Steps Tests
|
||||
- [ ] Old session loads without error (backward compat)
|
||||
- [ ] Insert step from library → `source_step_id` populated
|
||||
- [ ] Insert step from library → `StepUsageLog` created
|
||||
- [ ] Insert step from library → `usage_count` incremented
|
||||
- [ ] Insert same step twice → 2 log entries, count += 2
|
||||
- [ ] Delete step → `source_step_id` remains (orphaned)
|
||||
- [ ] Ad-hoc step → `source = "ad-hoc"`, `source_step_id = null`
|
||||
|
||||
### Session Sharing Tests
|
||||
- [ ] Create public share → token generated, unique
|
||||
- [ ] Access public share without auth → works
|
||||
- [ ] Create account-only share → requires auth
|
||||
- [ ] Access account-only share as outsider → 403 error
|
||||
- [ ] Access account-only share as member → works, view logged
|
||||
- [ ] Account with `allow_public_shares=False` → cannot create public share
|
||||
- [ ] Expired share → 410 error
|
||||
- [ ] Revoked share → 404 error
|
||||
- [ ] Multiple shares per session → all work independently
|
||||
- [ ] View tracking → `SessionShareView` entries created
|
||||
- [ ] Delete session → shares cascade deleted
|
||||
|
||||
---
|
||||
|
||||
## Future Enhancements (Out of Scope)
|
||||
|
||||
**Tree Forking:**
|
||||
- Merge fork changes back to parent (pull request model)
|
||||
- Visual diff view: fork vs parent tree
|
||||
- Bulk fork operations: fork all trees in category
|
||||
|
||||
**Custom Steps:**
|
||||
- One-click "Promote to library" for ad-hoc steps
|
||||
- Auto-suggest library steps based on session context
|
||||
- "Save session as tree" reconstruction
|
||||
|
||||
**Session Sharing:**
|
||||
- Embed shares in iframe (oEmbed support)
|
||||
- Password-protected shares
|
||||
- Share groups (one link, multiple sessions)
|
||||
- Share templates with pre-filled expiration/visibility
|
||||
|
||||
---
|
||||
|
||||
## References
|
||||
|
||||
- **Issues**: #4, #5, #6, #7 (Step Library), #11 (User Trees), #15 (Sharing)
|
||||
- **Existing Models**: Tree, Session, StepLibrary, StepRating, StepUsageLog
|
||||
- **Feature Brainstorm**: `docs/plans/2026-02-04-feature-ideas-brainstorm.md`
|
||||
- **Tree Structure**: `backend/scripts/seed_trees.py` (examples)
|
||||
1703
docs/archive/2026-02-08-admin-panel-design.md
Normal file
1703
docs/archive/2026-02-08-admin-panel-design.md
Normal file
File diff suppressed because it is too large
Load Diff
Reference in New Issue
Block a user