Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions .do/app.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,12 @@ alerts:

ingress:
rules:
# NextAuth OAuth routes → web (must be before /api catch-all)
- component:
name: web
match:
path:
prefix: /api/auth
# API endpoints → backend
- component:
name: api
Expand Down Expand Up @@ -130,3 +136,18 @@ services:
scope: RUN_TIME
type: SECRET
value: REPLACE_WITH_API_KEY
- key: GOOGLE_CLIENT_ID
scope: RUN_TIME
type: SECRET
value: REPLACE_WITH_GOOGLE_CLIENT_ID
- key: GOOGLE_CLIENT_SECRET
scope: RUN_TIME
type: SECRET
value: REPLACE_WITH_GOOGLE_CLIENT_SECRET
- key: AUTH_SECRET
scope: RUN_TIME
type: SECRET
value: REPLACE_WITH_AUTH_SECRET
- key: AUTH_URL
scope: RUN_TIME
value: ${APP_URL}
33 changes: 32 additions & 1 deletion agent/db/connection.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

_pool: asyncpg.Pool | None = None
_pool_lock: asyncio.Lock | None = None
_users_table_ensured: bool = False
_DEFAULT_POOL_MIN_SIZE = 2
_DEFAULT_POOL_MAX_SIZE = 10
_DEFAULT_POOL_RETRIES = 5
Expand All @@ -31,8 +32,28 @@ def _float_env(name: str, default: float, minimum: float = 0.0) -> float:
return default


async def ensure_users_table(pool: asyncpg.Pool) -> None:
"""Create the users table if it does not already exist (idempotent)."""
async with pool.acquire() as conn:
await conn.execute(
"""
CREATE TABLE IF NOT EXISTS users (
id TEXT PRIMARY KEY DEFAULT gen_random_uuid()::text,
email TEXT UNIQUE NOT NULL,
name TEXT,
image TEXT,
approved BOOLEAN NOT NULL DEFAULT FALSE,
domain TEXT NOT NULL,
created_at TIMESTAMPTZ NOT NULL DEFAULT now(),
last_login_at TIMESTAMPTZ NOT NULL DEFAULT now()
);
CREATE INDEX IF NOT EXISTS idx_users_email ON users(email);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

This index is redundant. The email column is already defined with a UNIQUE constraint on line 42, and PostgreSQL automatically creates a unique index for every unique constraint. Removing it will save storage and slightly improve write performance.

"""
)


async def get_pool() -> asyncpg.Pool:
global _pool, _pool_lock
global _pool, _pool_lock, _users_table_ensured
if _pool_lock is None:
_pool_lock = asyncio.Lock()
if _pool is not None:
Expand Down Expand Up @@ -65,6 +86,16 @@ async def get_pool() -> asyncpg.Pool:
await asyncio.sleep(retry_delay * (attempt + 1))
if _pool is None and last_error is not None:
raise last_error
if _pool is not None and not _users_table_ensured:
try:
await ensure_users_table(_pool)
_users_table_ensured = True
except Exception:
import logging

logging.getLogger(__name__).warning(
"Could not create users table — DB may not be ready yet"
)
Comment on lines +93 to +98
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Swallowing the exception makes it difficult to diagnose why the table creation failed (e.g., permission issues or syntax errors). Using logging.exception will capture the full stack trace. Also, consider moving the import logging to the top of the file for better performance and style.

Suggested change
except Exception:
import logging
logging.getLogger(__name__).warning(
"Could not create users table — DB may not be ready yet"
)
except Exception:
import logging
logging.getLogger(__name__).exception("Could not create users table")

Comment on lines +89 to +98
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

users 테이블 초기화 실패가 영구 고착될 수 있습니다.

