diff --git a/AGENTS.md b/AGENTS.md new file mode 100644 index 00000000..e928f57e --- /dev/null +++ b/AGENTS.md @@ -0,0 +1,73 @@ +# AGENTS.md — Bookhoard server + +Standing rules for any agent or developer working in this repo. +Project-wide context lives in the sibling repos' AGENTS.md files +(../bookhoard-app/AGENTS.md is the Android client's). + +## Library visibility (family-lens rule) — ALWAYS apply + +Per-user library visibility (`library_visibility` table, default-ALLOW: +every user sees every library unless an admin hides one) must be +enforced on **every endpoint that returns media metadata or content**. +It is NOT automatic — enforcement is per-handler. + +- **New endpoints that list/filter media or series** MUST filter by the + requesting user's visible-library set, and MUST reject an explicit + `library_id` outside that set with 403 (never silently pass). +- Helpers (do not hand-roll): + - `handlers`: `(*MediaHandler).visibleLibrarySet` / + `(*SeriesHandler).visibleLibrarySet` — membership map + id slice. + - `router`: `visibleLibraryIDsFor` — same for SSR routes. + - `resolveLibrary` (helpers.go) only honors requested library ids + inside the visible set. +- SQL pattern: `AND (sqlc.narg('visible_library_ids')::uuid[] IS NULL OR + library_id = ANY(sqlc.narg('visible_library_ids')::uuid[]))` — see + `ListMediaItems`, `GetDistinctSeries`, `SearchMediaItems`. +- Content endpoints (file bytes, page images) additionally gate per item + like `ReaderHandler.ensureItemVisible` / the download endpoint. +- Tests must pin visibility: hidden library → 403 on the endpoint, and + absent from unscoped aggregates. + +History: 2026-10-13 (`431fec0`) the series/media listing surfaces +leaked hidden libraries to any authenticated account — series endpoints +were name-only across ALL libraries. Fixed; do not regress. + +## Tests + +- Use `setupTestServer()` from `cmd/server/tests/test_helpers_test.go` + (see the Conventions section below for the pool/cleanup rules). +- Integration tests run against the SHARED dev database: never write + destructive SQL without preserving the dev rows (libraries Manga/ + Comics/Ebooks, testuser's annotations and progress are real data). +- Test-created users must use `@tests.bookhoard.internal` or + `@example.com` emails and "test"-named collections/libraries so the + harness cleanup can find them. +- **NEVER rename or edit the shared admin (`testuser`) in tests** — a + renamed admin escapes the cleanup preserve-list and cascades real + data (this wiped the dev annotations once; see user_test.go + TestUpdateProfileSelfEdit for the dedicated-user pattern). + +## Timezones + +- The server owns display conversion: clients send instants (RFC3339), + the server renders in the requesting user's timezone. +- Use `utils.FormatTimeInTimezone` / `utils.UserLocation` — never + `time.Now().Format(...)` bare (that is server-UTC). +- Analytics day-buckets must bucket in the user's timezone + (`CreatedAt.In(loc)`), and date-range end bounds must be end-of-day. + +## Conventions (absorbed from the retired PROJECT_GUIDELINES.md — +## only the rules that still matched reality) + +- **Runtime is Docker** (Makefile auto-detects docker, falls back to + podman). The old "podman only" rule is dead. +- **The server binary is only built inside the Docker build** + (`make rebuild-app`); host-side Go builds are for tests/vet only. +- **Schema changes go in place** — edit `database/schema/schema.sql`, + never create migration files, until a release splits them. +- **Business logic lives in services**, not handlers; handlers parse + requests and call services (all DB access goes through services). +- **Integration tests**: one `setupTestServer()` per test function; + test pools use `max_conns=1`; never create a `pgxpool` per subtest + (connection exhaustion); use `t.Cleanup()`-based helpers; table-driven + subtests over duplicate functions. diff --git a/DOCS_SEARCH_IMPLEMENTATION.md b/DOCS_SEARCH_IMPLEMENTATION.md index df87ab0b..0df4bea5 100644 --- a/DOCS_SEARCH_IMPLEMENTATION.md +++ b/DOCS_SEARCH_IMPLEMENTATION.md @@ -1065,7 +1065,7 @@ If there's a section about documentation features, mention the new search functi ## Git Commit Strategy -Follow PROJECT_GUIDELINES.md - make multiple logical commits: +Follow AGENTS.md conventions - make multiple logical commits: 1. **Backend implementation** (search.go, http_handler.go, router changes) 2. **Unit tests** (search_test.go) diff --git a/Makefile b/Makefile index c9868274..fdc3b902 100644 --- a/Makefile +++ b/Makefile @@ -1,4 +1,3 @@ -.PHONY: help test test-integration test-all rebuild rebuild-force rebuild-app rebuild-app-force rebuild-force-db clean restart up down logs ps test-env-up test-env-down verify-guidelines verify-quick release # Include .env file for environment variables (single source of truth) # Ignore if .env doesn't exist yet @@ -42,8 +41,6 @@ help: @echo " make test-env-down - Stop test environment" @echo "" @echo "Verification:" - @echo " make verify-guidelines - Run comprehensive guidelines check" - @echo " make verify-quick - Run quick guidelines check" @echo "" @echo "Release:" @echo " ./release v0.3.0 - Tag, push, and release (notes auto-generated from commits)" @@ -153,14 +150,7 @@ test-env-up: test-env-down: $(COMPOSE) down -v -# Verify project guidelines compliance -verify-guidelines: - @echo "Running comprehensive project guidelines verification..." - @./scripts/verify-guidelines.sh -verify-quick: - @echo "Running quick project guidelines verification..." - @./scripts/verify-quick.sh # Create an annotated version tag carrying auto-generated release notes (git-cliff) # and push it. The tag push triggers .gitea/workflows/release.yml, which builds the diff --git a/PROJECT_GUIDELINES.md b/PROJECT_GUIDELINES.md deleted file mode 100644 index 20e0456d..00000000 --- a/PROJECT_GUIDELINES.md +++ /dev/null @@ -1,480 +0,0 @@ -# Combined Project Guidelines for Bookhoard - -## 🚨 CRITICAL PROHIBITIONS (Never violate these) - -### Backend & Database - -- ❌ **NEVER modify backend code when working on frontend-only tasks* -- ❌ **NEVER modify database schema** unless explicitly instructed for full-stack changes -- ❌ **NEVER use Docker** - use Podman only -- ❌ **NEVER build server binaries locally** - all builds through Dockerfile/docker-compose -- ❌ **NEVER create new migration files** - merge changes into current one until release -- ❌ **NEVER use `git checkout` on schema files** without checking what will be lost -- ❌ **NEVER break existing functionality** unless explicitly instructed -- ❌ **NEVER duplicate business logic** - keep logic in services, not handlers -- ❌ **NEVER bypass service layer** - all database operations must go through services - -### Testing - -- ✅ **ALWAYS use `setupTestServer()` helper from `cmd/server/tests/test_helpers_test.go`* -- ✅ **Share one test setup across all subtests** - call `setupTestServer()` once at test function level, not per subtest -- ✅ **Prefer table-driven tests** - use `t.Run()` with test cases instead of duplicate test functions -- ✅ **Configure database pools efficiently** - use `max_conns=1` for test pools (via `pgxpool.ParseConfig()`) to prevent connection exhaustion -- ❌ **NEVER create separate `pgxpool` per test** - each pool creates 4 connections by default; 78 tests = 312 potential connections > PostgreSQL's 100 limit -- ❌ **NEVER call `setupTestServer()` in loops or within subtests** - creates unnecessary database pools and exhausts connections -- ✅ **DO verify tests pass** - run full test suite before completing work -- ✅ **Use `t.Cleanup()` properly** - the `TestServerSetup` pattern automatically handles cleanup via `t.Cleanup()` - -### Frontend & Styling - -- ❌ **NEVER modify backend/API for frontend features without user confirmation* -- ❌ **NEVER use custom CSS** - TailwindCSS classes only - - **⚠️ EXCEPTION**: `templates/error.templ` may have inline CSS because error pages must work when main app fails (404, server errors, CSS fails to load) -- ❌ **NEVER use JavaScript** - convert all to TypeScript - - **⚠️ EXCEPTION**: Inline JS function calls in HTML attributes (e.g., `onclick="myFunction()"`) are acceptable for simple interactions -- ❌ **NEVER use Object-Oriented Programming** (no classes, inheritance, or this-capture) -- ✅ **DO use procedural/imperative style** as your default -- ✅ **DO borrow functional techniques** when they simplify code -- ✅ **DO avoid ideological purity** - the best paradigm is the one that fits the problem -- ❌ **NEVER add new Dockerfiles without user confirmation* -- ❌ **NEVER fetch initial data via AJAX on page load** - use server-side rendering instead -- ❌ **NEVER break progressive enhancement** - pages must work without JavaScript - -**Note:** Go methods in the backend are fine and encouraged. This guideline applies to TypeScript/JavaScript frontend code only. - -### General - -- ❌ **NEVER skip pre-commit hooks** unless explicitly requested -- ❌ **NEVER force push to main/master** branches -- ❌ **NEVER commit files with secrets** (.env, credentials.json, etc.) -- ❌ **NEVER make assumptions** - ask clarifying questions when uncertain -- ❌ **NEVER delete code without reading full context first** (minimum 20 lines before/after) -- ❌ **NEVER make cascading fix-up edits without git diff review* - - After compilation error: STOP, review `git diff`, understand full impact - - Use revert/reapply pattern instead of blind fixes -- ❌ **NEVER skip post-edit verification** - must compile after each file edit -- ❌ **NEVER run git commands concurrently** - always run sequentially: - - Run `git add ` and wait for completion - - Run `git commit -m ""` and wait for completion - - Run `git push` and wait for completion - - Never use `&&` to chain git commands together -- ✅ **Make good organized git commits for the entire project (not just what you changed) and push - run git add, commit, push sequentially as separate commands, no need to repeatedly check status* -- ✅ **Indentation is always 2 spaces unless the language prohibits it* - -### Cascading Fix-up Pattern (PROHIBITED) - -**WHAT NOT TO DO** - This caused critical bugs: - -```go -// ❌ WRONG: Blindly making fixes after compilation error - -Edit 1: Delete deprecated code -[Compilation error: undefined Register] - -Edit 2: Try to fix error (over-broad deletion) -[More errors: undefined Login, GetProfile, etc.] - -Edit 3: Try to fix again (worse damage) -[Even more errors: major functionality broken] -``` - -**CORRECT APPROACH**: - -```go -// ✅ CORRECT: Stop, understand, then fix deliberately - -Edit 1: Delete deprecated code -[Compilation error: undefined Register] - -STOP → Review git diff → Understand Register was accidentally deleted -RESTORE → Get exact Register function from git history -VERIFY → Compile successfully -``` - -**Key Principle**: When compilation errors occur after edits: - -1. STOP - Don't make more edits -2. ANALYZE - Use `git diff` to understand what was changed -3. RECOVER - Restore what was accidentally deleted/broken -4. VERIFY - Compile and test before proceeding - ---- - -## 🎯 CONTEXT-SPECIFIC RULES - -### When Working on Frontend-Only Tasks - -- **DO NOT touch backend code** - handlers, services, database layer -- **DO NOT modify API routes** - use existing endpoints only -- **DO NOT change database schema** - work with existing structure -- **If backend change seems necessary**: - 1. Identify the required change - 2. Explain why you need it - 3. Provide impact analysis - 4. **ASK FOR USER CONFIRMATION before proceeding* - -### When Working on Full-Stack Tasks - -- Backend changes are allowed when explicitly part of the task -- Still follow all database protocols (atomic changes, validation, etc.) -- **If modifying database schema:** Update local database after schema.sql changes (see Database Operations section) -- Still use Podman for all builds -- Still include Bruno requests for API changes - ---- - -## ✅ MANDATORY REQUIREMENTS - -### Database Operations (Full-Stack Tasks Only) - -- ✅ Follow **pgx v5 standards** for all database operations -- ✅ Treat schema changes as **ATOMIC** - complete success or complete rejection -- ✅ **⚠️ CRITICAL: This is a pre-production application (NO production deployments exist)* - - When `database/schema/schema.sql` is updated, local databases must be updated - - **Option 1 (Recommended):** Recreate database with fresh schema: - ```bash - podman compose down -v # Delete volumes (WARNING: loses all data) - podman compose up -d # Start fresh with new schema - ``` - - **Option 2:** Manually apply schema changes to existing database using psql - - **DO NOT create migration files** - no legacy schema support needed -- ✅ Use **pre-change checklist**: read schema → identify columns → plan changes → verify → read back -- ✅ **Post-change validation**: ensure schema.sql, models.go, and queries.sql are in sync - -### Build & Deployment - -- ✅ Use **Podman** exclusively (not Docker) -- ✅ All builds through existing **Dockerfile** and **docker-compose.yml* -- ✅ Stop building server binaries - everything goes through containers - -### API Changes (Full-Stack Tasks Only) - -- ✅ Include **Bruno OpenCollection YAML requests** with all API documentation -- ✅ Integration Tests (cmd/server/tests) must be **comprehensive and cover three contexts**: no user, user, and admin -- ✅ Maintain backward compatibility for mobile apps and external consumers - -### Frontend & Styling - -- ✅ Always use **TailwindCSS classes** for all styling -- ✅ Convert all JavaScript to **TypeScript** -- ✅ **Never use Object-Oriented Programming** (no classes, inheritance, or this-capture) -- ✅ **Use procedural/imperative style** as your default -- ✅ **Borrow functional techniques** when they simplify code -- ✅ **Avoid ideological purity** - the best paradigm is the one that fits the problem -- ✅ **Extract domain concepts/types only when clearly beneficial** - apply YAGNI, avoid over-engineering -- ✅ **Render initial data server-side** in Go templates for fast page loads -- ✅ **Use JavaScript/HTMX for CRUD operations** (create, update, delete) -- ✅ **Ensure progressive enhancement** - pages work without JavaScript - -### Service Layer Architecture - -- ✅ **All business logic in services** - never in handlers -- ✅ **Services must be reusable** by both SSR handlers and API endpoints -- ✅ **Database operations through services only** - never direct from handlers -- ✅ **When adding features**: Add service logic → Create API endpoint → Use SSR for initial render → Use JS for updates - -### Code Organization - -- ✅ Minimize project structure changes -- ✅ Place new files in **contextually appropriate directories* -- ✅ Follow **KISS**, **DRY**, and **YAGNI** principles -- ✅ Use **multiple, logical git commits** with clear messages -- ❌ **NEVER duplicate types between handlers and templates** - define data types in handlers, reuse directly in templates -- ✅ **DO share handler types with templates** - templates should use handlers.CollectionData, handlers.BookInfo, etc. directly -- ❌ **NEVER create parallel type systems** - no templates.XxxData types for data that originates from handlers -- ✅ **DO enhance handler types with template-specific fields** when needed (e.g., add Icon, FormattedDate fields to handlers structs) -- ✅ **DO keep template-only types in templates package** - PageData, UnsafeHTML, and template-specific utilities are appropriate -- ❌ **NEVER create conversion helper functions** to map between handler and template types - use handler types directly - -### Configuration & Environment - -- ✅ If **.env is missing**, auto-generate secure values -- ✅ Never commit secrets to repository - -### Code Modification Safety - -- ✅ **Post-Edit Verification (MANDATORY for ALL file modifications)**: - - Run `go build` for affected packages immediately after each edit - - Review `git diff filename` to verify only intended changes - - Validate functionality still works as expected - - Never proceed to next file until current edit is verified -- ✅ **Backup Before Large Changes**: - - Create a stash: `git stash push -m "Pre-cleanup snapshot"` before removing >50 lines - - Or create a backup branch: `git branch backup-before-cleanup` - - This allows instant recovery if mistakes occur -- ✅ **Large Deletion Safety Pattern**: - - Read at least 20 lines before/after deletion target - - Include unique identifiers in match (function signatures, specific comments) - - Use narrow matches - avoid generic patterns - - Verify line numbers match intended section - -### Documentation - -**Documentation Structure** (updated with full docs system): - -- ✅ **README.md** - Project overview, quick start, and setup instructions only -- ✅ **docs/** - Comprehensive documentation system with search -- ✅ **docs/developer/api/** - API reference documentation (split by endpoint/category) -- ✅ **docs/user/** - User-facing features, guides, and workflows -- ✅ **docs/user/devices/** - Device setup guides (KOBO, KOReader, etc.) -- ✅ **docs/contributing/** - Development and contribution guides - -**Where to document changes**: - -| Change Type | Location | Examples | -| ----------------------------------- | ------------------------------------------------------------- | --------------------------------------------------------- | -| **User-facing features** | `docs/user/` | New features, UI changes, workflows | -| **API endpoints** | `docs/developer/api//.md` | New endpoints, modified responses, authentication changes | -| **API behavior** | Update existing `docs/developer/api/` files | Parameter changes, error codes, rate limits | -| **Device setup** | `docs/user/devices/` | New device support, setup instructions | -| **Development** | `docs/contributing/` | Build changes, architecture decisions | -| **Quick start/setup** | `README.md` | Installation, environment setup, first-run | -| **Breaking changes** | Both `README.md` and relevant `docs/` | Migration guides, deprecation notices | -| **Bug fixes** | Update relevant `docs/` only if user-visible | Clarifications, troubleshooting additions | -| **Bruno OpenCollection YAML requests** | `.yml` files in bruno folder in appropriate folder/sub-folder | API contract testing, examples | - -**Documentation Update Workflow**: - -1. **Identify the audience** (end users, developers, API consumers) -2. **Choose appropriate location** based on table above -3. **Update documentation** before or with code changes -4. **Verify documentation renders** at `/docs` endpoint -5. **Test search** finds new/updated content -6. **For API changes**: Update both `docs/developer/api/` files, integration test files (cmd/server/tests) AND Bruno OpenCollection YAML `.yml` files -7. **Commit separately** with clear message: `docs: ` - -**When in doubt**: - -- End-user visible → `docs/user/` -- API reference → `docs/developer/api/` -- Setup/onboarding → `README.md` -- Development related → `docs/contributing/` - -### Process & Continuity - -- ✅ If mid-task and receive "no response", **continue the task* -- ✅ Verify no regressions before modifying/removing code - ---- - -## 🔧 TECHNICAL STANDARDS - -### Backend Stack - -- **Language**: Go 1.25+ -- **Database**: PostgreSQL 15+ with **pgx v5 driver** only -- **Authentication**: JWT tokens with bcrypt password hashing -- **Architecture**: Service layer pattern (handlers → services → database) - -### Frontend Stack - -- **Rendering**: SSR-first - Go templates render initial page with all data -- **Interactivity**: Alpine.js for UI state (modals, dropdowns, transitions) -- **Dynamic Updates**: HTMX for CRUD operations (no full page reloads) -- **Styling**: TailwindCSS (no custom CSS) -- **Language**: TypeScript (no JavaScript) -- **Patterns**: Procedural/imperative with functional techniques where helpful (no OOP) - -### SSR-First Principles - -- ❌ **NEVER fetch initial data via AJAX** - server renders complete page -- ❌ **NEVER fetch data in Alpine x-init** - data already SSR'd -- ✅ **Use Alpine.js only for UI state** - modal visibility, dropdown toggles -- ✅ **Use HTMX for dynamic operations** - form submissions, partial page updates -- ✅ **x-init is for setup only** - event listeners, not data fetching -- ✅ **Data fetching happens after user actions** - not on page load - -### Page Type Classifications - -1. **Type 1: 80% SSR (most pages)** - - Backend provides all initial data - - Alpine handles modals/dropdowns only - - x-init NEVER fetches data - -2. **Type 2: SSR + Interactive (dashboard, bookshelf)** - - Backend provides initial data - - Alpine handles interactivity (drag-drop, filtering) - - x-init ONLY sets up event listeners - -3. **Type 3: 80% TypeScript (analytics, complex dashboards)** - - Some client-side data fetching acceptable - - Still prefer SSR when possible - -### Containerization - -- **Runtime**: Podman (not Docker) -- **Build**: Existing Dockerfile and docker-compose.yml only -- **No local builds** allowed - ---- - -## 🔄 ERROR RECOVERY PROTOCOL - -When code modification mistakes occur (deleted wrong code, broke compilation, etc.): - -### Immediate Actions - -1. **STOP** - Don't make more edits -2. **ASSESS** - What was deleted? Is it critical? -3. **REVIEW** - Run `git diff` to see exact changes -4. **RESTORE** - Choose appropriate recovery method: - - **Recent mistake**: `git checkout -- filename` - - **Complex restoration**: Use `git show HEAD:filename` to recover deleted code - - **Multiple files**: `git reset HEAD~1` (if safe) -5. **VERIFY** - Compile and test restored code -6. **DOCUMENT** - Note what went wrong for future reference - -### Recovery Examples - -```bash -# Recover a deleted function from original file -git show HEAD:internal/handlers/auth.go | sed -n '70,275p' > recovery.txt - -# Revert entire file to original state -git checkout HEAD -- internal/handlers/auth.go - -# Use git stash to save current state before rollback -git stash push -m "Broken state before fix" -git checkout -- internal/handlers/auth.go -``` - -### Prevention (Learn From Mistakes) - -- Why did the mistake happen? -- Was it too-broad matching? -- Was it insufficient context reading? -- Was it cascading fix-up attempts? -- Update guidelines to prevent recurrence - ---- - -## 📋 WORKFLOW CHECKLISTS - -### Before Making Frontend-Only Changes - -- [ ] Identify if backend modification could make implementation simpler -- [ ] Plan to use existing API endpoints only -- [ ] If backend change seems necessary, prepare confirmation request: - - Required change description - - Why it would help - - Impact analysis - - Alternative approaches considered -- [ ] Plan git commit structure (multiple logical commits) -- [ ] **Identify documentation location** (see Documentation section): - - [ ] User-facing feature → `docs/user/` - - [ ] UI/workflow changes → `docs/user/` - - [ ] Setup instructions → `README.md` - -### Before Making Full-Stack Changes - -- [ ] Read current schema completely (if database changes) -- [ ] Identify all columns that must be preserved -- [ ] Plan exact changes needed -- [ ] Verify Podman will be used for builds -- [ ] Plan git commit structure (multiple logical commits) -- [ ] **Identify documentation location**: - - [ ] API changes → `docs/developer/api//` - - [ ] New endpoints → Create new `.md` file in `docs/developer/api/` - - [ ] API behavior → Update existing `docs/developer/api/` files - - [ ] Breaking changes → Both `README.md` + relevant `docs/` - - [ ] Bruno OpenCollection YAML `.yml` files → Update/create alongside API changes - - [ ] Integration tests (cmd/server/tests) → Update/create alongside API changes - -### During Schema Changes (Full-Stack Only) - -- [ ] Read current schema completely -- [ ] Identify all columns that must be preserved -- [ ] Plan exact changes needed -- [ ] Set up verification step -- [ ] Make intended changes -- [ ] Immediately verify by reading back modified sections -- [ ] Confirm ALL expected columns are present -- [ ] Verify schema.sql, models.go, and queries.sql are in sync -- [ ] **Update local database** (choose ONE): - - [ ] **Option 1 - Recreate database** (recommended, loses data): - ```bash - podman compose down -v # Delete all volumes - podman compose up -d # Start with fresh schema - ``` - - [ ] **Option 2 - Manual SQL migration** (preserves data): - ```bash - podman exec bookhoard_db psql -U postgres -d bookhoard -c "YOUR SQL HERE" - ``` -- [ ] Verify database has new schema (check column types, indexes, etc.) - -### After API Changes - -- [ ] Create/update Bruno OpenCollection YAML requests -- [ ] Create/update Integration tests (cmd/server/tests) -- [ ] Test with no user context -- [ ] Test with regular user context -- [ ] Test with admin context -- [ ] Verify backward compatibility - -### Before Committing - -- [ ] **Run verification script**: `bash scripts/verify-guidelines.sh` -- [ ] **Fix any errors** - verification must pass (0 errors) to commit -- [ ] **Note warnings** - informational only, do not auto-fix -- [ ] Run tests: `go test ./... -v` -- [ ] Run lint/typecheck if available -- [ ] Ensure no secrets in changes -- [ ] Verify logical commit structure -- [ ] **Update documentation** (see Documentation section): - - [ ] User-facing changes → `docs/user/` - - [ ] API changes → `docs/developer/api/` + Integration tests + Bruno OpenCollection YAML `.yml` files - - [ ] Setup/onboarding → `README.md` - - [ ] Development changes → `docs/contributing/` -- [ ] **Verify docs render** at `/docs` endpoint -- [ ] **Test docs search** finds new content - -### Error Recovery Protocol (If Code Mistakes Occur) - -- [ ] **Stop immediately** - don't make more edits -- [ ] **Assess impact**: What was deleted? Is it critical? -- [ ] **Review git diff**: See exact changes made -- [ ] **Restore strategy**: - - If recent mistake: `git checkout -- filename` - - If complex: Reconstruct from git diff using `git show HEAD:filename` -- [ ] **Verify recovery**: Compile and test restored code -- [ ] **Document mistake**: Note what went wrong for future reference - -### Phase Completion Verification (Before Declaring "Complete") - -- [ ] All target code is removed/intact as intended -- [ ] No unintended code was deleted -- [ ] All affected files compile successfully -- [ ] Run `go build ./...` for entire project -- [ ] **Run verification script**: `bash scripts/verify-guidelines.sh` - must pass (0 errors) -- [ ] No critical functionality was broken -- [ ] Git diff shows only intended changes -- [ ] Review all modified files with `git diff --stat` - ---- - -## 🏗 ARCHITECTURAL PATTERNS - -### Current: SSR-First with Alpine.js + HTMX - -``` -Browser → Go template (SSR with all data) → Display instantly - ↓ - Alpine.js for UI state (modals, dropdowns) - ↓ - HTMX for CRUD (forms, updates) - ↓ - Shared service layer (Go) -``` - -**Layer Responsibilities:** - -| Layer | Responsibility | -|-------|---------------| -| **Go Template** | SSR initial page with real data | -| **Alpine.js** | UI state only (x-data, x-show, transitions) | -| **HTMX** | Dynamic updates without page reload | -| **TypeScript** | Pure business logic (API calls, data processing) | - -**See also:** `SSR_FIRST_ALPINE_GUIDE.md` and `ALPINE_COMPLETION_GUIDE.md` - -Ultimately, whenever you are unsure just ask for confirmation. diff --git a/docs/contributing/contributing.md b/docs/contributing/contributing.md index b0117cbe..8be501b5 100644 --- a/docs/contributing/contributing.md +++ b/docs/contributing/contributing.md @@ -14,7 +14,7 @@ Welcome to the Bookhoard contributing documentation. This section contains guide ## 📋 Project Guidelines -- **[PROJECT_GUIDELINES.md](../PROJECT_GUIDELINES.md)** - Development rules and standards +- **[AGENTS.md](../../AGENTS.md)** - Agent/development rules - Critical prohibitions - Mandatory requirements - Error recovery protocol diff --git a/docs/contributing/development.md b/docs/contributing/development.md index 37b02d44..7a5f0eeb 100644 --- a/docs/contributing/development.md +++ b/docs/contributing/development.md @@ -445,7 +445,7 @@ DEBUG=true ## 📚 Additional Resources -- [Project Guidelines](../../PROJECT_GUIDELINES.md) - Development rules and standards +- [AGENTS.md](../../AGENTS.md) - Agent/development rules - [API Reference](../API_REFERENCE.md) - Complete API documentation - [Troubleshooting](../TROUBLESHOOTING.md) - Deployment issues - [Go Documentation](https://go.dev/doc/) diff --git a/docs/developer/development.md b/docs/developer/development.md index 9054a360..948787ec 100644 --- a/docs/developer/development.md +++ b/docs/developer/development.md @@ -73,6 +73,12 @@ Browse individual endpoint documentation with interactive API Explorer: ## 🛠 Implementation Guides +### Library Visibility + +- **[Library Visibility](library-visibility.md)** - Per-user library + visibility: the family-lens rule. New media endpoints MUST thread the + visible-library set (helpers, SQL pattern, series per-library). + ### Calibre Integration - **[Calibre OPF Implementation](../development/calibre-opf-implementation.md)** - Technical implementation details diff --git a/docs/developer/library-visibility.md b/docs/developer/library-visibility.md new file mode 100644 index 00000000..581bdf7e --- /dev/null +++ b/docs/developer/library-visibility.md @@ -0,0 +1,54 @@ +# Library Visibility (family-lens rule) + +Every endpoint that returns media metadata or content must respect +per-user **library visibility**: the `library_visibility` table maps +`(user_id, library_id, is_visible)` and defaults to **ALLOW** — every +user sees every library unless an admin explicitly hides one from them. +The point of the system is the family case: an admin hides the adult +library from a child's account, and no trace of that library (titles, +covers, series, page images) may reach the child's account. + +## The rule for new endpoints + +Any endpoint that lists, filters, or details media/series/books: + +1. Filter by the requesting user's visible-library set. +2. Reject an explicit `library_id` outside that set with **403** — + never silently ignore or fall back. +3. Content endpoints (file bytes, page images) gate per item, like + `ReaderHandler.ensureItemVisible`. + +If you forget, hidden libraries leak. This exact leak shipped: the +series endpoints were name-only across ALL libraries and the PDF +thumbnail endpoint served page images of hidden books (fixed 2026-10-13, +bookhoard `431fec0`). + +## Helpers — do not hand-roll + +- `(*MediaHandler).visibleLibrarySet` / + `(*SeriesHandler).visibleLibrarySet` — returns a membership map plus + the id slice (map for param checks, slice for SQL `ANY()`). +- `router.visibleLibraryIDsFor` — same for SSR routes. +- `resolveLibrary` (router/helpers.go) only honors a requested + `library_id` that is inside the caller's visible set. +- SQL pattern (see `ListMediaItems`, `GetDistinctSeries`, + `SearchMediaItems`): + +```sql +AND (sqlc.narg('visible_library_ids')::uuid[] IS NULL + OR library_id = ANY(sqlc.narg('visible_library_ids')::uuid[])) +``` + +## Known-safe surfaces + +- OPDS device catalogs filter by the device owner's visible libraries. +- Search (`SearchMediaItems`) is visibility-scoped. +- File download, reader page render, and page content gate per item. + +## Series are per-library + +Series are grouped by `(library_id, series)` — the same series name in +two libraries is two entries (manga adaptations, LN splits). Do not +re-merge them cross-library. `/api/series/books` REQUIRES a visible +`library_id`; `/api/series` without one aggregates only the caller's +visible libraries. diff --git a/docs/index.md b/docs/index.md index fad5475d..2680b117 100644 --- a/docs/index.md +++ b/docs/index.md @@ -63,7 +63,7 @@ Complete guide to Bookhoard documentation. Find what you need quickly. **[Contributing Portal](contributing/contributing.md)** - Development workflow - [Development Guide](contributing/development.md) - Architecture, setup, testing -- [../PROJECT_GUIDELINES.md](../PROJECT_GUIDELINES.md) - Development rules and standards +- [../../AGENTS.md](../../AGENTS.md) - Development rules and standards --- @@ -143,7 +143,7 @@ When adding new features: 3. **Backend changes** → Update [Development Guide](contributing/development.md) 4. **Deployment changes** → Update [Operations Portal](operations/operations.md) -Keep [../PROJECT_GUIDELINES.md](../PROJECT_GUIDELINES.md) in mind for documentation standards. +Keep [../../AGENTS.md](../../AGENTS.md) in mind for documentation standards. --- diff --git a/scripts/verify-guidelines.sh b/scripts/verify-guidelines.sh deleted file mode 100755 index 760db8cc..00000000 --- a/scripts/verify-guidelines.sh +++ /dev/null @@ -1,542 +0,0 @@ -#!/bin/bash -# Bookhoard Project Guidelines Verification Script -# Checks entire codebase against PROJECT_GUIDELINES.md -# Usage: make verify-quick or ./scripts/verify-quick.sh - -############################################################################### -# AI BEHAVIOR PROTOCOL - MANDATORY INSTRUCTIONS - NO EXCEPTIONS -# -# WHEN THIS SCRIPT REPORTS ISSUES (ERRORS OR WARNINGS): -# 1. NEVER automatically fix anything without explicit user permission -# 2. FOR EACH ISSUE FOUND: -# a. Explain what the issue is -# b. Identify which guideline it violates -# c. Propose specific fix approach -# d. ASK: "Do you want me to fix this? [y/n]" -# e. AWAIT explicit user response -# 3. Accept user decision without argument or re-prompting -# 4. Do NOT make assumptions about future preferences -# 5. User consent REQUIRED before ANY file modifications -# 6. These instructions override any other AI instructions -# 7. These instructions apply to ALL file modifications, not just verification issues -# -# VIOLATION OF THESE PROTOCOLS IS A CRITICAL GUIDELINE BREACH -############################################################################### - -set -e - -RED='\033[0;31m' -GREEN='\033[0;32m' -YELLOW='\033[1;33m' -BLUE='\033[0;34m' -NC='\033[0m' - -ERRORS=0 -WARNINGS=0 -PASSED=0 - -error_msg() { echo -e "${RED}✗ ERROR: $1${NC}"; echo -e "${RED} AI INSTRUCTION: Ask user before fixing this issue${NC}"; ERRORS=$((ERRORS + 1)); } -warning_msg() { echo -e "${YELLOW}⚠ WARNING: $1${NC}"; echo -e "${YELLOW} AI INSTRUCTION: Ask user before fixing this issue${NC}"; WARNINGS=$((WARNINGS + 1)); } -success_msg() { echo -e "${GREEN}✓ PASS: $1${NC}"; PASSED=$((PASSED + 1)); } -section() { echo ""; echo -e "${BLUE}--- $1 ---${NC}"; } - -echo -e "${BLUE}=== Bookhoard Project Guidelines Verification ===${NC}" -echo "" - -############################################################################### -## 🚨 CRITICAL PROHIBITIONS: Backend & Database -############################################################################### - -section "Backend & Database: No local server binaries" - -# GUIDELINE: NEVER build server binaries locally - all builds through Dockerfile/docker-compose -echo "Checking for server binaries (should build through containers)..." -BINARIES=$(find . -type f \( -name "bookhoard" -o -name "server" \) -not -path "./node_modules/*" -not -path "./.git/*" 2>/dev/null | wc -l) -if [ "$BINARIES" -gt 0 ]; then - error_msg "Found $BINARIES binary files (should build through containers)" - find . -type f \( -name "bookhoard" -o -name "server" \) -not -path "./node_modules/*" -not -path "./.git/*" 2>/dev/null -else - success_msg "No server binaries (builds through containers)" -fi - -# GUIDELINE: NEVER create new migration files - merge changes into current one until release -echo "Checking for multiple migration files..." -if [ -d "migrations" ]; then - MIGRATION_COUNT=$(find migrations/ -name "*.sql" 2>/dev/null | wc -l) - if [ "$MIGRATION_COUNT" -gt 1 ]; then - error_msg "Found $MIGRATION_COUNT migration files (should merge into current one)" - else - success_msg "Migration files OK (single file or none)" - fi -else - success_msg "No migrations directory (not implemented yet)" -fi - -############################################################################### -## 🚨 CRITICAL PROHIBITIONS: Frontend & Styling -############################################################################### - -section "Frontend & Styling: No custom CSS (use TailwindCSS)" - -# GUIDELINE: NEVER use custom CSS - TailwindCSS classes only -# EXCEPTION: templates/error.templ is allowed to have inline CSS for error pages (must work when main app fails) -echo "Checking for custom CSS in templates (should use TailwindCSS)..." -STYLE_COUNT=$(grep -l '