Skip to content

feat: AUTH_MODE support, anonymous suggestions, and sign-in-to-edit UX#57

Open
damienriehl wants to merge 26 commits intodevfrom
feat/optional-auth
Open

feat: AUTH_MODE support, anonymous suggestions, and sign-in-to-edit UX#57
damienriehl wants to merge 26 commits intodevfrom
feat/optional-auth

Conversation

@damienriehl
Copy link
Copy Markdown
Contributor

@damienriehl damienriehl commented Apr 3, 2026

Summary

AUTH_MODE (Phase 8)

  • AUTH_MODE=required|optional|disabled support across the frontend
  • lib/auth-mode.ts — centralized helpers (getAuthMode, isZitadelConfigured, isAuthRequired)
  • Conditional Zitadel provider in NextAuth, env validation, NEXTAUTH_SECRET fallback
  • Header sign-in UI hidden when Zitadel not configured
  • "Sign in to edit" buttons with callbackUrl for return-to-same-page

Anonymous Suggestions (Phase 10)

  • Anonymous API client + useAnonymousSuggestion hook + localStorage token/credit stores
  • "Propose Edit" button on ClassDetailPanel for anonymous visitors on public projects
  • "Want credit for your suggestions?" modal with optional name/email (remembered in localStorage)
  • "Sign in for full editing" link alongside Propose Edit when Zitadel configured
  • Editor page wiring for anonymous proposal mode (batch edits, submit, discard)
  • Review page displays submitter name/email or "Anonymous" badge

GSD Planning Artifacts

  • .planning/ directory with PROJECT.md, ROADMAP.md, REQUIREMENTS.md, STATE.md
  • Phase plans, summaries, and context for phases 7-10

Test plan

  • npx tsc --noEmit — type check passes
  • npx vitest run — 95 tests pass
  • AUTH_MODE=optional + no Zitadel → anonymous browsing, no sign-in button, "Propose Edit" visible
  • Click "Propose Edit" → edit mode activates → submit shows credit modal
  • AUTH_MODE=required → unchanged behavior

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Optional auth modes (required/optional/disabled) with anonymous proposal flow and “Propose Edit” for unauthenticated users.
    • Anonymous suggestion sessions: create/save/submit/discard flows, credit modal, persisted anonymous tokens, and client hook/store support.
  • Improvements

    • Conditional auth UI: sign-in CTAs, user menu, and header actions respect runtime auth configuration; signIn callbacks restored to return users to the same page.
  • Documentation

    • Added roadmap, milestones, requirements, deployment and phase planning for v0.3.0 (including anonymous-suggestions and optional-auth).

damienriehl and others added 15 commits April 3, 2026 09:58
Replaces AUTH_DISABLED boolean with three-mode AUTH_MODE enum.
Key change: optional mode allows anonymous browsing with sign-in
available when Zitadel is configured (progressive disclosure UX).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Anonymous users see "Sign in to edit" buttons in editor header and
ClassDetailPanel when Zitadel is configured. Clicking triggers auth
flow with callbackUrl to return to same page. Replaces the "hide
everything" approach with progressive disclosure UX.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Add lib/auth-mode.ts with getAuthMode(), isZitadelConfigured(), isAuthRequired()
- Conditionally skip Zitadel env validation when auth mode is not required
- AUTH_MODE=optional|disabled makes all Zitadel vars optional with sensible defaults
- Import getAuthMode and isZitadelConfigured from auth-mode helper
- Default NEXTAUTH_SECRET when auth mode is not required
- Conditionally include Zitadel provider (empty array when disabled or unconfigured)
- Skip token refresh when Zitadel is not configured
- Conditional custom sign-in pages only in required mode
…ability

- Expose NEXT_PUBLIC_AUTH_MODE and NEXT_PUBLIC_ZITADEL_CONFIGURED via next.config.ts
- Header: compute showAuthUI from auth mode + Zitadel config; hide NotificationBell and UserMenu when no auth
- UserMenu: return null when showAuthUI=false and no active session (graceful degradation)
- AUTH_MODE=disabled or optional without Zitadel → no sign-in UI shown
… ontokit-api

- Create 08-01-SUMMARY.md documenting AUTH_MODE implementation
- Update STATE.md: advance plan, record metrics and decisions, update session
- Update ROADMAP.md: phase 8 plan progress (1/2 summaries)
- Update REQUIREMENTS.md: mark AUTH-01, AUTH-04, AUTH-05 complete
- EditorPage: add zitadelConfigured flag, wrap sign-in CTAs in Zitadel check
- EditorPage: all signIn() calls now pass callbackUrl for post-auth redirect
- EditorPage: thread showSignInToEdit/onSignInToEdit to both layout components
- DeveloperEditorLayout: accept + forward showSignInToEdit/onSignInToEdit to ClassDetailPanel
- StandardEditorLayout: accept + forward showSignInToEdit/onSignInToEdit to ClassDetailPanel
- ClassDetailPanel: add showSignInToEdit/onSignInToEdit props, render Sign in to edit button when anonymous + Zitadel configured
- ClassDetailPanel: add LogIn icon import
Sequential deployment: install Postgres → switch repos to main →
configure env + migrate → start services + verify.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…lining

Module-level constants from process.env get evaluated at next.config.ts
load time, before .env.local is parsed. Moving inside the component
ensures the NEXT_PUBLIC_ vars are available when the check runs.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 3, 2026

📝 Walkthrough

Walkthrough

Adds runtime AUTH_MODE ("required" | "optional" | "disabled") and conditional Zitadel integration across frontend and backend; implements anonymous suggestion sessions (client-side stores/hooks, API endpoints, HMAC anonymous tokens, model/schema changes); and adds extensive planning/ops documentation and deployment plans.

Changes