여기서 예외를 삼키고 계속 진행하면, Line 59가 이후 호출에서 _pool을 바로 반환하므로 ensure_users_table()를 다시 시도하지 않습니다. 게다가 첫 호출이 아직 테이블을 만드는 중일 때 다른 코루틴이 이미 _pool을 받아 users를 조회할 수 있어서, /dashboard/auth/*가 간헐적으로 relation "users" does not exist로 깨질 수 있습니다.

🔧 수정 방향 예시
 async def get_pool() -> asyncpg.Pool:
     global _pool, _pool_lock, _users_table_ensured
     if _pool_lock is None:
         _pool_lock = asyncio.Lock()
-    if _pool is not None:
+    if _pool is not None and _users_table_ensured:
         return _pool
     async with _pool_lock:
-        if _pool is not None:
+        if _pool is not None and _users_table_ensured:
             return _pool
         database_url = os.environ.get("DATABASE_URL", "")
         if not database_url:
             raise RuntimeError("DATABASE_URL environment variable is required")
@@
-        if _pool is not None and not _users_table_ensured:
+        if _pool is not None and not _users_table_ensured:
             try:
                 await ensure_users_table(_pool)
                 _users_table_ensured = True
-            except Exception:
+            except Exception:
                 import logging

                 logging.getLogger(__name__).warning(
-                    "Could not create users table — DB may not be ready yet"
+                    "Could not create users table — DB may not be ready yet",
+                    exc_info=True,
                 )
     return _pool
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@agent/db/connection.py` around lines 89 - 98, The current code swallows
exceptions from ensure_users_table(_pool) and may mark the DB as ready, causing
permanent failure and race conditions; change the logic so _users_table_ensured
is only set to True after ensure_users_table returns successfully, and on
exception do not leave a usable _pool — instead log the exception details and
clean up/reset _pool (or re-raise) so subsequent calls will retry table
creation; update the block around _pool, _users_table_ensured and
ensure_users_table() to close/reset _pool on failure and include the caught
exception in the warning log.

return _pool


Expand Down
85 changes: 85 additions & 0 deletions agent/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -1970,6 +1970,91 @@ async def zero_prompt_build_status(session_id: str, card_id: str):
}


# ── Dashboard Auth ────────────────────────────────────────────────────


class UpsertUserRequest(BaseModel):
email: str
name: str = ""
image: str = ""


class UpsertUserResponse(BaseModel):
id: str
email: str
name: str
approved: bool
domain: str
Comment on lines +1988 to +1999
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

OAuth 프로필의 nullable 필드와 API 계약이 맞지 않습니다.

웹 쪽에서는 user.name / user.imagenull일 수 있는데, 여기서는 둘 다 str로 강제하고 있습니다. 그 경우 /dashboard/auth/upsert-user가 422를 반환하고, name이 NULL로 저장되면 UpsertUserResponse.name도 다시 검증에서 터집니다.

🔧 수정 예시
 class UpsertUserRequest(BaseModel):
     email: str
-    name: str = ""
-    image: str = ""
+    name: str | None = None
+    image: str | None = None
@@
 class UpsertUserResponse(BaseModel):
     id: str
     email: str
-    name: str
+    name: str | None = None
     approved: bool
     domain: str
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@agent/server.py` around lines 1976 - 1987, UpsertUserRequest and
UpsertUserResponse currently force name and image to non-null str causing 422
when OAuth returns null; update the models (UpsertUserRequest.name,
UpsertUserRequest.image and UpsertUserResponse.name, UpsertUserResponse.image)
to allow nullable values by using Optional[str] with a default of None so the
/dashboard/auth/upsert-user flow accepts and returns null values instead of
failing validation.



class CheckUserResponse(BaseModel):
approved: bool
domain: str
email: str


@app.post("/dashboard/auth/upsert-user")
async def dashboard_auth_upsert_user(body: UpsertUserRequest):
from .db.connection import get_pool

if not body.email or "@" not in body.email:
raise HTTPException(status_code=400, detail="invalid_email")

domain = body.email.rsplit("@", 1)[1].lower()
approved = domain == "2weeks.co"
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The domain name is hardcoded here and in the frontend. It should be moved to an environment variable to make the application more configurable and avoid duplication across the codebase.

Suggested change
approved = domain == "2weeks.co"
approved = domain == os.getenv("ALLOWED_DOMAIN", "2weeks.co")


pool = await get_pool()
async with pool.acquire() as conn:
row = await conn.fetchrow(
"""
INSERT INTO users (email, name, image, approved, domain)
VALUES ($1, $2, $3, $4, $5)
ON CONFLICT (email) DO UPDATE
SET name = EXCLUDED.name,
image = EXCLUDED.image,
last_login_at = now()
RETURNING id, email, name, approved, domain
""",
body.email,
body.name,
body.image,
approved,
domain,
)

return UpsertUserResponse(
id=row["id"],
email=row["email"],
name=row["name"],
approved=row["approved"],
domain=row["domain"],
)


@app.get("/dashboard/auth/check-user")
async def dashboard_auth_check_user(email: str):
from .db.connection import get_pool

if not email or "@" not in email:
raise HTTPException(status_code=400, detail="invalid_email")

pool = await get_pool()
async with pool.acquire() as conn:
row = await conn.fetchrow(
"SELECT approved, domain, email FROM users WHERE email = $1",
email,
)
Comment on lines +2020 to +2058
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

users 테이블 스키마와 이 SQL이 맞지 않아서 첫 로그인부터 500이 납니다.

agent/db/connection.pyensure_users_table()는 현재 id, email, created_at, metadata만 만들고 있는데, 여기서는 name, image, approved, domain, last_login_at 컬럼을 바로 읽고 씁니다. 그래서 /dashboard/auth/upsert-user/dashboard/auth/check-user는 배포 직후 undefined column으로 깨집니다. 릴리즈 전에 스키마 마이그레이션을 먼저 맞추거나, 이 값을 metadata에 저장하는 방식으로 일관되게 바꿔야 합니다.

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

In `@agent/server.py` around lines 2020 - 2058, The users table schema created by
ensure_users_table() is missing columns (name, image, approved, domain,
last_login_at) that the upsert-user flow and dashboard_auth_check_user expect;
either update ensure_users_table() to add these columns with appropriate
types/defaults (e.g., name text, image text, approved boolean default false,
domain text, last_login_at timestamptz) or modify the upsert and check handlers
to persist/read those values inside the existing metadata JSON column instead of
direct columns; locate ensure_users_table in agent/db/connection.py and the
handlers used by /dashboard/auth/upsert-user and dashboard_auth_check_user to
apply the chosen consistent schema change.


if row is None:
raise HTTPException(status_code=404, detail="user_not_found")

return CheckUserResponse(
approved=row["approved"],
domain=row["domain"],
email=row["email"],
)
Comment on lines +2008 to +2067
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

사용자 저장/조회 SQL은 라우트 밖으로 빼는 편이 좋겠습니다.

이번 변경은 get_pool()과 SQL이 HTTP 핸들러 안에 직접 들어와서 인증 정책과 저장 규칙이 라우트 계층에 묶였습니다. 전용 저장소 계층이나 ResultStore 추상화로 옮겨 두는 쪽이 이후 정책 변경과 테스트에 안전합니다.

As per coding guidelines agent/server.py: "Use ResultStore abstractions; do not scatter direct DB logic through route handlers".

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

In `@agent/server.py` around lines 2008 - 2067, The route handlers
dashboard_auth_upsert_user and dashboard_auth_check_user currently call
get_pool() and contain raw SQL; move this DB logic into the ResultStore
abstraction (e.g., add methods ResultStore.upsert_user(email, name, image,
approved, domain) returning the row and ResultStore.get_user_by_email(email)
returning the row or None), replace the SQL in server.py with calls to those
ResultStore methods, and inject/obtain ResultStore in the handlers instead of
get_pool(); keep request/response validation (UpsertUserRequest,
UpsertUserResponse, CheckUserResponse) in place and update any tests to use the
ResultStore mock/stub.



if __name__ == "__main__":
uvicorn.run(
"agent.server:app",
Expand Down
5 changes: 5 additions & 0 deletions web/next.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,11 @@ import type { NextConfig } from "next";

const nextConfig: NextConfig = {
output: "standalone",
images: {
remotePatterns: [
{ protocol: "https", hostname: "lh3.googleusercontent.com" },
],
},
};

export default nextConfig;
96 changes: 95 additions & 1 deletion web/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions web/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
"test": "vitest run"
},
"dependencies": {
"@auth/core": "^0.41.0",
"@dnd-kit/core": "^6.3.1",
"@dnd-kit/sortable": "^10.0.0",
"@dnd-kit/utilities": "^3.2.2",
Expand All @@ -19,6 +20,7 @@
"framer-motion": "^12.37.0",
"lucide-react": "^0.577.0",
"next": "16.1.7",
"next-auth": "^5.0.0-beta.30",
"radix-ui": "^1.4.3",
"react": "19.2.4",
"react-dom": "19.2.4",
Expand Down
3 changes: 3 additions & 0 deletions web/src/app/api/auth/[...nextauth]/route.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import { handlers } from "@/lib/auth";

export const { GET, POST } = handlers;
Loading
Loading