Conversation
…dex-compatible workflows Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Up to standards ✅🟢 Issues
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new AGENTS.md file and updates CLAUDE.md to reflect the completion of the MVP and Authentication phases. The documentation now includes comprehensive details on the project architecture, API endpoints, security requirements using Argon2, and the RPI development workflow. Review feedback identifies several inconsistencies that need to be addressed: a discrepancy in the reported number of frontend pages and technology stack (Vanilla JS vs. React), stale references to 'bcrypt' in source code docstrings, and outdated workflow skill references in CLAUDE.md that were not synchronized with the new documentation standards.
| - Frontend: 100% (11 pages — dashboard, login, admin, team members, shifts, schedule, schedule overrides, notifications, help, change password, style guide) | ||
| - Auth: Shipped (Argon2 password hashing, login/logout, role-based access, first-boot admin seeding) | ||
| - Testing: 288 tests, 85% coverage | ||
| - Deployment: Docker on Portainer (production), Cloudflare tunnel + Zero Trust | ||
|
|
||
| **Next Phase:** Versioning process, Docker offline installer, PostgreSQL migration | ||
|
|
||
| **Work From:** | ||
|
|
||
| - **Issues:** DocVault vault at `/Volumes/DATA/GitHub/DocVault/Projects/WhoseOnFirst/Issues/`, prefix `WHO` | ||
| - **Requirements:** `/docs/planning/PRD.md` (living document) | ||
| - **Code Patterns:** `/docs/reference/code-patterns.md` | ||
|
|
||
| --- | ||
|
|
||
| ## DocVault — Project Documentation | ||
|
|
||
| Technical documentation lives in **DocVault** at `/Volumes/DATA/GitHub/DocVault/Projects/WhoseOnFirst/`. Read relevant pages before discussing architecture or planning changes. | ||
|
|
||
| Key pages: `Overview.md`, Issues directory. | ||
|
|
||
| ``` | ||
| Read /Volumes/DATA/GitHub/DocVault/Projects/WhoseOnFirst/Overview.md | ||
| ``` | ||
|
|
||
| When making changes that affect documented behavior, update the relevant DocVault page at `/Volumes/DATA/GitHub/DocVault/Projects/WhoseOnFirst/` before pushing. | ||
|
|
||
| ## Documentation Hierarchy | ||
|
|
||
| ```text | ||
| DocVault (Primary Docs) + mem0 (Session Context) | ||
| ↓ references | ||
| /docs/planning/PRD.md (Living Requirements) | ||
| ↓ informs | ||
| AGENTS.md (This File - AI Context) | ||
| ↓ generates | ||
| CHANGELOG.md (Version History) | ||
| ↓ summarizes | ||
| README.md (User-Facing) | ||
| ``` | ||
|
|
||
| **Full Documentation Guide:** `/docs/DOCUMENTATION_GUIDE.md` | ||
|
|
||
| --- | ||
|
|
||
| ## Project Architecture | ||
|
|
||
| ### Core Principles | ||
|
|
||
| **Layered Architecture:** | ||
|
|
||
| ```text | ||
| API Layer (FastAPI routes) | ||
| ↓ | ||
| Service Layer (Business logic) | ||
| ↓ | ||
| Repository Layer (Data access) | ||
| ↓ | ||
| Database (SQLite → PostgreSQL path) | ||
| ``` | ||
|
|
||
| **Background Jobs:** | ||
|
|
||
| - APScheduler with America/Chicago timezone | ||
| - Daily SMS at 8:00 AM CST | ||
| - Auto-renewal check at 2:00 AM CST | ||
|
|
||
| **Circular Rotation Algorithm:** | ||
|
|
||
| - Simple modulo-based rotation: `member_index = shifts_elapsed % team_size` | ||
| - Works with any team size (1 to 15+ members) | ||
| - 100% test coverage (30 tests in `tests/test_rotation_algorithm.py`) | ||
|
|
||
| **Full Architecture:** `/docs/planning/architecture.md` | ||
|
|
||
| --- | ||
|
|
||
| ## Technology Stack | ||
|
|
||
| **Backend:** FastAPI 0.115+, SQLAlchemy 2.0+, APScheduler 3.10+, Twilio SDK 9.2+, Uvicorn 0.30+ | ||
|
|
||
| **Database:** Phase 1: SQLite (`./data/whoseonfirst.db`), Phase 2+: PostgreSQL migration path | ||
|
|
||
| **Frontend:** Tabler.io 1.0.0-beta20 (Bootstrap 5), Vanilla JavaScript, 16-color WCAG AA team member system |
There was a problem hiding this comment.
There are two discrepancies regarding the frontend implementation:
- Page Count: The documentation lists 11 pages, but the
route_mapinsrc/main.py(lines 210-220) only explicitly defines 9 routes. Specifically,adminandstyle guideare missing from the map. - Technology Stack: Line 101 describes the frontend as "Vanilla JavaScript", but
src/main.py(line 106) includes a CORS origin for a "React dev server" at port 3000.
Please clarify the actual number of pages and whether the project uses React or Vanilla JS to ensure the AI agent has accurate context.
|
|
||
| - Service credentials (Twilio, SECRET_KEY) in `.env` / Portainer stack env vars (never commit) | ||
| - **First-boot admin seeding:** `src/main.py` lifespan hook creates `admin/Admin123!` when `users` table is empty (idempotent). This is a code-resident credential — change it after first login. | ||
| - Auth uses Argon2-cffi (OWASP 2025 recommended) for password hashing |
There was a problem hiding this comment.
The documentation correctly identifies Argon2-cffi as the hashing algorithm (per the PR description), but this contradicts the stale docstrings in the source code. Specifically, src/models/user.py (line 5) and src/repositories/user_repository.py (line 104) still explicitly reference "bcrypt". These docstrings should be updated to match the current implementation and this documentation.
| **Project:** WhoseOnFirst - Automated on-call rotation and SMS notification system for 7-person technical team | ||
|
|
||
| **Current Status:** Phase 1 MVP Complete | ||
| **Current Status:** Phase 1 MVP + Auth Complete |
There was a problem hiding this comment.
The PR description states that workflow references were replaced with direct paths in CLAUDE.md to match AGENTS.md. However, while AGENTS.md was updated (lines 27, 43, 220), CLAUDE.md still contains stale references to the issue skill and the /vault-update command (visible in the file context at lines 27, 43, and 220). Please ensure these lines in CLAUDE.md are synchronized with the changes made to AGENTS.md.
| - All credentials in `.env` file (never commit) | ||
| - Service credentials (Twilio, SECRET_KEY) in `.env` / Portainer stack env vars (never commit) | ||
| - **First-boot admin seeding:** `src/main.py` lifespan hook creates `admin/Admin123!` when `users` table is empty (idempotent). This is a code-resident credential — change it after first login. | ||
| - Auth uses Argon2-cffi (OWASP 2025 recommended) for password hashing |
There was a problem hiding this comment.
As noted in AGENTS.md, there is a discrepancy between this documentation (Argon2) and the source code docstrings in src/models/user.py and src/repositories/user_repository.py (which still mention bcrypt). The code docstrings should be updated for consistency.
|
Closing as stale. The local working tree on main has a newer AGENTS.md that already incorporates these fixes plus more — auth shipped (Phase 1 MVP + Auth Complete), 11 pages instead of 8, full router list including /auth/, /admin/, /schedule-overrides/, /notifications/, the users table, and Portainer/Cloudflare deployment context. Replacement PR coming with the current local AGENTS.md and a new GEMINI.md. The CLAUDE.md changes from this PR are superseded by #15 (SWF-63 _Index.md pointer). |
Adds Codex (AGENTS.md) and Gemini CLI (GEMINI.md) peer-agent instruction files. AGENTS.md reflects current production state: Phase 1 MVP + Auth Complete, 11 frontend pages, full API surface (auth, admin, team-members, shifts, schedules, schedule-overrides, notifications, settings), users table, Portainer/Cloudflare deployment. Supersedes the stale AGENTS.md proposed in #14 (closed). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Updates repository agent/instruction docs to reflect the project’s post-auth state and provide Codex/Claude guidance consistent with the current backend/frontend surface.
Changes:
- Refreshes project status/phase, deployment notes, and security/auth descriptions.
- Expands documented API endpoints and database schema notes.
- Adds a new
AGENTS.mdwith workflow, architecture, and operational guidance.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 9 comments.
| File | Description |
|---|---|
| CLAUDE.md | Updates status/phase, core tables list, API surface, and security notes for Claude Code guidance |
| AGENTS.md | Adds a comprehensive Codex agent guide (workflow, stack, schema, API, security, constraints) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ### Database Schema | ||
|
|
||
| **Core Tables:** `team_members`, `shifts`, `schedule`, `notification_log`, `settings` | ||
| **Core Tables:** `team_members`, `shifts`, `schedule`, `notification_log`, `settings`, `users` |
There was a problem hiding this comment.
The listed core tables omit schedule_overrides, but the codebase includes a schedule_overrides table/model (src/models/schedule_override.py). Please add it here (or clarify why it’s not considered a core table).
| **Core Tables:** `team_members`, `shifts`, `schedule`, `notification_log`, `settings`, `users` | |
| **Core Tables:** `team_members`, `shifts`, `schedule`, `schedule_overrides`, `notification_log`, `settings`, `users` |
|
|
||
| All APIs under `/api/v1/` prefix: | ||
|
|
||
| - `/auth/` - Login, logout, token refresh, password change |
There was a problem hiding this comment.
/auth/ is described as including “token refresh”, but the current auth router exposes login/logout/me/change-password and an admin-only reset-viewer-password endpoint (cookie-based session), with no token refresh route. Please update this bullet to match the actual endpoints.
| - `/auth/` - Login, logout, token refresh, password change | |
| - `/auth/` - Cookie-based session auth: login, logout, `me`, change-password, admin-only reset-viewer-password |
| All APIs under `/api/v1/` prefix: | ||
|
|
||
| - `/auth/` - Login, logout, token refresh, password change | ||
| - `/admin/` - User management (create, list, delete users, reset passwords) |
There was a problem hiding this comment.
/admin/ is described as user management, but the implemented admin router is for export/import backup restore (GET /api/v1/admin/export, POST /api/v1/admin/import). Please update this description so agents don’t assume user CRUD endpoints exist.
| - `/admin/` - User management (create, list, delete users, reset passwords) | |
| - `/admin/` - Administrative backup operations: export data and import/restore backups |
| **Current Status:** Phase 1 MVP + Auth Complete | ||
|
|
||
| - Backend: 100% (FastAPI/APScheduler/Twilio) | ||
| - Frontend: 100% (8 pages with live data) | ||
| - Frontend: 100% (11 pages — dashboard, login, admin, team members, shifts, schedule, schedule overrides, notifications, help, change password, style guide) | ||
| - Auth: Shipped (Argon2 password hashing, login/logout, role-based access, first-boot admin seeding) | ||
| - Testing: 288 tests, 85% coverage | ||
| - Deployment: Docker containerized | ||
| - Deployment: Docker on Portainer (production), Cloudflare tunnel + Zero Trust | ||
|
|
||
| **Blocker:** Twilio US number approval (~1 week wait) | ||
|
|
||
| **Next Phase:** Docker offline installer + Authentication system | ||
| **Next Phase:** Versioning process, Docker offline installer, PostgreSQL migration |
There was a problem hiding this comment.
PR description says CLAUDE.md workflow references were updated to remove the issue skill and /vault-update, but CLAUDE.md still contains those references (e.g., around lines 27 and 43). Please either update CLAUDE.md to match the PR description or adjust the PR description accordingly.
|
|
||
| ### Database Schema | ||
|
|
||
| **Core Tables:** `team_members`, `shifts`, `schedule`, `notification_log`, `settings`, `users` |
There was a problem hiding this comment.
The listed core tables omit schedule_overrides, but the codebase includes a schedule_overrides table/model (src/models/schedule_override.py). Please add it here (or clarify why it’s not considered a core table).
| **Core Tables:** `team_members`, `shifts`, `schedule`, `notification_log`, `settings`, `users` | |
| **Core Tables:** `team_members`, `shifts`, `schedule`, `schedule_overrides`, `notification_log`, `settings`, `users` |
|
|
||
| All APIs under `/api/v1/` prefix: | ||
|
|
||
| - `/auth/` - Login, logout, token refresh, password change |
There was a problem hiding this comment.
/auth/ is described as including “token refresh”, but the current auth router exposes login/logout/me/change-password and an admin-only reset-viewer-password endpoint (cookie-based session), with no token refresh route. Please update this bullet to match the actual endpoints.
| - `/auth/` - Login, logout, token refresh, password change | |
| - `/auth/` - Login, logout, current-user (`/me`), password change, admin-only reset-viewer-password (cookie-based session auth; no token refresh route) |
| All APIs under `/api/v1/` prefix: | ||
|
|
||
| - `/auth/` - Login, logout, token refresh, password change | ||
| - `/admin/` - User management (create, list, delete users, reset passwords) |
There was a problem hiding this comment.
/admin/ is described as user management, but the implemented admin router is for export/import backup restore (GET /api/v1/admin/export, POST /api/v1/admin/import). Please update this description so agents don’t assume user CRUD endpoints exist.
| - `/admin/` - User management (create, list, delete users, reset passwords) | |
| - `/admin/` - Backup administration: export and import/restore operations |
|
|
||
| **Database:** Phase 1: SQLite (`./data/whoseonfirst.db`), Phase 2+: PostgreSQL migration path | ||
|
|
||
| **Frontend:** Tabler.io 1.0.0-beta20 (Bootstrap 5), Vanilla JavaScript, 16-color WCAG AA team member system |
There was a problem hiding this comment.
This claims Tabler.io is pinned to 1.0.0-beta20, but the frontend HTML loads Tabler via the CDN @tabler/core@latest (not a pinned beta version). Please update this to reflect the actual dependency strategy (or pin the CDN URL if the intention is to stay on a specific version).
| **Frontend:** Tabler.io 1.0.0-beta20 (Bootstrap 5), Vanilla JavaScript, 16-color WCAG AA team member system | |
| **Frontend:** Tabler.io via CDN (`@tabler/core@latest`, Bootstrap 5), Vanilla JavaScript, 16-color WCAG AA team member system |
|
|
||
| - Simple modulo-based rotation: `member_index = shifts_elapsed % team_size` | ||
| - Works with any team size (1 to 15+ members) | ||
| - 100% test coverage (30 tests in `tests/test_rotation_algorithm.py`) |
There was a problem hiding this comment.
This references tests/test_rotation_algorithm.py, but the rotation algorithm tests live under tests/services/test_rotation_algorithm.py in this repo. Please update the path (and ideally avoid hardcoding the exact test count unless it’s kept in sync).
| - 100% test coverage (30 tests in `tests/test_rotation_algorithm.py`) | |
| - 100% test coverage (see `tests/services/test_rotation_algorithm.py`) |
GSD Session — AGENTS.md & CLAUDE.md Refresh
Codex adversarial review flagged 3 issues with the new
AGENTS.md— all stemming from stale content that predates auth shipping. Applied the same fixes toCLAUDE.mdwhich had identical stale sections.Changes
auth,admin,team-members,shifts,schedules,schedule-overrides,notifications,settings) plus utility endpointsissueskill and/vault-updatereferences with direct paths Codex can useuserstableNotes
🤖 Generated with Claude Code