Cohort / File(s) Summary
Planning & Roadmap
\.planning/MILESTONES.md, \.planning/PROJECT.md, \.planning/REQUIREMENTS.md, \.planning/ROADMAP.md, \.planning/STATE.md, \.planning/config.json, \.planning/phases/..., \.planning/quick/...
Added comprehensive planning, phase plans (optional-auth, production deploy, anonymous suggestions), state/progress tracking, quick-task sync steps, and config to disable auto-chaining.
Backend: Optional Auth
ontokit/core/config.py, ontokit/core/auth.py, .planning/phases/08-optional-auth/08-01-PLAN.md
Introduced auth_mode setting; added ANONYMOUS_USER and early-return behavior in get_current_user* helpers for disabled mode; added unit tests plan.
Backend: Anonymous Suggestions
ontokit/core/anonymous_token.py, ontokit/models/suggestion_session.py, ontokit/schemas/anonymous_suggestion.py, ontokit/schemas/suggestion.py, ontokit/api/routes/anonymous_suggestions.py, ontokit/services/suggestion_service.py, .planning/phases/10-anonymous-suggestions/*
Added HMAC anonymous tokens, extended SuggestionSession model (is_anonymous, submitter_name/email, client_ip), new anonymous endpoints (create/save/submit/discard/beacon) with rate limiting and honeypot, and corresponding service methods and schemas.
Frontend: Auth-mode helpers & env
lib/auth-mode.ts, lib/env.ts, next.config.ts
New AuthMode helpers (getAuthMode, isZitadelConfigured, isAuthRequired); conditional server env validation when auth not required; expose NEXT_PUBLIC_AUTH_MODE and NEXT_PUBLIC_ZITADEL_CONFIGURED.
NextAuth / Auth config
auth.ts
Conditionally include Zitadel provider/pages based on auth mode and Zitadel config; fallback NEXTAUTH_SECRET for non-required modes; skip JWT refresh when Zitadel unconfigured.
Frontend: Anonymous suggestion client & stores
lib/api/suggestions.ts, lib/hooks/useAnonymousSuggestion.ts, lib/stores/anonymousCreditStore.ts
Added anonymousSuggestionsApi and types; implemented useAnonymousSuggestion hook for lifecycle; added persisted localStorage stores for anonymous tokens and credit info.
Frontend: UI components & wiring
app/projects/[id]/editor/page.tsx, components/editor/ClassDetailPanel.tsx, components/editor/developer/DeveloperEditorLayout.tsx, components/editor/standard/StandardEditorLayout.tsx, components/layout/header.tsx, components/auth/user-menu.tsx, components/suggestions/CreditModal.tsx, app/projects/[id]/suggestions/review/page.tsx
Threaded anonymous-propose props through editor layouts and ClassDetailPanel (new props and buttons), added CreditModal, wired anonymous flows in editor page (useAnonymousSuggestion, anonymous proposal mode), conditional header/user-menu rendering based on auth-mode/zitadel config, and updated review UI to show "Anonymous" badge.

Sequence Diagram(s)

sequenceDiagram
  participant Client as Client (browser)
  participant Frontend as OntoKit Web
  participant AuthCfg as Auth Mode / NextAuth
  participant API as OntoKit API
  participant DB as Database / Git service

  Client->>Frontend: User clicks "Propose Edit"
  Frontend->>AuthCfg: getAuthMode()/isZitadelConfigured()
  alt AUTH_MODE != "required" (anonymous allowed)
    Frontend->>API: POST /anonymous_suggestions/create
    API->>API: rate-limit by client IP
    API->>DB: create SuggestionSession (is_anonymous=true), create branch/commit
    API->>API: create_anonymous_token(session_id) (HMAC token)
    API-->>Frontend: { session_id, branch, anonymous_token }
    Frontend->>LocalStorage: persist anonymous_token/session
    Frontend->>API: POST /anonymous_suggestions/save (X-Anonymous-Token)
    API->>API: verify_anonymous_token -> session_id
    API->>DB: update session branch/commit
    Client->>Frontend: User submits -> POST /anonymous_suggestions/submit (X-Anonymous-Token)
    API->>API: verify token, honeypot check
    API->>DB: create PR, record submitter_name/email
    API-->>Frontend: { pr_number, pr_url, status }
  else AUTH_MODE == "required"
    Frontend->>AuthCfg: show Sign-in CTA -> redirect to Zitadel sign-in (callbackUrl)
  end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Suggested labels

enhancement, UX

Suggested reviewers

  • JohnRDOrazio

Poem

🐰 A token hops on a secret string,
Propose a change, let the tiny bell ring.
Flags flip, buttons hide, anon voices say,
"I suggested a fix—keep my name at bay."
Hop, patch, and push — a carrot-shaped hooray!

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 44.44% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'feat: AUTH_MODE support, anonymous suggestions, and sign-in-to-edit UX' accurately summarizes the primary changes: AUTH_MODE environment configuration, anonymous suggestion workflows, and sign-in UI affordances throughout the editor.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/optional-auth

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 12

🧹 Nitpick comments (1)
components/auth/user-menu.tsx (1)

12-15: Consider moving auth mode flags inside the component for consistency.

These flags are computed at module level, but in header.tsx (lines 20-23), the same logic is computed inside the Header() component with a comment stating it "must be inside component for correct client-side hydration." The PR description also mentions a fix to "move showAuthUI check inside component to avoid build-time inlining issues."

For consistency and to avoid potential hydration mismatches, consider moving these inside UserMenu():

♻️ Suggested refactor
-// Auth mode flags — set at build time by next.config.ts
-const authMode = process.env.NEXT_PUBLIC_AUTH_MODE || "required";
-const zitadelConfigured = process.env.NEXT_PUBLIC_ZITADEL_CONFIGURED === "true";
-const showAuthUI = authMode === "required" || (authMode === "optional" && zitadelConfigured);
-
 export function UserMenu() {
+  // Auth mode flags — must be inside component for correct client-side hydration
+  const authMode = process.env.NEXT_PUBLIC_AUTH_MODE || "required";
+  const zitadelConfigured = process.env.NEXT_PUBLIC_ZITADEL_CONFIGURED === "true";
+  const showAuthUI = authMode === "required" || (authMode === "optional" && zitadelConfigured);
+
   const { data: session, status } = useSession();
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@components/auth/user-menu.tsx` around lines 12 - 15, The module-level flags
authMode, zitadelConfigured, and showAuthUI should be moved into the UserMenu()
component to avoid build-time inlining and hydration mismatches; update the
UserMenu function to compute authMode = process.env.NEXT_PUBLIC_AUTH_MODE ||
"required", zitadelConfigured = process.env.NEXT_PUBLIC_ZITADEL_CONFIGURED ===
"true", and showAuthUI = authMode === "required" || (authMode === "optional" &&
zitadelConfigured") inside the component body and use that local showAuthUI in
the component render logic (mirroring the approach used in Header()).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.planning/phases/08-optional-auth/08-01-PLAN.md:
- Around line 127-142: Replace hardcoded 'python -c "' invocations in the
automated verification command with the repo-standard invocation (e.g., 'uv run
python' or 'python3') for reproducible CI/local runs; update every occurrence of
the command string shown in the diff (the automated block that starts with cd
... && python -c "...") and similarly update the other occurrences referenced
(around the other checks) so the script uses 'uv run python -c' or 'python3 -c'
instead of plain 'python -c'.
- Around line 215-216: Remove the unmatched trailing XML/HTML tags at the end of
the plan by deleting the stray closing tags `</content>` and `</invoke>` present
at EOF (they are the unmatched tokens causing parse failures); ensure the file
ends with the intended final section or a single closing tag if one was
intended, then run the plan parser/validator to confirm no remaining unmatched
tags.

In @.planning/phases/08-optional-auth/08-02-PLAN.md:
- Line 130: The fenced code block ending with ``` on the shown line lacks a
language identifier, triggering markdownlint MD040; update the opening fence for
that code block (the ``` fence paired with the shown closing fence) to include
the appropriate language tag (for example ```json, ```bash, or ```text) so the
block is language-identified and MD040 is resolved.
- Around line 250-253: The client-side NEXT_PUBLIC_ZITADEL_CONFIGURED flag in
next.config.ts is currently derived only from ZITADEL_ISSUER, which can wrongly
indicate a configured provider; update the env export in next.config.ts (or
next.config.mjs) so NEXT_PUBLIC_ZITADEL_CONFIGURED is true only when both
process.env.ZITADEL_ISSUER and process.env.ZITADEL_CLIENT_ID are present (e.g.,
set NEXT_PUBLIC_ZITADEL_CONFIGURED to a conditional expression that checks both
variables and returns "true" or "false"), and keep NEXT_PUBLIC_AUTH_MODE logic
unchanged.

In @.planning/phases/09-production-deployment/09-01-PLAN.md:
- Line 213: The markdown file contains an unlabeled fenced code block (a
triple-backtick fence) on the flagged line; update the opening fence from ``` to
include an explicit language identifier (for example ```bash, ```sh, ```yaml, or
```text as appropriate for the block's content) so the fence is labeled and
markdownlint MD040 is satisfied.
- Around line 84-90: The deployment plan currently hardcodes the DB password in
the psql commands (e.g., CREATE USER ontokit WITH PASSWORD 'ontokit-folio-prod';
and PGPASSWORD='ontokit-folio-prod' psql ...), so replace the literal password
with a placeholder and document secure injection at runtime: change the CREATE
USER/GRANT commands and the verification command to use a variable like
'${DB_PASSWORD}' or rely on PGPASSWORD provided from the environment/secret
manager, and update the instructions to show how to export or inject DB_PASSWORD
(or use a secret-vault/CI variable) before running the ssh/psql commands so
credentials are never committed; apply this change to every occurrence of the
quoted password in the file.

In @.planning/REQUIREMENTS.md:
- Around line 15-19: Update the requirements checklist and AUTH traceability to
use the new AUTH_MODE configuration instead of the obsolete AUTH_DISABLED:
change the items currently labeled AUTH-01..AUTH-05 to reference AUTH_MODE with
the allowed values (required|optional|disabled) and rewrite each requirement
description to map to those modes (e.g., "App runs without Zitadel when
AUTH_MODE=disabled", "Unauthenticated users can browse... when
AUTH_MODE=disabled or optional", "Edit/commit/PR features are hidden when
AUTH_MODE=disabled", "Existing Zitadel-based auth continues when
AUTH_MODE=required or optional", "API accepts requests without Bearer tokens
when AUTH_MODE=disabled or optional"). Also update the AUTH traceability section
(the block referencing AUTH-01..AUTH-05) to use AUTH_MODE semantics and ensure
requirement identifiers and wording reflect AUTH_MODE instead of AUTH_DISABLED.

In @.planning/ROADMAP.md:
- Around line 21-23: Update the roadmap to reference AUTH_MODE instead of the
obsolete AUTH_DISABLED flag: replace any occurrences of "AUTH_DISABLED" with
"AUTH_MODE" and describe the accepted values (required|optional|disabled) and
intended operator actions (e.g., set AUTH_MODE=disabled to turn auth off) so
rollout steps and validation guidance match the implementation and test matrix;
update all instances noted (including the checklist entry and the other
occurrences mentioned) and ensure the Phase 8 line and nearby instructions use
AUTH_MODE consistently.

In @.planning/STATE.md:
- Around line 2-7: The front-matter and decision metadata are out of sync with
the current Phase 8 of the v0.3.0 deployment; update the front-matter keys
(milestone -> "v0.3.0", milestone_name -> a clear name like "deployment v0.3.0",
last_updated -> current ISO timestamp) and remove or replace the stale
AUTH_DISABLED state in the decision log so the state reflects the Phase 8 auth
decision; also audit and correct the corresponding sections previously noted
(the blocks around the decision log and milestone text) so all occurrences of
v0.2/AUTH_DISABLED are replaced with the v0.3.0 phase and current auth status.

In `@app/projects/`[id]/editor/page.tsx:
- Around line 67-68: The UI renders Zitadel sign-in CTAs based only on the const
zitadelConfigured; update the render gates so they also check that auth mode is
not disabled (e.g. change conditions to zitadelConfigured &&
process.env.NEXT_PUBLIC_AUTH_MODE !== "disabled") wherever those Zitadel buttons
are conditionally rendered in this file (the places that currently use
zitadelConfigured for showing sign-in CTAs), or alternatively add a shared
helper isAuthEnabled() in lib/auth-mode.ts and replace uses of zitadelConfigured
with zitadelConfigured && isAuthEnabled() to match other components that check
NEXT_PUBLIC_AUTH_MODE.

In `@auth.ts`:
- Around line 6-9: Remove the hard-coded shared NEXTAUTH_SECRET fallback and
require an explicit secret whenever auth can issue sessions: stop setting
process.env.NEXTAUTH_SECRET = "ontokit-optional-auth-secret" in the
getAuthMode() branch; instead, if getAuthMode() indicates auth may issue
sessions and process.env.NEXTAUTH_SECRET is missing, throw/log a clear error and
exit (or surface a startup failure) unless running in an isolated dev/test
environment (check NODE_ENV === "development" or "test" or use the existing dev
helper); if you must provide a fallback for dev/test only, reuse the single
canonical dev fallback defined in lib/env.ts (or centralize that value) so both
entry points match.

In `@lib/env.ts`:
- Around line 22-35: The validateServerEnv function currently casts the parsed
result to ServerEnv despite using a conditional schema (serverSchema vs
optional-zitadel schema); remove the unsafe "as ServerEnv" cast and change the
function's return type to reflect the conditional shape (e.g., ServerEnv |
Partial<ServerEnv> or a new AuthOptionalServerEnv type) so TypeScript matches
the actual zod schema; update any callers to handle optional Zitadel fields or,
if serverEnv is unused, remove its export instead. Ensure to reference
validateServerEnv, serverSchema, and the local schema variable when making the
change.

---

Nitpick comments:
In `@components/auth/user-menu.tsx`:
- Around line 12-15: The module-level flags authMode, zitadelConfigured, and
showAuthUI should be moved into the UserMenu() component to avoid build-time
inlining and hydration mismatches; update the UserMenu function to compute
authMode = process.env.NEXT_PUBLIC_AUTH_MODE || "required", zitadelConfigured =
process.env.NEXT_PUBLIC_ZITADEL_CONFIGURED === "true", and showAuthUI = authMode
=== "required" || (authMode === "optional" && zitadelConfigured") inside the
component body and use that local showAuthUI in the component render logic
(mirroring the approach used in Header()).
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 124726de-1f9e-4931-8810-9e0dd770705f

📥 Commits

Reviewing files that changed from the base of the PR and between 80adae2 and 8026c4a.

📒 Files selected for processing (23)
  • .planning/MILESTONES.md
  • .planning/PROJECT.md
  • .planning/REQUIREMENTS.md
  • .planning/ROADMAP.md
  • .planning/STATE.md
  • .planning/config.json
  • .planning/phases/08-optional-auth/08-01-PLAN.md
  • .planning/phases/08-optional-auth/08-01-SUMMARY.md
  • .planning/phases/08-optional-auth/08-02-PLAN.md
  • .planning/phases/08-optional-auth/08-02-SUMMARY.md
  • .planning/phases/09-production-deployment/09-01-PLAN.md
  • .planning/quick/260403-dth-phase-7-sync-alea-forks-with-catholicos-/260403-dth-PLAN.md
  • .planning/quick/260403-dth-phase-7-sync-alea-forks-with-catholicos-/260403-dth-SUMMARY.md
  • app/projects/[id]/editor/page.tsx
  • auth.ts
  • components/auth/user-menu.tsx
  • components/editor/ClassDetailPanel.tsx
  • components/editor/developer/DeveloperEditorLayout.tsx
  • components/editor/standard/StandardEditorLayout.tsx
  • components/layout/header.tsx
  • lib/auth-mode.ts
  • lib/env.ts
  • next.config.ts

Comment on lines +127 to +142
<automated>cd "/home/damienriehl/Coding Projects/ontokit-api" && python -c "
from ontokit.core.config import Settings
s = Settings(auth_mode='optional')
assert s.auth_mode == 'optional'
s2 = Settings()
assert s2.auth_mode == 'required'
s3 = Settings(auth_mode='disabled')
assert s3.auth_mode == 'disabled'
print('Config OK')

from ontokit.core.auth import ANONYMOUS_USER
assert ANONYMOUS_USER.id == 'anonymous'
assert ANONYMOUS_USER.roles == ['viewer']
print('Anonymous user OK')
print('ALL CHECKS PASSED')
"</automated>
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Use repo-standard Python invocation in verification commands.

These checks assume python is available. This plan’s own phase summary records environments where python is missing. Use uv run python (or python3) for reproducibility.

🔧 Proposed fix
-<automated>cd ".../ontokit-api" && python -c "
+<automated>cd ".../ontokit-api" && uv run python -c "

-1. Run tests: `cd ".../ontokit-api" && python -m pytest tests/unit/test_auth_disabled.py -v`
+1. Run tests: `cd ".../ontokit-api" && uv run python -m pytest tests/unit/test_auth_disabled.py -v`

-2. Verify config default: `python -c "from ontokit.core.config import Settings; assert Settings().auth_mode == 'required'"`
+2. Verify config default: `uv run python -c "from ontokit.core.config import Settings; assert Settings().auth_mode == 'required'"`

Also applies to: 200-201

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.planning/phases/08-optional-auth/08-01-PLAN.md around lines 127 - 142,
Replace hardcoded 'python -c "' invocations in the automated verification
command with the repo-standard invocation (e.g., 'uv run python' or 'python3')
for reproducible CI/local runs; update every occurrence of the command string
shown in the diff (the automated block that starts with cd ... && python -c
"...") and similarly update the other occurrences referenced (around the other
checks) so the script uses 'uv run python -c' or 'python3 -c' instead of plain
'python -c'.

Comment on lines +215 to +216
</content>
</invoke> No newline at end of file
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Remove stray closing tags at EOF.

Lines 215-216 contain unmatched </content> and </invoke> tags. If this plan is machine-parsed, these can break execution/parsing.

🔧 Proposed fix
-</content>
-</invoke>
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
</content>
</invoke>
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.planning/phases/08-optional-auth/08-01-PLAN.md around lines 215 - 216,
Remove the unmatched trailing XML/HTML tags at the end of the plan by deleting
the stray closing tags `</content>` and `</invoke>` present at EOF (they are the
unmatched tokens causing parse failures); ensure the file ends with the intended
final section or a single closing tag if one was intended, then run the plan
parser/validator to confirm no remaining unmatched tags.

export function isAuthRequired(): boolean {
return getAuthMode() === "required";
}
```
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Add a language identifier to the fenced code block.

Static analysis reports MD040 at Line 130. Please add a language tag for that fence to keep markdownlint clean.

🧰 Tools
🪛 markdownlint-cli2 (0.22.0)

[warning] 130-130: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.planning/phases/08-optional-auth/08-02-PLAN.md at line 130, The fenced code
block ending with ``` on the shown line lacks a language identifier, triggering
markdownlint MD040; update the opening fence for that code block (the ``` fence
paired with the shown closing fence) to include the appropriate language tag
(for example ```json, ```bash, or ```text) so the block is language-identified
and MD040 is resolved.

Comment on lines +250 to +253
1. In `next.config.ts` (or `next.config.mjs`), expose `AUTH_MODE` and a `ZITADEL_CONFIGURED` flag via `env`:
- Read the next.config file first to understand its current structure
- Add to the config: `env: { NEXT_PUBLIC_AUTH_MODE: process.env.AUTH_MODE || "required", NEXT_PUBLIC_ZITADEL_CONFIGURED: process.env.ZITADEL_ISSUER ? "true" : "false" }`

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Align NEXT_PUBLIC_ZITADEL_CONFIGURED with server-side Zitadel checks.

Line 252 derives the client flag from ZITADEL_ISSUER only, but server auth wiring requires both issuer and client ID. This can show sign-in affordances when no provider exists.

🔧 Proposed fix
-  - Add to the config: `env: { NEXT_PUBLIC_AUTH_MODE: process.env.AUTH_MODE || "required", NEXT_PUBLIC_ZITADEL_CONFIGURED: process.env.ZITADEL_ISSUER ? "true" : "false" }`
+  - Add to the config: `env: { NEXT_PUBLIC_AUTH_MODE: process.env.AUTH_MODE || "required", NEXT_PUBLIC_ZITADEL_CONFIGURED: process.env.ZITADEL_ISSUER && process.env.ZITADEL_CLIENT_ID ? "true" : "false" }`
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
1. In `next.config.ts` (or `next.config.mjs`), expose `AUTH_MODE` and a `ZITADEL_CONFIGURED` flag via `env`:
- Read the next.config file first to understand its current structure
- Add to the config: `env: { NEXT_PUBLIC_AUTH_MODE: process.env.AUTH_MODE || "required", NEXT_PUBLIC_ZITADEL_CONFIGURED: process.env.ZITADEL_ISSUER ? "true" : "false" }`
1. In `next.config.ts` (or `next.config.mjs`), expose `AUTH_MODE` and a `ZITADEL_CONFIGURED` flag via `env`:
- Read the next.config file first to understand its current structure
- Add to the config: `env: { NEXT_PUBLIC_AUTH_MODE: process.env.AUTH_MODE || "required", NEXT_PUBLIC_ZITADEL_CONFIGURED: process.env.ZITADEL_ISSUER && process.env.ZITADEL_CLIENT_ID ? "true" : "false" }`
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.planning/phases/08-optional-auth/08-02-PLAN.md around lines 250 - 253, The
client-side NEXT_PUBLIC_ZITADEL_CONFIGURED flag in next.config.ts is currently
derived only from ZITADEL_ISSUER, which can wrongly indicate a configured
provider; update the env export in next.config.ts (or next.config.mjs) so
NEXT_PUBLIC_ZITADEL_CONFIGURED is true only when both process.env.ZITADEL_ISSUER
and process.env.ZITADEL_CLIENT_ID are present (e.g., set
NEXT_PUBLIC_ZITADEL_CONFIGURED to a conditional expression that checks both
variables and returns "true" or "false"), and keep NEXT_PUBLIC_AUTH_MODE logic
unchanged.

Comment on lines +84 to +90
ssh ... "sudo -u postgres psql -c \"CREATE USER ontokit WITH PASSWORD 'ontokit-folio-prod';\" 2>/dev/null; sudo -u postgres psql -c \"CREATE DATABASE ontokit OWNER ontokit;\" 2>/dev/null; sudo -u postgres psql -c \"GRANT ALL PRIVILEGES ON DATABASE ontokit TO ontokit;\""
```

5. Verify PostgreSQL accepts connections:
```bash
ssh ... "PGPASSWORD='ontokit-folio-prod' psql -h localhost -U ontokit -d ontokit -c 'SELECT 1;'"
```
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Do not hardcode production database credentials in committed deployment plans.

Lines 84/89/93/170 embed a real password value. This is sensitive configuration leakage and makes rotation harder. Use placeholders + secure injection at execution time.

🔧 Proposed fix
-ssh ... "sudo -u postgres psql -c \"CREATE USER ontokit WITH PASSWORD 'ontokit-folio-prod';\" ...
+ssh ... "sudo -u postgres psql -c \"CREATE USER ontokit WITH PASSWORD '${ONTO_DB_PASSWORD}';\" ...

-ssh ... "PGPASSWORD='ontokit-folio-prod' psql -h localhost -U ontokit -d ontokit -c 'SELECT 1;'"
+ssh ... "PGPASSWORD='${ONTO_DB_PASSWORD}' psql -h localhost -U ontokit -d ontokit -c 'SELECT 1;'"

-DATABASE_URL=postgresql+asyncpg://ontokit:ontokit-folio-prod@localhost:5432/ontokit
+DATABASE_URL=postgresql+asyncpg://ontokit:${ONTO_DB_PASSWORD}@localhost:5432/ontokit

Also applies to: 93-93, 170-170

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.planning/phases/09-production-deployment/09-01-PLAN.md around lines 84 -
90, The deployment plan currently hardcodes the DB password in the psql commands
(e.g., CREATE USER ontokit WITH PASSWORD 'ontokit-folio-prod'; and
PGPASSWORD='ontokit-folio-prod' psql ...), so replace the literal password with
a placeholder and document secure injection at runtime: change the CREATE
USER/GRANT commands and the verification command to use a variable like
'${DB_PASSWORD}' or rely on PGPASSWORD provided from the environment/secret
manager, and update the instructions to show how to export or inject DB_PASSWORD
(or use a secret-vault/CI variable) before running the ssh/psql commands so
credentials are never committed; apply this change to every occurrence of the
quoted password in the file.

Comment thread .planning/ROADMAP.md
Comment on lines +21 to +23
- [ ] **Phase 7: Sync ALEA Forks** - Fast-forward both ALEA repos to CatholicOS main
- [x] **Phase 8: Optional Auth** - Make authentication optional via AUTH_DISABLED env var across both repos (completed 2026-04-03)
- [ ] **Phase 9: Production Deployment** - Install Postgres, switch branches, configure env, rebuild, restart on production server
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Update the rollout docs to AUTH_MODE.

This roadmap still tells operators to use AUTH_DISABLED, but the implementation and test matrix in this PR have moved to AUTH_MODE=required|optional|disabled. Leaving the old flag here will send deployment and validation work down the wrong path.

Also applies to: 37-50, 57-61

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.planning/ROADMAP.md around lines 21 - 23, Update the roadmap to reference
AUTH_MODE instead of the obsolete AUTH_DISABLED flag: replace any occurrences of
"AUTH_DISABLED" with "AUTH_MODE" and describe the accepted values
(required|optional|disabled) and intended operator actions (e.g., set
AUTH_MODE=disabled to turn auth off) so rollout steps and validation guidance
match the implementation and test matrix; update all instances noted (including
the checklist entry and the other occurrences mentioned) and ensure the Phase 8
line and nearby instructions use AUTH_MODE consistently.

Comment thread .planning/STATE.md Outdated
Comment on lines +2 to +7
gsd_state_version: 1.0
milestone: v0.2
milestone_name: milestone
status: planning
stopped_at: Completed 08-02-PLAN.md
last_updated: "2026-04-03T16:23:01.929Z"
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Fix the milestone/auth metadata drift in the state file.

The front matter still says v0.2, while the body tracks Phase 8 of the v0.3.0 deployment milestone, and the decision log still records AUTH_DISABLED. If this file drives resume or planning automation, the next session will restore stale context.

Also applies to: 24-31, 56-63

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.planning/STATE.md around lines 2 - 7, The front-matter and decision
metadata are out of sync with the current Phase 8 of the v0.3.0 deployment;
update the front-matter keys (milestone -> "v0.3.0", milestone_name -> a clear
name like "deployment v0.3.0", last_updated -> current ISO timestamp) and remove
or replace the stale AUTH_DISABLED state in the decision log so the state
reflects the Phase 8 auth decision; also audit and correct the corresponding
sections previously noted (the blocks around the decision log and milestone
text) so all occurrences of v0.2/AUTH_DISABLED are replaced with the v0.3.0
phase and current auth status.

Comment thread app/projects/[id]/editor/page.tsx
Comment thread auth.ts
Comment on lines +6 to +9
// When auth is not required, provide a default secret so NextAuth doesn't crash
if (getAuthMode() !== "required" && !process.env.NEXTAUTH_SECRET) {
process.env.NEXTAUTH_SECRET = "ontokit-optional-auth-secret";
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "Expected: there should be a single source of truth for NEXTAUTH_SECRET, and production auth-capable modes should not rely on a public fallback."

echo
echo "== secret fallbacks and reads =="
rg -nP --type=ts 'NEXTAUTH_SECRET|AUTH_SECRET|ontokit-optional-auth-secret|ontokit-dev-secret' auth.ts lib

echo
echo "== auth.ts =="
sed -n '1,140p' auth.ts

Repository: CatholicOS/ontokit-web

Length of output: 4639


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== Checking lib/env.ts context around NEXTAUTH_SECRET defaults ==="
rg -B5 -A5 'NEXTAUTH_SECRET.*default' lib/env.ts

echo
echo "=== Checking how lib/env.ts is imported/used in auth.ts ==="
rg -n 'env|import.*env' auth.ts | head -20

echo
echo "=== Full lib/env.ts to understand the schema structure ==="
wc -l lib/env.ts && head -50 lib/env.ts

Repository: CatholicOS/ontokit-web

Length of output: 133


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== Listing files in lib directory ==="
fd . lib --max-depth 2 -t f

echo
echo "=== Check if lib/env.ts exists ==="
ls -la lib/env.ts 2>&1 || echo "File not found"

echo
echo "=== Reading lib/env.ts entirely ==="
cat -n lib/env.ts 2>&1 || echo "Cannot read file"

Repository: CatholicOS/ontokit-web

Length of output: 5855


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== Reading lib/auth-mode.ts to understand getAuthMode() and isAuthRequired() ==="
cat -n lib/auth-mode.ts

echo
echo "=== Checking where auth.ts is imported to understand initialization order ==="
rg -l 'import.*auth' --type=ts --type=tsx | head -10

Repository: CatholicOS/ontokit-web

Length of output: 1395


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== Check if auth.ts is imported in middleware or app entry points ==="
rg 'import.*auth' . --type ts -l | grep -E '(middleware|layout|page)' | head -10

echo
echo "=== Check middleware.ts ==="
cat -n middleware.ts 2>/dev/null || echo "No middleware.ts found"

echo
echo "=== Check app layout ==="
find . -name 'layout.ts' -o -name 'layout.tsx' | head -3 | xargs cat -n 2>/dev/null || echo "Layout file not found or error"

echo
echo "=== Key question: When auth.ts sets NEXTAUTH_SECRET, does lib/env.ts validation run before or after? ==="
echo "Checking if auth.ts is imported at module level in app initialization..."
rg 'from.*auth' . --type ts | grep -v node_modules | head -10

Repository: CatholicOS/ontokit-web

Length of output: 5610


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== Check if lib/env.ts is imported anywhere that would run at module init ==="
rg 'import.*env' . --type ts | grep -v node_modules | head -20

echo
echo "=== Check app/providers.ts which is imported at root layout ==="
cat -n app/providers.ts 2>/dev/null || echo "Not found"

echo
echo "=== Search for where serverEnv from lib/env.ts is actually used ==="
rg 'serverEnv|validateServerEnv|clientEnv|validateClientEnv' . --type ts | grep -v node_modules | head -15

Repository: CatholicOS/ontokit-web

Length of output: 1480


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== Search for actual usage of serverEnv export from lib/env.ts ==="
rg 'import.*serverEnv|from.*lib/env' . --type ts | grep -v test | grep -v node_modules

echo
echo "=== Check if NextAuth initialization in auth.ts happens before env validation anywhere ==="
echo "Looking for any files that import from both auth.ts and lib/env.ts..."
rg 'import.*auth' . --type ts | grep -v node_modules | xargs rg 'import.*env' | head -10

echo
echo "=== Check what actually imports auth.ts at the app level ==="
rg 'import.*auth' app --type ts 2>/dev/null | head -20

Repository: CatholicOS/ontokit-web

Length of output: 133


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== Check what happens when auth.ts is loaded - look at auth.ts more closely ==="
cat -n auth.ts | head -15

echo
echo "=== Confirm: the NEXTAUTH_SECRET fallback in auth.ts is set BEFORE NextAuth() is called ==="
grep -n 'NextAuth(' auth.ts

echo
echo "=== When does lib/env.ts validation actually run? Check if serverEnv is exported/used ==="
rg 'serverEnv' . --type ts | grep -v test | grep -v 'function validateServerEnv' | grep -v 'export const serverEnv' | grep -v 'return validateServerEnv'

echo
echo "=== Summary: Is auth.ts imported/executed at module level by NextAuth setup? ==="
echo "NextAuth handlers are exported from auth.ts, so auth.ts runs immediately when imported."
echo "The NEXTAUTH_SECRET assignment on lines 7-8 runs BEFORE NextAuth(authConfig) on line 107."
echo "This means the fallback secret is set into process.env before any other code can read it."

Repository: CatholicOS/ontokit-web

Length of output: 1031


Don't ship a shared fallback NEXTAUTH_SECRET.

Lines 7–8 set NEXTAUTH_SECRET to "ontokit-optional-auth-secret" when auth mode is not "required" and the env var is missing. In optional mode, the app still issues authenticated sessions via Zitadel, so every deployment that omits this env var will sign sessions with the same public secret—a session-forgery risk. Additionally, lib/env.ts (lines 31–34) defines a different hard-coded fallback ("ontokit-dev-secret"), creating configuration drift between the two auth entry points. Require an explicit secret whenever auth can issue sessions; limit any fallback to isolated dev/test environments only.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@auth.ts` around lines 6 - 9, Remove the hard-coded shared NEXTAUTH_SECRET
fallback and require an explicit secret whenever auth can issue sessions: stop
setting process.env.NEXTAUTH_SECRET = "ontokit-optional-auth-secret" in the
getAuthMode() branch; instead, if getAuthMode() indicates auth may issue
sessions and process.env.NEXTAUTH_SECRET is missing, throw/log a clear error and
exit (or surface a startup failure) unless running in an isolated dev/test
environment (check NODE_ENV === "development" or "test" or use the existing dev
helper); if you must provide a fallback for dev/test only, reuse the single
canonical dev fallback defined in lib/env.ts (or centralize that value) so both
entry points match.

Comment thread lib/env.ts
Comment on lines 22 to +35
function validateServerEnv(): ServerEnv {
const result = serverSchema.safeParse(process.env);
// When auth is not required, Zitadel vars are optional
const schema = isAuthRequired()
? serverSchema
: z.object({
ZITADEL_ISSUER: z.url().optional(),
ZITADEL_CLIENT_ID: z.string().optional(),
ZITADEL_CLIENT_SECRET: z.string().optional(),
NEXTAUTH_URL: z.url().optional(),
NEXTAUTH_SECRET: z
.string()
.min(1, "NEXTAUTH_SECRET is required")
.default("ontokit-dev-secret"),
});
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "Expected: any downstream code that reads Zitadel env values should either narrow auth mode first or see those fields typed as optional."

echo
echo "== lib/env.ts =="
sed -n '1,140p' lib/env.ts

echo
echo "== direct reads of serverEnv / validateServerEnv =="
rg -nP --type=ts '\bserverEnv\.(ZITADEL_ISSUER|ZITADEL_CLIENT_ID|ZITADEL_CLIENT_SECRET|NEXTAUTH_SECRET)\b|\bvalidateServerEnv\s*\('

Repository: CatholicOS/ontokit-web

Length of output: 3941


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== Check all files that import or use serverEnv =="
rg -l 'serverEnv|validateServerEnv' --type=ts --type=tsx | head -20

echo
echo "== Lines that use serverEnv (not just define it) =="
rg -n 'serverEnv\.' --type=ts --type=tsx -A 2 | head -50

echo
echo "== Middleware and auth setup files =="
find . -path ./node_modules -prune -o -type f \( -name '*middleware*' -o -name '*auth*' \) -print | grep -E '\.(ts|tsx)$' | head -20

Repository: CatholicOS/ontokit-web

Length of output: 161


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== Check auth.ts for how it uses serverEnv =="
if [ -f "auth.ts" ]; then
  cat -n auth.ts | head -100
fi

echo
echo "== Check for NextAuth configuration and env access =="
rg -n 'ZITADEL_ISSUER|ZITADEL_CLIENT_ID|NEXTAUTH_SECRET' --type=ts --type=tsx -B 2 -A 2 | head -80

Repository: CatholicOS/ontokit-web

Length of output: 4097


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== Files that import serverEnv =="
rg 'serverEnv' --type ts -l | head -20

echo
echo "== All lines using serverEnv (not just defining) =="
rg 'serverEnv\.' --type ts -n -B 1 -A 1

echo
echo "== Check lib/api/client.ts for env usage =="
if [ -f "lib/api/client.ts" ]; then
  head -80 lib/api/client.ts
fi

Repository: CatholicOS/ontokit-web

Length of output: 189


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== Search for middleware files =="
find . -name 'middleware.*' -type f 2>/dev/null | head -10

echo
echo "== Check what serverEnv actually exports and how it's consumed =="
rg 'export.*serverEnv|from.*env' --type ts -n -B 1 -A 1 | head -60

echo
echo "== Check if ZITADEL fields are accessed from serverEnv anywhere =="
rg 'serverEnv\[|serverEnv\.ZITADEL|serverEnv\.NEXTAUTH' --type ts

Repository: CatholicOS/ontokit-web

Length of output: 359


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== Check test file usage of serverEnv =="
cat -n __tests__/lib/env.test.ts

Repository: CatholicOS/ontokit-web

Length of output: 3818


Remove the as ServerEnv type assertion or return a conditional type that reflects the schema.

When auth is optional, validateServerEnv() parses with a schema where ZITADEL_ISSUER, ZITADEL_CLIENT_ID, and ZITADEL_CLIENT_SECRET are optional, but the return type is always ServerEnv (which requires them). The cast silently hides this mismatch from TypeScript.

While this doesn't currently cause issues since serverEnv itself is not consumed by any production code, the type lie is semantically incorrect and could become a problem if code later imports serverEnv. Consider either:

  • Returning a type that conditionally matches the schema (e.g., auth ? ServerEnv : PartialServerEnv)
  • Removing the cast and updating the return type signature to reflect reality
  • Removing the unused serverEnv export if it's not needed
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/env.ts` around lines 22 - 35, The validateServerEnv function currently
casts the parsed result to ServerEnv despite using a conditional schema
(serverSchema vs optional-zitadel schema); remove the unsafe "as ServerEnv" cast
and change the function's return type to reflect the conditional shape (e.g.,
ServerEnv | Partial<ServerEnv> or a new AuthOptionalServerEnv type) so
TypeScript matches the actual zod schema; update any callers to handle optional
Zitadel fields or, if serverEnv is unused, remove its export instead. Ensure to
reference validateServerEnv, serverSchema, and the local schema variable when
making the change.

damienriehl and others added 11 commits April 3, 2026 17:14
- Create 10-01-SUMMARY.md documenting anonymous session backend implementation
- Update STATE.md with position, decisions, and session info
- Update ROADMAP.md with Phase 10 progress (1/3 plans complete)
- Mark ANON-04 and ANON-05 requirements complete in REQUIREMENTS.md
- Add anonymousSuggestionsApi to suggestions.ts with createSession, save, submit, discard, beacon methods using X-Anonymous-Token header
- Add AnonymousSessionCreateResponse and AnonymousSubmitPayload types (website honeypot field)
- Create anonymousCreditStore.ts with useAnonymousCreditStore (name/email persistence) and useAnonymousTokenStore (token per projectId persistence)
- Both stores use zustand persist middleware with distinct localStorage keys
- Create useAnonymousSuggestion hook managing full anonymous session lifecycle (startSession, saveToSession, submitSession, discardSession)
- Hook restores active session from localStorage via useAnonymousTokenStore on mount
- Hook clears token store on submit and discard
- submitSession always sends website="" honeypot field, accepts optional submitterName/email
- Create CreditModal component shown post-submission with optional name/email fields
- CreditModal pre-fills from useAnonymousCreditStore cached values, caches on Save
- CreditModal has invisible honeypot field (name="website", tabIndex=-1, off-screen positioned)
- Re-exports SuggestionStatus type from useSuggestionSession
- Add canPropose, onProposeEdit, isAnonymousProposalMode props to ClassDetailPanelProps
- Render emerald-colored "Propose Edit" button when canPropose && !canEnterEdit
- Show "Sign in for full editing" alongside Propose Edit when Zitadel configured
- Thread new props through DeveloperEditorLayout and StandardEditorLayout
- Existing "Edit Item" and "Suggest Changes" buttons unchanged
… page

- Import useAnonymousSuggestion hook and CreditModal component
- Add canPropose flag (true when AUTH_MODE != required and user is not signed in)
- Add isAnonymousProposalMode flag (canPropose && anonymousSuggestion.isActive)
- Add handleProposeEdit, handleAnonymousClassUpdate, handleAnonymousSubmit callbacks
- Pass canPropose/onProposeEdit/isAnonymousProposalMode to both layout components
- When isAnonymousProposalMode: canEdit=true, isSuggestionMode=true for form editing
- Submit Proposal button opens CreditModal pre-submit; CreditModal calls submitSession
- Add Proposing indicator and Discard button in editor header for anonymous mode
- Add is_anonymous badge to review page list and detail views
- Add is_anonymous to SuggestionSessionSummary type
- 10-03-SUMMARY.md: Propose Edit button, anonymous proposal mode, CreditModal pre-submit, Anonymous badge in review page
- STATE.md: progress updated to 83%, decisions and session recorded
- ROADMAP.md: Phase 10 all 3 plans complete
- REQUIREMENTS.md: ANON-01, ANON-06, ANON-07 marked complete

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@damienriehl damienriehl changed the title feat: AUTH_MODE support with sign-in-to-edit UX feat: AUTH_MODE support, anonymous suggestions, and sign-in-to-edit UX Apr 4, 2026
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

🧹 Nitpick comments (5)
lib/stores/anonymousCreditStore.ts (2)

1-14: Consider adding "use client" directive for defensive SSR safety.

The stores use localStorage directly via createJSONStorage(() => localStorage). While all current consumers are client components (per context snippets), adding a "use client" directive at the top of this file would provide an extra safety layer and make the client-only requirement explicit.

+/**
+ * `@client-only` — uses localStorage for persistence
+ */
+"use client";
+
 /**
  * Anonymous suggestion stores

This is optional since the current architecture ensures client-only usage, but it documents the constraint and prevents accidental server-side imports.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/stores/anonymousCreditStore.ts` around lines 1 - 14, Add the "use client"
directive at the top of this module to mark it as client-only and avoid
accidental server-side imports; update the file containing the zustand stores
(where createJSONStorage(() => localStorage) is used) by inserting "use client"
as the very first line above the imports so useAnonymousCreditStore and
useAnonymousTokenStore are explicitly client-only.

73-103: Anonymous tokens persist indefinitely — consider expiration handling.

The token store has no mechanism to detect or handle token expiration. If the server-side anonymous tokens have a TTL (e.g., 24 hours), a user could restore a stale token from localStorage and face 401 errors when trying to resume their session.

Consider either:

  1. Storing the token creation timestamp and clearing stale tokens on restore
  2. Handling 401 errors in useAnonymousSuggestion by clearing the token and prompting the user to start a new session

This isn't blocking for initial release but could cause confusing UX for users who leave sessions overnight.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/stores/anonymousCreditStore.ts` around lines 73 - 103, The anonymous
token store (useAnonymousTokenStore) currently persists tokens indefinitely; add
expiration handling by storing a createdAt timestamp with each token in setToken
(e.g., { token, sessionId, branch, createdAt }) and update getToken to check
that timestamp against a TTL (clearToken and return null if stale).
Alternatively or additionally, update useAnonymousSuggestion to detect 401
responses from the server, call clearToken(projectId) and surface a prompt to
start a new session. Ensure references to setToken, getToken, clearToken and
useAnonymousSuggestion are used so the expiry logic is applied both on restore
and on API 401 handling.
components/editor/ClassDetailPanel.tsx (1)

111-111: Unused prop: isAnonymousProposalMode is destructured but never used.

The prop is renamed to _isAnonymousProposalMode to suppress the unused variable warning, but this suggests it was added for completeness without an actual use case in this component. If it's not needed, consider removing it from the interface to reduce API surface.

-  isAnonymousProposalMode: _isAnonymousProposalMode,
+  // isAnonymousProposalMode — not currently used in this component

Or remove from ClassDetailPanelProps entirely if downstream components don't need to pass it.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@components/editor/ClassDetailPanel.tsx` at line 111, The prop
isAnonymousProposalMode is being destructured as _isAnonymousProposalMode in
ClassDetailPanel (and thus unused); either remove it from the
ClassDetailPanelProps interface and any callers that pass it, or keep it in the
interface and actually use it inside the ClassDetailPanel component (referencing
isAnonymousProposalMode/_isAnonymousProposalMode where needed) so the rename
hack is unnecessary; update all references to ClassDetailPanelProps and the
ClassDetailPanel component signature to reflect the chosen approach.
lib/hooks/useAnonymousSuggestion.ts (1)

132-139: Error recovery preserves session state, allowing retry.

When saveToSession fails, the hook sets status: "error" but keeps sessionId, branch, and anonymousToken intact. This allows the user to retry the save operation, which is good UX.

However, if the error is a 401 (expired token), the retry will also fail. Consider detecting 401 errors and clearing the token to prompt a fresh session:

} catch (err) {
  const msg = err instanceof Error ? err.message : "Failed to save anonymous suggestion";
  // If token is invalid/expired, clear session to allow recovery
  if (msg.includes("401") || msg.includes("Unauthorized")) {
    tokenStore.clearToken(projectId);
    setSessionId(null);
    setBranch(null);
    setAnonymousToken(null);
  }
  setStatus("error");
  setError(msg);
  onError?.(msg);
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/hooks/useAnonymousSuggestion.ts` around lines 132 - 139, When
saveToSession inside the useAnonymousSuggestion hook throws an error, detect
401/Unauthorized errors and clear the cached token/session so retries can prompt
a fresh session: in the catch block that currently builds msg and calls
setStatus("error"), setError(msg), and onError?.(msg), add logic to detect 401
or "Unauthorized" in the error message (or inspect err.status if available) and
call tokenStore.clearToken(projectId) then setSessionId(null), setBranch(null),
and setAnonymousToken(null) before setting status/error and invoking onError;
keep the existing error handling for non-401 failures.
.planning/STATE.md (1)

50-54: Performance metrics contain data quality issues.

  • Line 50: "4min" should be "4 min" (missing space)
  • Line 52: "525602min" (~365 days) appears to be erroneous data
  • Lines 71-74: "Phase Phase 10-anonymous-suggestions" has duplicated "Phase" prefix

These are planning artifacts, so impact is low, but they may confuse future readers or automation that parses this file.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.planning/STATE.md around lines 50 - 54, Fix the data-quality issues in the
planning table: add the missing space in "4min" so "Phase 08-optional-auth P01"
reads "4 min"; correct or sanitize the clearly erroneous "525602min" value for
"Phase 10-anonymous-suggestions P01" (replace with the intended duration, a
reasonable calculated value, or a placeholder like "TBD"); and remove the
duplicated prefix in the rows that show "Phase Phase 10-anonymous-suggestions"
so they read "Phase 10-anonymous-suggestions". Also scan neighboring rows for
similar spacing or prefix duplication and make them consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.planning/phases/10-anonymous-suggestions/10-02-PLAN.md:
- Around line 287-317: The flow currently opens CreditModal after
anonymousSuggestionsApi.submit() so submitter_name/submitter_email/website never
reach the backend; update the flow so submitSession() receives and sends the
credit fields (or open the CreditModal before calling submitSession()).
Concretely, modify the submit action that calls
anonymousSuggestionsApi.submit(...) (the submitSession() function) to accept
name/email/website parameters and include them in the request payload (and still
call useAnonymousCreditStore.setCredit in the modal on Save), or alternatively
change the UI to show CreditModal before invoking submitSession() so the modal
returns name/email/website which you then pass into
anonymousSuggestionsApi.submit(...). Ensure the honeypot "website" field value
is forwarded in the payload and that CreditModal pre-fills from
useAnonymousCreditStore and caches via
useAnonymousCreditStore.setCredit(name,email).

In `@app/projects/`[id]/editor/page.tsx:
- Around line 1544-1548: The CreditModal is wired so onClose calls
handleAnonymousSubmit(null, null), causing backdrop/ESC dismissals to submit;
change the parent JSX so onClose only closes the modal (e.g.,
setCreditModalOpen(false)) and keep onSubmitCredit pointing at
handleAnonymousSubmit; inside CreditModal update handleSkip to call
onSubmitCredit(null, null) then call onClose(), and update handleSave to call
onSubmitCredit(...) without calling onClose itself (the parent will close after
successful submit) so dismissing the modal does not trigger a submit.

In `@app/projects/`[id]/suggestions/review/page.tsx:
- Around line 415-420: The code currently renders the literal "Anonymous" twice
because the text fallback uses s.submitter?.name || s.submitter?.email ||
(s.is_anonymous ? "Anonymous" : "Unknown user") and you always render the
Anonymous badge; change the logic so the badge is only shown when there is an
actual credited identity (i.e. s.is_anonymous && (s.submitter?.name ||
s.submitter?.email)), or alternatively keep the badge for any anonymous but
change the text fallback when the badge is shown; update the rendering around s,
s.submitter, and s.is_anonymous in the component (page.tsx) to implement one of
these fixes to avoid "Anonymous Anonymous".

In `@components/suggestions/CreditModal.tsx`:
- Around line 22-33: The docstring for the CreditModal component is inaccurate:
update it to state that CreditModal opens BEFORE submission (it is shown
pre-submit), collects credit info that is passed directly into submitSession()
so the credit and suggestion are sent in a single API call, and that the modal
can affect submission flow (i.e., credit data is included on submit rather than
being stored only for future proposals); still mention useAnonymousCreditStore
remembers values and the honeypot field (name="website") is included and handled
by useAnonymousSuggestion. Reference the CreditModal component, submitSession(),
useAnonymousCreditStore, and useAnonymousSuggestion when updating the comment.
- Around line 65-66: In handleSave, avoid calling onClose immediately after
onSubmitCredit to prevent double invocation of handleAnonymousSubmit (which is
passed as both onSubmitCredit and wrapped by onClose); remove the onClose() call
from handleSave and let the parent component close the modal after
onSubmitCredit resolves (or have the parent decide when to call onClose),
referencing handleSave, onSubmitCredit, onClose, handleAnonymousSubmit and
submitSession so the submit flow is only triggered once and relies on the parent
to close the modal.

In `@lib/api/suggestions.ts`:
- Around line 108-112: The AnonymousSessionCreateResponse shape currently
exposes anonymous_token and the client uses that for the beacon URL, leaking a
token that permits save/submit/discard; change the response/schema and client
usage to return and consume a short‑lived scoped beacon token (e.g., add
beacon_token or scoped_beacon_token to AnonymousSessionCreateResponse) and
ensure any code that constructs the beacon URL uses this new beacon token
instead of anonymous_token; update backend response to emit the scoped beacon
token and keep anonymous_token as the longer‑lived session credential (or remove
exposing it to beacon flows) so write privileges are not leaked via proxy/access
logs.

---

Nitpick comments:
In @.planning/STATE.md:
- Around line 50-54: Fix the data-quality issues in the planning table: add the
missing space in "4min" so "Phase 08-optional-auth P01" reads "4 min"; correct
or sanitize the clearly erroneous "525602min" value for "Phase
10-anonymous-suggestions P01" (replace with the intended duration, a reasonable
calculated value, or a placeholder like "TBD"); and remove the duplicated prefix
in the rows that show "Phase Phase 10-anonymous-suggestions" so they read "Phase
10-anonymous-suggestions". Also scan neighboring rows for similar spacing or
prefix duplication and make them consistent.

In `@components/editor/ClassDetailPanel.tsx`:
- Line 111: The prop isAnonymousProposalMode is being destructured as
_isAnonymousProposalMode in ClassDetailPanel (and thus unused); either remove it
from the ClassDetailPanelProps interface and any callers that pass it, or keep
it in the interface and actually use it inside the ClassDetailPanel component
(referencing isAnonymousProposalMode/_isAnonymousProposalMode where needed) so
the rename hack is unnecessary; update all references to ClassDetailPanelProps
and the ClassDetailPanel component signature to reflect the chosen approach.

In `@lib/hooks/useAnonymousSuggestion.ts`:
- Around line 132-139: When saveToSession inside the useAnonymousSuggestion hook
throws an error, detect 401/Unauthorized errors and clear the cached
token/session so retries can prompt a fresh session: in the catch block that
currently builds msg and calls setStatus("error"), setError(msg), and
onError?.(msg), add logic to detect 401 or "Unauthorized" in the error message
(or inspect err.status if available) and call tokenStore.clearToken(projectId)
then setSessionId(null), setBranch(null), and setAnonymousToken(null) before
setting status/error and invoking onError; keep the existing error handling for
non-401 failures.

In `@lib/stores/anonymousCreditStore.ts`:
- Around line 1-14: Add the "use client" directive at the top of this module to
mark it as client-only and avoid accidental server-side imports; update the file
containing the zustand stores (where createJSONStorage(() => localStorage) is
used) by inserting "use client" as the very first line above the imports so
useAnonymousCreditStore and useAnonymousTokenStore are explicitly client-only.
- Around line 73-103: The anonymous token store (useAnonymousTokenStore)
currently persists tokens indefinitely; add expiration handling by storing a
createdAt timestamp with each token in setToken (e.g., { token, sessionId,
branch, createdAt }) and update getToken to check that timestamp against a TTL
(clearToken and return null if stale). Alternatively or additionally, update
useAnonymousSuggestion to detect 401 responses from the server, call
clearToken(projectId) and surface a prompt to start a new session. Ensure
references to setToken, getToken, clearToken and useAnonymousSuggestion are used
so the expiry logic is applied both on restore and on API 401 handling.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: a8462b0c-9395-454a-918b-98db4dcf4b68

📥 Commits

Reviewing files that changed from the base of the PR and between 8026c4a and 03765a6.

📒 Files selected for processing (20)
  • .planning/REQUIREMENTS.md
  • .planning/ROADMAP.md
  • .planning/STATE.md
  • .planning/phases/10-anonymous-suggestions/10-01-PLAN.md
  • .planning/phases/10-anonymous-suggestions/10-01-SUMMARY.md
  • .planning/phases/10-anonymous-suggestions/10-02-PLAN.md
  • .planning/phases/10-anonymous-suggestions/10-02-SUMMARY.md
  • .planning/phases/10-anonymous-suggestions/10-03-PLAN.md
  • .planning/phases/10-anonymous-suggestions/10-03-SUMMARY.md
  • .planning/phases/10-anonymous-suggestions/10-CONTEXT.md
  • .planning/phases/10-anonymous-suggestions/10-DISCUSSION-LOG.md
  • app/projects/[id]/editor/page.tsx
  • app/projects/[id]/suggestions/review/page.tsx
  • components/editor/ClassDetailPanel.tsx
  • components/editor/developer/DeveloperEditorLayout.tsx
  • components/editor/standard/StandardEditorLayout.tsx
  • components/suggestions/CreditModal.tsx
  • lib/api/suggestions.ts
  • lib/hooks/useAnonymousSuggestion.ts
  • lib/stores/anonymousCreditStore.ts
✅ Files skipped from review due to trivial changes (4)
  • .planning/ROADMAP.md
  • .planning/phases/10-anonymous-suggestions/10-02-SUMMARY.md
  • .planning/REQUIREMENTS.md
  • .planning/phases/10-anonymous-suggestions/10-03-PLAN.md
🚧 Files skipped from review as they are similar to previous changes (1)
  • components/editor/standard/StandardEditorLayout.tsx

Comment on lines +287 to +317
Implementation details:
- On mount, check `useAnonymousTokenStore.getToken(projectId)` for an existing active session. If found, restore `sessionId`, `branch`, `anonymousToken`, set status to "active".
- `startSession()`: call `anonymousSuggestionsApi.createSession(projectId)`, store the returned token in `useAnonymousTokenStore.setToken(projectId, ...)`, set state.
- `saveToSession()`: call `anonymousSuggestionsApi.save(...)` with the stored anonymous token.
- `submitSession()`: call `anonymousSuggestionsApi.submit(...)` with summary + submitter info + `website: ""` (honeypot always empty). On success, call `useAnonymousTokenStore.clearToken(projectId)` and reset state.
- `discardSession()`: call `anonymousSuggestionsApi.discard(...)`, clear token store, reset state.
- Export `SuggestionStatus` type from `useSuggestionSession.ts` and reuse it (or re-export if already exported).

**2. Create `components/suggestions/CreditModal.tsx`:**

A modal dialog that appears AFTER successful submission. It does NOT block the submission — it appears after the submit API call succeeds.

Props:
```typescript
interface CreditModalProps {
open: boolean;
onClose: () => void;
onSubmitCredit: (name: string | null, email: string | null) => void;
}
```

UI (follow existing dialog patterns from RejectSuggestionDialog.tsx):
- Title: "Want credit for your suggestions?"
- Subtitle: "Your changes have been submitted for review. Optionally share your name and email so the project maintainers can follow up."
- Input field: "Name" (optional, placeholder: "Your name")
- Input field: "Email" (optional, placeholder: "your@email.com", type="email")
- Hidden honeypot field: `<input name="website" tabIndex={-1} autoComplete="off" style={{ position: "absolute", left: "-9999px", opacity: 0 }} />` — invisible to humans, filled by bots
- Pre-fill from `useAnonymousCreditStore` if cached values exist
- Buttons: "Skip" (calls `onClose`), "Save" (calls `onSubmitCredit` with name/email values, also calls `useAnonymousCreditStore.setCredit(name, email)` to cache for future proposals)
- If user clicks "Save" with empty fields, treat as skip (pass null/null)
- Use `@/components/ui/dialog` or whatever dialog primitive the project uses. Check existing dialog patterns.
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

The credit modal is sequenced too late to populate the submit payload.

Line 291 expects submitSession() to send submitter_name, submitter_email, and website, but Lines 297-317 open the modal only after that request succeeds and only hand the collected values back to the UI. As written, credit is cached locally but never reaches the backend, and the honeypot is effectively a no-op. Either collect these fields before anonymousSuggestionsApi.submit(), or add a follow-up endpoint that persists credit after submission.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.planning/phases/10-anonymous-suggestions/10-02-PLAN.md around lines 287 -
317, The flow currently opens CreditModal after anonymousSuggestionsApi.submit()
so submitter_name/submitter_email/website never reach the backend; update the
flow so submitSession() receives and sends the credit fields (or open the
CreditModal before calling submitSession()). Concretely, modify the submit
action that calls anonymousSuggestionsApi.submit(...) (the submitSession()
function) to accept name/email/website parameters and include them in the
request payload (and still call useAnonymousCreditStore.setCredit in the modal
on Save), or alternatively change the UI to show CreditModal before invoking
submitSession() so the modal returns name/email/website which you then pass into
anonymousSuggestionsApi.submit(...). Ensure the honeypot "website" field value
is forwarded in the payload and that CreditModal pre-fills from
useAnonymousCreditStore and caches via
useAnonymousCreditStore.setCredit(name,email).

Comment on lines +1544 to +1548
<CreditModal
open={creditModalOpen}
onClose={() => handleAnonymousSubmit(null, null)}
onSubmitCredit={handleAnonymousSubmit}
/>
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

CreditModal onClose triggers submission — dismissing the modal submits with null credit.

The onClose handler is set to () => handleAnonymousSubmit(null, null), which means:

  • Clicking outside the modal → submits proposal with no credit
  • Pressing ESC → submits proposal with no credit
  • Clicking "Skip" → correctly skips (via handleSkiponClose)
  • Clicking "Save" → submits with credit, then onClose tries to submit again (see CreditModal review)

This behavior may be intentional for "Skip" but is surprising for dismiss-by-backdrop/ESC. Users might accidentally submit when trying to cancel.

🛡️ Suggested fix: separate skip from dismiss
       <CreditModal
         open={creditModalOpen}
-        onClose={() => handleAnonymousSubmit(null, null)}
+        onClose={() => setCreditModalOpen(false)}
         onSubmitCredit={handleAnonymousSubmit}
       />

Then update CreditModal's handleSkip to call onSubmitCredit(null, null) before onClose(), and update handleSave to not call onClose() (parent closes after submit).

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<CreditModal
open={creditModalOpen}
onClose={() => handleAnonymousSubmit(null, null)}
onSubmitCredit={handleAnonymousSubmit}
/>
<CreditModal
open={creditModalOpen}
onClose={() => setCreditModalOpen(false)}
onSubmitCredit={handleAnonymousSubmit}
/>
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/projects/`[id]/editor/page.tsx around lines 1544 - 1548, The CreditModal
is wired so onClose calls handleAnonymousSubmit(null, null), causing
backdrop/ESC dismissals to submit; change the parent JSX so onClose only closes
the modal (e.g., setCreditModalOpen(false)) and keep onSubmitCredit pointing at
handleAnonymousSubmit; inside CreditModal update handleSkip to call
onSubmitCredit(null, null) then call onClose(), and update handleSave to call
onSubmitCredit(...) without calling onClose itself (the parent will close after
successful submit) so dismissing the modal does not trigger a submit.

Comment on lines +415 to +420
{s.submitter?.name || s.submitter?.email || (s.is_anonymous ? "Anonymous" : "Unknown user")}
</span>
{s.is_anonymous && (
<span className="rounded bg-slate-100 px-1.5 py-0.5 text-xs text-slate-600 dark:bg-slate-700 dark:text-slate-300">
Anonymous
</span>
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Avoid the duplicated “Anonymous” label.

When submitter.name/email is absent, the text fallback already becomes "Anonymous", so the extra badge renders Anonymous Anonymous in both views. Gate the badge on actual credited identity, or change the fallback copy when the badge is shown.

Also applies to: 523-531

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/projects/`[id]/suggestions/review/page.tsx around lines 415 - 420, The
code currently renders the literal "Anonymous" twice because the text fallback
uses s.submitter?.name || s.submitter?.email || (s.is_anonymous ? "Anonymous" :
"Unknown user") and you always render the Anonymous badge; change the logic so
the badge is only shown when there is an actual credited identity (i.e.
s.is_anonymous && (s.submitter?.name || s.submitter?.email)), or alternatively
keep the badge for any anonymous but change the text fallback when the badge is
shown; update the rendering around s, s.submitter, and s.is_anonymous in the
component (page.tsx) to implement one of these fixes to avoid "Anonymous
Anonymous".

Comment on lines +22 to +33
/**
* Post-submission modal that lets anonymous contributors optionally
* provide their name and email for credit attribution.
*
* Appears AFTER a successful suggestion submit — it does not block the
* submission itself. Credit info is remembered in localStorage via
* useAnonymousCreditStore for future proposals.
*
* Includes a hidden honeypot field (name="website") that bots will fill
* but human users won't see. The parent should pass this value through
* to the submit payload as website="" (already handled by useAnonymousSuggestion).
*/
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Docstring is inconsistent with actual usage.

