Add code review workflow and project cleanup tasks

- Create docs/project-workflows/code-review.org with comprehensive
  code review checklist based on Code Review Pyramid framework
- Add 14 new tasks to todo.org from senior developer code review:
  - Priority A: README GRUB refs, missing LICENSE, skeleton script
  - Priority B: Stale files, Makefile lint, documentation gaps
  - Priority C: Style consistency, .editorconfig, test docs
- Add Makefile targets to todo.org: deps, lint, deploy
- Create docs/session-context.org for session tracking

3 files changed, 449 insertions, 0 deletions

diff --git a/docs/project-workflows/code-review.org b/docs/project-workflows/code-review.org
new file mode 100644
index 0000000..79ef0ed
--- /dev/null
+++ b/docs/project-workflows/code-review.org
@@ -0,0 +1,275 @@
+#+TITLE: Code Review Workflow
+#+AUTHOR: Craig Jennings & Claude
+#+DATE: 2026-01-23
+
+* Overview
+
+This workflow guides code review with a focus on what matters most. It's based on the Code Review Pyramid (Gunnar Morling) which prioritizes review effort by cost of change - spend the most time on things that are hardest to fix later.
+
+#+begin_example
+                    ┌─────────────────┐
+                    │   CODE STYLE    │  ← Automate
+                ┌───┴─────────────────┴───┐
+                │         TESTS           │  ← Automate
+            ┌───┴─────────────────────────┴───┐
+            │        DOCUMENTATION            │
+        ┌───┴─────────────────────────────────┴───┐
+        │      IMPLEMENTATION SEMANTICS           │  ← Focus
+    ┌───┴─────────────────────────────────────────┴───┐
+    │              API SEMANTICS                      │  ← Most Focus
+    └─────────────────────────────────────────────────┘
+
+    ↑ HARDER TO CHANGE LATER = MORE HUMAN REVIEW TIME
+#+end_example
+
+*Key Insight:* A formatting issue is a 2-minute fix. A breaking API change after release is a multi-sprint disaster. Prioritize accordingly.
+
+* When to Use This Workflow
+
+Use this workflow when:
+- Reviewing pull requests / merge requests
+- Doing self-review before submitting code
+- Pair reviewing with another developer
+
+* Effort Scaling
+
+Not all changes need the same scrutiny:
+
+| Change Type | Review Depth | Time |
+|-------------+--------------+------|
+| Typo/docs fix | Quick scan for correctness | 2-5 min |
+| Bug fix | Verify fix + no regressions | 10-15 min |
+| New feature | Full checklist, deep dive on logic | 30-60 min |
+| Architectural change | Bring in senior reviewers, verify design | 60+ min |
+
+* The Core Checklist
+
+Use this for every review. If you answer "no" or "maybe" to any item, dig deeper using the relevant Deep Dive section.
+
+** 1. API Semantics (Hardest to Change - Focus Most Here)
+
+These questions matter most because changes here break things for other people.
+
+- [ ] *Simple?* Is the API as small as possible but as large as needed?
+- [ ] *Predictable?* Does it follow existing patterns and the principle of least surprise?
+- [ ] *Clean boundaries?* Is there clear separation between public API and internal implementation?
+- [ ] *Generally useful?* Is the API helpful for common cases, not just one specific scenario?
+- [ ] *Backwards compatible?* Are there any breaking changes to things users already rely on?
+
+** 2. Implementation Semantics (Focus Here)
+
+Once the API looks good, check how the code actually works.
+
+- [ ] *Correct?* Does it actually do what the requirements/ticket says?
+- [ ] *Logically sound?* Are there edge cases, off-by-one errors, or logic bugs?
+- [ ] *Simple?* Could this be simpler? Is there unnecessary complexity or over-engineering?
+- [ ] *Robust?* Does it handle errors gracefully? Concurrency issues?
+- [ ] *Secure?* Is user input validated? No hardcoded secrets? (See Security Deep Dive)
+- [ ] *Performant?* Any expensive operations, N+1 queries, or scaling concerns? (See Performance Deep Dive)
+- [ ] *Observable?* Is there appropriate logging, metrics, or tracing?
+- [ ] *Dependencies OK?* Do new libraries pull their weight? License acceptable?
+
+** 3. Documentation
+
+Good code still needs good explanations.
+
+- [ ] *New features documented?* README, API docs, or user guides updated?
+- [ ] *Code comments explain "why"?* Complex logic or non-obvious decisions explained?
+- [ ] *Readable?* Documentation is clear, accurate, no major errors?
+
+** 4. Tests (Automate the "Pass/Fail" Check)
+
+Don't just check if tests pass - check if they're good tests.
+
+- [ ] *Tests exist?* Are new features and bug fixes covered by tests?
+- [ ] *Test the right things?* Do tests verify behavior, not implementation details?
+- [ ] *Edge cases covered?* Are boundary conditions and error paths tested?
+- [ ] *Right test type?* Unit tests where possible, integration tests where necessary?
+- [ ] *Would catch regressions?* If someone breaks this later, will a test fail?
+
+** 5. Code Style (Automate This)
+
+Let linters and formatters handle these. Only check manually if tooling isn't set up.
+
+- [ ] *Linter passes?* No style violations flagged by automated tools?
+- [ ] *Readable?* Methods reasonable length? Clear naming?
+- [ ] *DRY?* No unnecessary duplication?
+
+** 6. Process & Collaboration
+
+Code review is a conversation, not an inspection.
+
+- [ ] *PR has context?* Does the description explain WHY, not just what?
+- [ ] *Reasonable size?* Is this reviewable in one sitting? (<400 lines ideal)
+- [ ] *Self-reviewed?* Has the author clearly reviewed their own code first?
+- [ ] *No debug artifacts?* Console.logs, commented code, TODOs without tickets removed?
+
+* Deep Dives
+
+Use these when the Core Checklist flags a concern. Each section provides detailed checks for specific areas.
+
+** Security Deep Dive
+
+When "Is it secure?" raises concerns, check these:
+
+*** Input Validation
+- [ ] All user input validated and sanitized
+- [ ] SQL queries parameterized (no string concatenation)
+- [ ] Output encoded to prevent XSS
+- [ ] File uploads validated (type, size, content)
+- [ ] Path traversal prevented
+
+*** Authentication & Authorization
+- [ ] Auth checks at every access point
+- [ ] Session management secure
+- [ ] CSRF protections for state-changing operations
+- [ ] Rate limiting where appropriate
+
+*** Secrets & Data
+- [ ] No hardcoded credentials (use env vars/secrets manager)
+- [ ] Sensitive data encrypted at rest and in transit
+- [ ] Error messages don't leak system information
+- [ ] Logs exclude sensitive data
+
+*** Common Vulnerabilities (OWASP)
+- [ ] No SQL injection vectors
+- [ ] No command injection vectors
+- [ ] No unsafe deserialization
+- [ ] Proper HTTP security headers
+
+** Performance Deep Dive
+
+When "Is it performant?" raises concerns, check these:
+
+*** Database
+- [ ] No N+1 query problems
+- [ ] Appropriate indexes exist
+- [ ] No SELECT * - explicit columns only
+- [ ] Pagination for large result sets
+- [ ] Connection pooling used
+
+*** Code Efficiency
+- [ ] No unnecessary loops or iterations
+- [ ] Appropriate data structures (HashMap vs List lookups)
+- [ ] Expensive operations cached where beneficial
+- [ ] Large collections processed efficiently (streaming vs loading all)
+- [ ] Resources properly closed (connections, file handles)
+
+*** Scaling
+- [ ] Will this work with 10x/100x the data?
+- [ ] Async operations used where beneficial
+- [ ] No blocking operations in hot paths
+
+** Concurrency Deep Dive
+
+When reviewing multi-threaded or async code:
+
+- [ ] Shared mutable state properly synchronized
+- [ ] No race conditions
+- [ ] Lazy initialization is thread-safe
+- [ ] Getters don't return mutable internal objects
+- [ ] Thread-safe collections used where needed
+- [ ] Consistent lock ordering (no deadlocks)
+- [ ] Loop variables passed into closures/lambdas (not captured)
+- [ ] Proper timeouts on blocking operations
+- [ ] Tests run with race detector where available
+
+** Error Handling Deep Dive
+
+When reviewing error handling and logging:
+
+- [ ] Exceptions caught at appropriate levels
+- [ ] System "fails safe" (deny on error, not grant)
+- [ ] Transactions rolled back on failure
+- [ ] Error messages help debugging but don't leak info
+- [ ] Correlation IDs for tracing across services
+- [ ] Log levels appropriate (DEBUG/INFO/WARN/ERROR)
+- [ ] Critical failures trigger alerts
+
+** API Compatibility Deep Dive
+
+When reviewing API changes:
+
+*** Non-Breaking (Safe)
+- Adding optional fields
+- Adding new endpoints
+- Making required fields optional
+- Adding new enum values (if clients handle unknown)
+
+*** Breaking (Dangerous)
+- Removing fields or endpoints
+- Changing field types
+- Making optional fields required
+- Changing semantic meaning of fields
+- Changing response status codes
+
+*** If Breaking Changes Required
+- [ ] Version bumped appropriately
+- [ ] Migration path documented
+- [ ] Deprecation warnings added
+- [ ] Transition period planned
+
+** Dependencies Deep Dive
+
+When new dependencies are added:
+
+- [ ] Actually necessary (not just convenience)
+- [ ] From trusted, reputable source
+- [ ] Actively maintained (recent commits, responsive maintainers)
+- [ ] No known vulnerabilities (check Dependabot/Snyk)
+- [ ] License compatible with project
+- [ ] Doesn't bloat dependency tree excessively
+- [ ] Version pinned in lock file
+
+* How to Give Feedback
+
+** Frame as Questions
+Bad: "This is wrong."
+Good: "I noticed we used X here - would Y be more efficient for this case?"
+
+** Distinguish Severity
+- *Blocker:* Must fix before merge (security issue, breaking bug)
+- *Should fix:* Important but could be follow-up PR
+- *Suggestion:* Nice to have, author's discretion
+- *Nit:* Minor style preference, totally optional
+
+** Call Out Good Stuff
+Reviews are for learning too. If you see a clever solution or clean pattern, say so.
+
+** Be Timely
+Don't let PRs sit. If you're stuck, ask someone to pair-review with you.
+
+* Anti-Patterns to Avoid
+
+** The Empty LGTM
+Never rubber-stamp with "Looks Good To Me" without actually reviewing. Even noting one thing you liked shows you read it.
+
+** Nitpicking
+Don't comment on spaces, tabs, or semicolons. Let automated tools handle formatting. Focus on logic and design.
+
+** The Stall
+Don't let PRs languish for days. Review within 24 hours or communicate delays.
+
+** Bikeshedding
+Don't spend 30 minutes debating variable names while ignoring architectural concerns.
+
+** Review by Line Count
+400 lines of careful refactoring may need less scrutiny than 40 lines of new auth logic. Adjust effort to risk, not size.
+
+* Validation Checklist
+
+Before approving, verify:
+
+- [ ] Core checklist complete (all 5 pyramid levels)
+- [ ] Any "maybe" answers investigated with Deep Dives
+- [ ] Feedback framed constructively
+- [ ] Blockers clearly marked vs suggestions
+- [ ] You could explain this code to someone else
+
+* Sources
+
+- [[https://www.morling.dev/blog/the-code-review-pyramid/][The Code Review Pyramid]] - Gunnar Morling
+- [[https://google.github.io/eng-practices/review/][Google Engineering Practices]] - Code Review Guidelines
+- [[https://owasp.org/www-project-code-review-guide/][OWASP Code Review Guide]] - Security Checklist
+- [[https://github.com/code-review-checklists/java-concurrency][Java Concurrency Checklist]] - Thread Safety
+- [[https://go.dev/wiki/CodeReviewConcurrency][Go Concurrency Wiki]] - Race Conditions
diff --git a/docs/session-context.org b/docs/session-context.org
new file mode 100644
index 0000000..580be99
--- /dev/null
+++ b/docs/session-context.org
@@ -0,0 +1,51 @@
+#+TITLE: Session Context - Active Session
+#+DATE: 2026-01-23
+
+* Current Session: Friday 2026-01-23 @ 11:28 CST
+
+** Current Task
+Code review of archzfs project complete. Issues documented in todo.org.
+
+** Completed This Session
+- Merged zfsbootmenu branch to main (fast-forward, pushed to origin)
+- ratio booted successfully with ZFSBootMenu
+- Researched code review best practices (10+ web searches)
+- Created docs/project-workflows/code-review.org
+  - Core Checklist: ~27 items across pyramid levels
+  - Deep Dives: Security, Performance, Concurrency, Error Handling, API Compatibility, Dependencies
+- Conducted senior developer code review of entire archzfs codebase
+- Added 14 new tasks to todo.org based on code review findings
+
+** Code Review Findings Summary
+*** Critical (Priority A)
+- README.org has outdated GRUB references (now uses ZFSBootMenu)
+- Missing LICENSE file (referenced but doesn't exist)
+- custom/archsetup-zfs is a non-functional skeleton
+- Initial password should be configurable, not hardcoded
+
+*** Important (Priority B)
+- Stale SESSION-CONTEXT.md should be deleted
+- PLAN-zfsbootmenu-implementation.org should move to docs/
+- Makefile lint target swallows errors
+- Unclear directories need documentation or gitignore
+- Empty docs/someday-maybe.org
+
+*** Cleanup (Priority C)
+- Inconsistent shebangs across scripts
+- Inconsistent email addresses
+- No .editorconfig for formatting
+- Test scripts need documentation
+
+** Files Created This Session
+- docs/project-workflows/code-review.org - Code review workflow
+
+** Files Modified This Session
+- todo.org - Added Makefile targets, code review tasks (14 new items)
+
+** Pending User Review
+- docs/project-workflows/code-review.org (task in todo.org)
+
+** Next Steps
+- User to review code-review.org document
+- Address Priority A issues from code review
+- Delete SESSION-CONTEXT.md (root) after confirming this file is canonical
diff --git a/todo.org b/todo.org
index 9415a18..7d7b192 100644
--- a/todo.org
+++ b/todo.org
@@ -1,5 +1,9 @@
 * Open Work
 
+** TODO [#A] Review code review workflow document and provide feedback
+Review [[file:docs/project-workflows/code-review.org][docs/project-workflows/code-review.org]] and refine based on feedback.
+Created: 2026-01-23
+
 ** TODO [#A] Fix mkinitcpio configuration in install-archzfs (causes boot failure)
 After kernel updates or mkinitcpio regeneration, systems fail to boot because install-archzfs
 leaves incorrect mkinitcpio configuration from the live ISO environment.
@@ -252,6 +256,98 @@ It provides boot environment selection, snapshot rollback from boot menu, and re
 *** Reference
 https://zfsbootmenu.org/
 
+** TODO [#A] Update README.org - remove all GRUB references (now uses ZFSBootMenu)
+README.org contains multiple outdated references to GRUB that are now incorrect:
+- Line 19: "EFI Boot Redundancy - GRUB installed on all disks" - now uses ZFSBootMenu
+- Lines 417-472: Entire section on "grub-zfs-snap" and GRUB snapshot boot entries - doesn't exist
+- Lines 98-100: Project structure lists grub-zfs-snap, zfs-snap-prune, 40_zfs_snapshots - files don't exist
+
+*** Actions
+- Remove/update "EFI Boot Redundancy" line to mention ZFSBootMenu
+- Delete or rewrite "ZFS Snapshot Boot Entries (grub-zfs-snap)" section
+- Update project structure to reflect actual files
+- Update "Post-Installation" section for ZFSBootMenu workflow
+
+** TODO [#A] Add LICENSE file (GPL-3.0)
+README.org line 723 references [[file:LICENSE][LICENSE]] but the file doesn't exist.
+Create LICENSE file with GPL-3.0 text as stated in README.
+
+** TODO [#A] Delete or complete custom/archsetup-zfs
+The script has full function definitions but main() just prints "this is a skeleton".
+A skeleton script that pretends to work is worse than nothing.
+
+*** Options
+1. Delete it entirely - users can run archsetup from ~/code/archsetup
+2. Complete the implementation
+3. Replace with a simple launcher that calls archsetup with ZFS-specific flags
+
+** TODO [#A] Add initial user password to install-archzfs config
+Currently hardcoded as "welcome" in archsetup-zfs. Should be configurable via:
+- Interactive prompt during install-archzfs
+- Config file option for unattended installs
+- Document that password must be changed on first login
+
+** TODO [#B] Delete stale SESSION-CONTEXT.md
+SESSION-CONTEXT.md in project root is from 2026-01-19 and references old GRUB workflow.
+Superseded by docs/session-context.org. Delete to avoid confusion.
+
+** TODO [#B] Move PLAN-zfsbootmenu-implementation.org to docs/
+Implementation plan files should be in docs/ or archived after completion.
+The plan is complete - ZFSBootMenu is now the bootloader.
+
+** TODO [#B] Clean up docs/ directory
+- Delete docs/someday-maybe.org (empty, 0 bytes)
+- Move date-specific docs from assets/ to docs/ for consistency
+- Document or delete docs/scripts/ directory (unclear purpose)
+
+** TODO [#B] Fix Makefile lint target to fail on errors
+Current lint target has `|| true` which swallows shellcheck errors:
+#+BEGIN_SRC makefile
+lint:
+    @shellcheck -x build.sh scripts/*.sh custom/install-archzfs ... || true
+#+END_SRC
+
+Change to actually fail on lint errors so CI can catch issues.
+
+** TODO [#B] Document or gitignore unclear directories
+These directories exist but aren't documented or gitignored:
+- zfs-packages/ - unclear purpose
+- reference-repos/ - unclear purpose
+- test-logs/ - should probably be gitignored
+
+** TODO [#B] Fill in README.org #+AUTHOR field
+Line 2 has empty #+AUTHOR: - looks unfinished. Add author info.
+
+** TODO [#C] Standardize shell script conventions
+*** Shebang inconsistency
+- build.sh: #!/bin/bash
+- zfssnapshot: #!/bin/env bash
+- archsetup-zfs: #!/bin/sh
+Pick one convention (recommend #!/usr/bin/env bash for portability)
+
+*** Email inconsistency
+- Some files: c@cjennings.net
+- archsetup-zfs: craigmartinjennings@gmail.com
+Standardize to one email address.
+
+** TODO [#C] Add .editorconfig for consistent formatting
+No project-wide formatting rules. Add .editorconfig to enforce:
+- Indent style (spaces vs tabs)
+- Indent size
+- End of line
+- Trim trailing whitespace
+- Final newline
+
+** TODO [#C] Consolidate test scripts documentation
+scripts/ has multiple test files with unclear relationships:
+- test-vm.sh - Manual VM testing
+- sanity-test.sh - Quick automated checks
+- test-install.sh - Installation testing
+- full-test.sh - Comprehensive testing
+- test-zfs-snap-prune.sh - Unit tests for prune script
+
+Document the testing strategy and when to use each script.
+
 ** TODO [#B] Set up CI/CD pipeline for automated ISO builds
 
 *** Options to evaluate
@@ -365,6 +461,33 @@ Extract into library of functions that can be sourced and tested:
 - Reusable functions for other projects
 - Clearer code organization
 
+** TODO [#B] Create Makefile with distinct build targets
+Replace or supplement build.sh with a Makefile for cleaner build orchestration.
+
+*** Proposed targets
+- make deps - Install all dependencies (pacman + AUR) needed to build and test
+- make lint - Run shellcheck on all bash scripts
+- make build - Full ISO build (current build.sh behavior)
+- make clean - Remove work/ and output/ directories
+- make test - Run VM tests (single disk, mirror, raidz) - depends on lint
+- make test-quick - Quick single-disk test only - depends on lint
+- make aur - Build AUR packages only
+- make iso - Build ISO only (skip AUR if already built)
+- make deploy - Copy ISO to truenas.local AND Ventoy drive (if inserted)
+- make all - Full build + tests
+
+*** Benefits
+- Familiar interface for developers
+- Dependency tracking (rebuild only what changed)
+- Parallel execution where possible
+- Self-documenting (make help)
+- Easy CI/CD integration
+
+*** Considerations
+- Keep build.sh as the underlying implementation
+- Makefile calls build.sh with appropriate flags
+- Or refactor build.sh logic into Makefile directly
+
 ** TODO [#B] Add Docker/Podman container support for builds
 Idea from: https://github.com/stevleibelt/arch-linux-live-cd-iso-with-zfs