The docstring states this modal "Appears AFTER a successful suggestion submit" and "does not block the submission itself." However, based on the wiring in app/projects/[id]/editor/page.tsx (lines 908-912), the modal opens BEFORE submission and the credit data is passed to submitSession() directly.

The .planning/STATE.md decisions also confirm: "CreditModal opens pre-submit so credit info is passed directly to submitSession() in one API call."

📝 Suggested docstring update
 /**
- * Post-submission modal that lets anonymous contributors optionally
+ * Pre-submission modal that lets anonymous contributors optionally
  * provide their name and email for credit attribution.
  *
- * Appears AFTER a successful suggestion submit — it does not block the
- * submission itself. Credit info is remembered in localStorage via
+ * Appears when "Submit Proposal" is clicked — credit info is passed
+ * directly to the submission API call. Credit info is remembered in localStorage via
  * useAnonymousCreditStore for future proposals.
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
/**
* Post-submission modal that lets anonymous contributors optionally
* provide their name and email for credit attribution.
*
* Appears AFTER a successful suggestion submit it does not block the
* submission itself. Credit info is remembered in localStorage via
* useAnonymousCreditStore for future proposals.
*
* Includes a hidden honeypot field (name="website") that bots will fill
* but human users won't see. The parent should pass this value through
* to the submit payload as website="" (already handled by useAnonymousSuggestion).
*/
/**
* Pre-submission modal that lets anonymous contributors optionally
* provide their name and email for credit attribution.
*
* Appears when "Submit Proposal" is clicked credit info is passed
* directly to the submission API call. Credit info is remembered in localStorage via
* useAnonymousCreditStore for future proposals.
*
* Includes a hidden honeypot field (name="website") that bots will fill
* but human users won't see. The parent should pass this value through
* to the submit payload as website="" (already handled by useAnonymousSuggestion).
*/
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@components/suggestions/CreditModal.tsx` around lines 22 - 33, The docstring
for the CreditModal component is inaccurate: update it to state that CreditModal
opens BEFORE submission (it is shown pre-submit), collects credit info that is
passed directly into submitSession() so the credit and suggestion are sent in a
single API call, and that the modal can affect submission flow (i.e., credit
data is included on submit rather than being stored only for future proposals);
still mention useAnonymousCreditStore remembers values and the honeypot field
(name="website") is included and handled by useAnonymousSuggestion. Reference
the CreditModal component, submitSession(), useAnonymousCreditStore, and
useAnonymousSuggestion when updating the comment.

Comment on lines +65 to +66
onSubmitCredit(trimmedName, trimmedEmail);
onClose();
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Potential double-close when onClose wraps onSubmitCredit.

In handleSave, both onSubmitCredit and onClose are called sequentially. Looking at the editor page wiring:

  • onClose={() => handleAnonymousSubmit(null, null)}
  • onSubmitCredit={handleAnonymousSubmit}

When Save is clicked with credit data, handleAnonymousSubmit(name, email) is called, then onClose() calls handleAnonymousSubmit(null, null) again, potentially triggering a duplicate submission attempt.

The submitSession call has an early return if !sessionId || !anonymousToken, so the second call should be a no-op after the first clears session state. However, this is fragile and depends on the async timing of submitSession.

🛡️ Suggested fix: remove redundant onClose call
     onSubmitCredit(trimmedName, trimmedEmail);
-    onClose();
   };

The parent should handle closing the modal after submission completes.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@components/suggestions/CreditModal.tsx` around lines 65 - 66, In handleSave,
avoid calling onClose immediately after onSubmitCredit to prevent double
invocation of handleAnonymousSubmit (which is passed as both onSubmitCredit and
wrapped by onClose); remove the onClose() call from handleSave and let the
parent component close the modal after onSubmitCredit resolves (or have the
parent decide when to call onClose), referencing handleSave, onSubmitCredit,
onClose, handleAnonymousSubmit and submitSession so the submit flow is only
triggered once and relies on the parent to close the modal.

Comment thread lib/api/suggestions.ts
Comment on lines +108 to +112
export interface AnonymousSessionCreateResponse {
session_id: string;
branch: string;
created_at: string;
anonymous_token: string;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Use a scoped beacon token instead of the anonymous session token.

The authenticated flow already separates the short-lived beacon credential from the main session credential, but the anonymous path sends anonymous_token in the beacon URL. That token also authorizes save/submit/discard, so any proxy or access logging leaks write access to the session.

🔒 Suggested shape for this client
 export interface AnonymousSessionCreateResponse {
   session_id: string;
   branch: string;
   created_at: string;
   anonymous_token: string;
+  beacon_token: string;
 }
@@
   beacon: (
     projectId: string,
     sessionId: string,
     content: string,
-    anonymousToken: string,
+    beaconToken: string,
   ) => {
-    const url = `${API_BASE}/api/v1/projects/${projectId}/suggestions/anonymous/beacon?token=${encodeURIComponent(anonymousToken)}`;
+    const url = `${API_BASE}/api/v1/projects/${projectId}/suggestions/anonymous/beacon?token=${encodeURIComponent(beaconToken)}`;
     const payload = JSON.stringify({ session_id: sessionId, content });
     const blob = new Blob([payload], { type: "application/json" });
     return navigator.sendBeacon(url, blob);
   },

This still needs the matching backend response/schema change so the client receives the scoped beacon token.

Also applies to: 343-352

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/api/suggestions.ts` around lines 108 - 112, The
AnonymousSessionCreateResponse shape currently exposes anonymous_token and the
client uses that for the beacon URL, leaking a token that permits
save/submit/discard; change the response/schema and client usage to return and
consume a short‑lived scoped beacon token (e.g., add beacon_token or
scoped_beacon_token to AnonymousSessionCreateResponse) and ensure any code that
constructs the beacon URL uses this new beacon token instead of anonymous_token;
update backend response to emit the scoped beacon token and keep anonymous_token
as the longer‑lived session credential (or remove exposing it to beacon flows)
so write privileges are not leaked via proxy/access logs.

@JohnRDOrazio
Copy link
Copy Markdown
Member

I don't really agree with this approach, of allowing anonymous suggestions. Everyone should take responsibility for the changes they suggest. And most project admins will probably want to know who is making which suggestion, and for what reason. Before making any more PRs for these kinds of changes, we should really think them through first. Could you please open issues first, so that we can discuss them, and then if we have an agreed upon direction, we can move to making the PRs? It's better to lay out the roadmap for what we want to achieve ahead of time by opening issues, and once we have a clear roadmap then we start working towards the next release.

@JohnRDOrazio JohnRDOrazio changed the base branch from main to dev April 13, 2026 19:56
@JohnRDOrazio JohnRDOrazio added enhancement New feature or request question Further information is requested labels Apr 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request question Further information is requested

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants