Skip to content

Feature/atcoder account model#3328

Merged
KATO-Hiro merged 6 commits intostagingfrom
feature/atcoder-account-model
Mar 29, 2026
Merged

Feature/atcoder account model#3328
KATO-Hiro merged 6 commits intostagingfrom
feature/atcoder-account-model

Conversation

@KATO-Hiro
Copy link
Copy Markdown
Collaborator

@KATO-Hiro KATO-Hiro commented Mar 29, 2026

#3324 に関連した単体・e2eテストの追加やレビューツールの指摘事項のうち必要だと判断したものを修正していただきました

Summary by CodeRabbit

リリースノート

  • New Features

    • AtCoderアカウント認証機能を強化し、専用のサービスに統合しました
    • 投票機能をAtCoderアカウント認証済みユーザーのみに制限しました
    • ユーザー認証状態の確認フローを改善しました
  • Bug Fixes

    • 環境設定の管理を改善し、APIエンドポイントを動的に指定できるようにしました
  • Tests

    • ユーザー編集ページの自動テストを追加しました

river0525 and others added 6 commits March 28, 2026 15:26
…or user edit page

Unit tests cover:
- generate(): returns sha256 code, calls upsert with correct payloads
- validate(): no account, empty code, API mismatch, API confirm (update DB),
  non-OK response, invalid JSON, missing CONFIRM_API_URL env var
- reset(): calls deleteMany with correct userId

E2E tests cover:
- unauthenticated user redirected to /login
- logged-in guest can access /users/edit
- 基本情報 tab visible by default
- アカウント削除 tab not shown for guest user

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- encodeURIComponent(handle) to prevent URL parameter injection
- validate action: return red message on validation failure
- atcoder_verification: deleteMany -> delete for 1:1 relation
- compose.yaml: use ${CONFIRM_API_URL:-<default>} with fallback; add .env.example
- users/[username]/+page.server.ts: remove unnecessary ?. and type casts after null check
- +page.svelte: replace unused openAtCoderTab prop with explanatory comment
- E2E: move page.goto to beforeEach; use getByRole textbox for username assertion;
  add aria-selected assertion for active tab; skip: migration SQL already applied,
  $env/static/private unsupported in this tsconfig setup, schema unique handle deferred
- test: fix test name 'returns false' -> 'throws' for non-OK API response;
  add delete mock alongside deleteMany

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- +page.server.ts: add session auth check to generate/validate/reset actions
- atcoder_verification.ts: revert delete -> deleteMany (delete throws for missing record)
- compose.yaml: remove hardcoded default URL from CONFIRM_API_URL
- .env.example: replace real URL with placeholder
- E2E: remove redundant 'can view' test; add timeout to toHaveAttribute;
  wait for page stabilization before not.toBeVisible; use SHORT_TIMEOUT for absence assertion
- test: update reset test description to explain why deleteMany is used

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- atcoder_verification.ts: add Array.isArray guard before .some() call
- atcoder_verification.ts: remove username from error message (PII)
- +page.server.ts: extract requireSelf() helper to remove auth check duplication
- e2e: use regex /\/login/ for URL assertion (tolerates query params)
- compose.yaml: add comment describing CONFIRM_API_URL

Skipped (with reason):
- migration SQL COALESCE: already applied, cannot modify
- is_validated null fallback: intentional, declared as boolean|null in app.d.ts
- ERD.md camelCase: correct since Prisma uses camelCase column names without @Map
- $env/static/private: tsconfig include override prevents ambient.d.ts resolution

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- +page.server.ts: add null check for username from formData (BAD_REQUEST)
- atcoder_verification.ts: include HTTP status in error message
- atcoder_verification.ts: use Error cause option to preserve original error
- compose.yaml: use :? syntax to fail-fast when CONFIRM_API_URL is unset
- E2E: wait for 基本情報 tab in beforeEach for page stability;
  simplify aria-selected assertion with SHORT_TIMEOUT
- test: remove unused delete mock; update error message assertion

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Add COALESCE for atcoder_validation_code in migration to guard against NULL
- Validate atcoder_username in generate action before passing to service
- Use safe fallback (?? '') for atcoder_username in reset action return value
- Use SHORT_TIMEOUT for username textbox assertion in E2E (page already stable)
- Remove redundant beforeEach stabilization assertion from deletion tab test
- Add missing test: validate propagates error when db lookup fails

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 29, 2026

📝 Walkthrough

Walkthrough

AtCoderアカウント管理をユーザーモデルから独立したAtCoderAccountモデルへ分離し、検証サービスを実装。外部APIとの連携、投票機能への検証ゲートを追加、環境構成と包括的なテストで整備。

Changes

Cohort / File(s) Summary
環境構成
.env.example, compose.yaml
CONFIRM_API_URLを環境変数化;ハードコード排除
AtCoderアカウント検証サービス
src/features/account/services/atcoder_verification.ts, .test.ts
新規サービスで検証コード生成・外部API確認・リセット実装;URLエンコード追加、エラーハンドリング強化、配列検証ロジック追加
データアクセス層
src/lib/services/users.ts
getUser/getUserByIdatCoderAccountリレーション自動インクルード;updateValicationCode削除
サーバーアクション(編集ページ)
src/routes/users/edit/+page.server.ts
requireSelfヘルパー追加で認証検証集約;generate/validate/resetで入力検証・エラーハンドリング強化
プロフィール表示ページ
src/routes/users/[username]/+page.server.ts
非nullable型の直接参照に変更;型アサーション排除
UI更新
src/routes/users/edit/+page.svelte
openAtCoderTabプロパティ削除(再有効化時の準備)
データベース移行
prisma/migrations/20260328002556_split_atcoder_account/migration.sql
AtCoder関連フィールドをuserからatcoder_accountへ移行;nullコアレス処理追加
計画ドキュメント
docs/dev-notes/2026-03-27/atcoder-account-model/plan.md, atcoder-verified-voting/plan.md
スキーマ分離の詳細設計、投票制限仕様、UI実装ロードマップを記述
E2Eテスト
e2e/user_edit.spec.ts
未認証リダイレクト、タブ表示状態、ユーザー名表示、アカウント削除タブ非表示を検証

Sequence Diagram(s)

sequenceDiagram
    actor User
    participant Client as ブラウザ<br/>(/users/edit)
    participant Server as サーバー<br/>アクション
    participant DB as Prisma<br/>データベース
    participant API as 外部API<br/>(確認エンドポイント)

    User->>Client: AtCoder検証開始<br/>(generate)
    Client->>Server: username,<br/>atcoder_username送信
    Server->>DB: SHA256ハッシュ生成<br/>→ atCoderAccount.upsert
    DB-->>Server: validationCode返却
    Server-->>Client: コード表示<br/>(ユーザーが外部で検証)

    User->>Client: 検証完了(validate)
    Client->>Server: username送信
    Server->>DB: atCoderAccountクエリ
    DB-->>Server: validationCode取得
    Server->>API: handle+code確認<br/>URL: CONFIRM_API_URL?handle=...
    API-->>Server: contents配列で確認
    alt 検証成功
        Server->>DB: isValidated: true更新
        DB-->>Server: ✓
        Server-->>Client: 検証完了
    else 検証失敗
        Server-->>Client: false返却
    end
Loading

予定されている次フェーズ

  • 投票制限機能: is_validatedに基づき未認証ユーザーを403でブロック;UI側で誘導表示
  • タブ開閉制御: $derivedで条件制御(サーバーdata / form変更をリアクティブに反映)
  • バリデーション形式削除: AtCoderUserValidationForm.svelteから$bindable除去→ローカル$state

推定レビュー時間

🎯 4 (Complex) | ⏱️ ~60 minutes

理由: スキーマ分離・外部API連携・複数箇所の構造変更で高密度;異種変更(DB移行、新サービス、複数ページのアクション、UIプロップ伝播)で各セクション独立評価要


✨ AtCoderの検証、アカウント分離で整理整頓〜 🎯
投票も認証済みユーザーのみへ制限〜
外部APIとの握手も確かに、セキュアに 🔐

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% 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 PR タイトルは AtCoder アカウントモデルの新機能実装という主要な変更を的確に反映しており、簡潔で明確です。

✏️ 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 feature/atcoder-account-model

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
Contributor

@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: 4

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (4)
src/routes/users/[username]/+page.server.ts (1)

33-35: 🧹 Nitpick | 🔵 Trivial

軽微: 不要なオプショナルチェイニング

Line 22で!userチェック済みのため、Line 34のuser?.iduser.idで十分。

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

In `@src/routes/users/`[username]/+page.server.ts around lines 33 - 35, The call
to getTaskResultsOnlyResultExists uses unnecessary optional chaining — replace
user?.id with user.id in the assignment to taskResultsMap since user is already
null-checked earlier (see the user variable and the existing !user check);
update the call site where taskResultsMap is assigned to call
taskResultService.getTaskResultsOnlyResultExists(user.id).
src/routes/users/edit/+page.svelte (2)

54-64: ⚠️ Potential issue | 🟠 Major

message_type: 'red'のハンドリング漏れ

validateアクションは失敗時にmessage_type: 'red'を返すが、テンプレートは'default''green'のみ対応。エラーメッセージが表示されない。

🐛 修正案
 {:else if message_type === 'green'}
   <Alert color="green">
     <span class="font-medium">Success alert!</span>
     Change a few things up and try submitting again.
   </Alert>
+{:else if message_type === 'red'}
+  <Alert color="red">
+    <span class="font-medium">Error:</span>
+    {message}
+  </Alert>
 {/if}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/routes/users/edit/`+page.svelte around lines 54 - 64, The template
currently only handles message_type 'default' and 'green' so error responses
with message_type === 'red' are never shown; update the Svelte conditional in
+page.svelte to add an {:else if message_type === 'red'} branch that renders
<Alert color="red"> with the error message (use the existing message variable
and a clear label) while keeping the existing 'default' and 'green' branches
intact so validation failures from the validate action are displayed correctly.

13-25: ⚠️ Potential issue | 🟡 Minor

Props定義とサーバーレスポンスの型不一致

+page.server.tsのLine 30でopenAtCoderTabを返却しているが、Propsインターフェースで宣言されていない。型安全性のため、以下のいずれかを実施すべき:

  1. PropsにopenAtCoderTab?: booleanを追加
  2. サーバー側で返却を削除
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/routes/users/edit/`+page.svelte around lines 13 - 25, The Props interface
in +page.svelte is missing the openAtCoderTab field that +page.server.ts returns
(openAtCoderTab at line 30); fix by updating the Props interface to include
openAtCoderTab?: boolean (optional) so the client type matches the server
response, and adjust any consumer code referencing Props or openAtCoderTab to
handle the optional boolean accordingly.
src/routes/users/edit/+page.server.ts (1)

70-78: ⚠️ Potential issue | 🟡 Minor

サービス呼び出しの例外ハンドリング未実装

verificationService.generateがエラーをスローした場合、500エラーがそのまま返る。ユーザーフレンドリーなエラーレスポンスを検討。

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

In `@src/routes/users/edit/`+page.server.ts around lines 70 - 78, Wrap the call to
verificationService.generate(username, atcoder_username) in a try/catch, so if
generate throws you catch the error, log it, and return a user-friendly error
response instead of letting a 500 bubble up; specifically, catch around the
generate call that assigns validationCode, log the caught error, and return an
object such as { success: false, username, atcoder_username, error: "Could not
generate validation code. Please try again." } (preserving the existing
successful-return shape when generate succeeds).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@docs/dev-notes/2026-03-27/atcoder-account-model/plan.md`:
- Around line 269-279: The function confirmWithExternalApi builds the query URL
using the raw handle, which can break for handles with special characters;
update where url is constructed in confirmWithExternalApi to URL-encode the
handle (use encodeURIComponent on the handle variable) when interpolating into
CONFIRM_API_URL so the request is valid for all AtCoder handles; keep the rest
of the logic (fetch, response.ok check, json parsing, and return expression)
unchanged.

In `@docs/dev-notes/2026-03-27/atcoder-verified-voting/plan.md`:
- Around line 563-569: The fenced code block showing the props chain lacks a
language tag (markdownlint MD040); update that block in the markdown to include
a language specifier (e.g., "text" or "plaintext") so the block becomes a
labeled fenced code block; locate the block that lists "page.server.ts
(isAtCoderVerified) → page.svelte (data.isAtCoderVerified) → TaskTable
(isAtCoderVerified prop) → TaskTableBodyCell (isAtCoderVerified prop) →
VotableGrade (isAtCoderVerified prop)" and add the language token after the
opening ``` fence.

In `@src/features/account/services/atcoder_verification.test.ts`:
- Around line 100-108: The tests manually mutate process.env.CONFIRM_API_URL in
beforeEach/afterEach which can cause cross-test interference when Vitest runs
tests in parallel; replace the manual set/delete with vitest's environment
stubbing by using vi.stubEnv to set CONFIRM_API_URL in beforeEach and remove
reliance on delete in afterEach (keep vi.clearAllMocks/vi.unstubAllGlobals as
needed), i.e., call vi.stubEnv({ CONFIRM_API_URL: SAMPLE_API_URL }) in the
beforeEach and stop mutating process.env directly so each test gets an isolated
env.

In `@src/routes/users/edit/`+page.server.ts:
- Around line 44-49: The first auth check returns FORBIDDEN for an
unauthenticated session; change that to the proper 401 UNAUTHORIZED response by
returning fail(UNAUTHORIZED, ...) (or numeric 401) where the code currently does
"if (!session) { return fail(FORBIDDEN, ... ) }"; leave the subsequent
authorization check "if (session.user.username !== username) { return
fail(FORBIDDEN, ... ) }" as-is so unauthenticated vs unauthorized responses are
semantically correct.

---

Outside diff comments:
In `@src/routes/users/`[username]/+page.server.ts:
- Around line 33-35: The call to getTaskResultsOnlyResultExists uses unnecessary
optional chaining — replace user?.id with user.id in the assignment to
taskResultsMap since user is already null-checked earlier (see the user variable
and the existing !user check); update the call site where taskResultsMap is
assigned to call taskResultService.getTaskResultsOnlyResultExists(user.id).

In `@src/routes/users/edit/`+page.server.ts:
- Around line 70-78: Wrap the call to verificationService.generate(username,
atcoder_username) in a try/catch, so if generate throws you catch the error, log
it, and return a user-friendly error response instead of letting a 500 bubble
up; specifically, catch around the generate call that assigns validationCode,
log the caught error, and return an object such as { success: false, username,
atcoder_username, error: "Could not generate validation code. Please try again."
} (preserving the existing successful-return shape when generate succeeds).

In `@src/routes/users/edit/`+page.svelte:
- Around line 54-64: The template currently only handles message_type 'default'
and 'green' so error responses with message_type === 'red' are never shown;
update the Svelte conditional in +page.svelte to add an {:else if message_type
=== 'red'} branch that renders <Alert color="red"> with the error message (use
the existing message variable and a clear label) while keeping the existing
'default' and 'green' branches intact so validation failures from the validate
action are displayed correctly.
- Around line 13-25: The Props interface in +page.svelte is missing the
openAtCoderTab field that +page.server.ts returns (openAtCoderTab at line 30);
fix by updating the Props interface to include openAtCoderTab?: boolean
(optional) so the client type matches the server response, and adjust any
consumer code referencing Props or openAtCoderTab to handle the optional boolean
accordingly.
🪄 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: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: f5bebc60-2017-4eac-ba39-53ea5b5de5c7

📥 Commits

Reviewing files that changed from the base of the PR and between a5ca66c and 960fdb1.

📒 Files selected for processing (11)
  • .env.example
  • compose.yaml
  • docs/dev-notes/2026-03-27/atcoder-account-model/plan.md
  • docs/dev-notes/2026-03-27/atcoder-verified-voting/plan.md
  • e2e/user_edit.spec.ts
  • prisma/migrations/20260328002556_split_atcoder_account/migration.sql
  • src/features/account/services/atcoder_verification.test.ts
  • src/features/account/services/atcoder_verification.ts
  • src/routes/users/[username]/+page.server.ts
  • src/routes/users/edit/+page.server.ts
  • src/routes/users/edit/+page.svelte

Comment on lines +269 to +279
async function confirmWithExternalApi(handle: string, validationCode: string): Promise<boolean> {
const url = `${CONFIRM_API_URL}?user=${handle}`;
const response = await fetch(url);

if (!response.ok) {
throw new Error('Network response was not ok.');
}

const jsonData = await response.json();
return jsonData.contents?.some((item: string) => item === validationCode) ?? false;
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

handle パラメータの URL エンコードが必要です

confirmWithExternalApihandle を URL に直接埋め込んでいますが、特殊文字を含む AtCoder ハンドル名で問題が発生する可能性があります。

🐛 修正案
 async function confirmWithExternalApi(handle: string, validationCode: string): Promise<boolean> {
-  const url = `${CONFIRM_API_URL}?user=${handle}`;
+  const url = `${CONFIRM_API_URL}?user=${encodeURIComponent(handle)}`;
   const response = await fetch(url);
📝 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
async function confirmWithExternalApi(handle: string, validationCode: string): Promise<boolean> {
const url = `${CONFIRM_API_URL}?user=${handle}`;
const response = await fetch(url);
if (!response.ok) {
throw new Error('Network response was not ok.');
}
const jsonData = await response.json();
return jsonData.contents?.some((item: string) => item === validationCode) ?? false;
}
async function confirmWithExternalApi(handle: string, validationCode: string): Promise<boolean> {
const url = `${CONFIRM_API_URL}?user=${encodeURIComponent(handle)}`;
const response = await fetch(url);
if (!response.ok) {
throw new Error('Network response was not ok.');
}
const jsonData = await response.json();
return jsonData.contents?.some((item: string) => item === validationCode) ?? false;
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docs/dev-notes/2026-03-27/atcoder-account-model/plan.md` around lines 269 -
279, The function confirmWithExternalApi builds the query URL using the raw
handle, which can break for handles with special characters; update where url is
constructed in confirmWithExternalApi to URL-encode the handle (use
encodeURIComponent on the handle variable) when interpolating into
CONFIRM_API_URL so the request is valid for all AtCoder handles; keep the rest
of the logic (fetch, response.ok check, json parsing, and return expression)
unchanged.

Comment on lines +563 to +569
```
+page.server.ts (isAtCoderVerified)
→ +page.svelte (data.isAtCoderVerified)
→ TaskTable (isAtCoderVerified prop)
→ TaskTableBodyCell (isAtCoderVerified prop)
→ VotableGrade (isAtCoderVerified prop)
```
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Props チェーン図に言語指定がありません(markdownlint MD040)

静的解析ツールが指摘している通り、fenced code block に言語指定がありません。textplaintext を指定してください。

📝 修正案
-```
+```text
 +page.server.ts (isAtCoderVerified)
   → +page.svelte (data.isAtCoderVerified)
     → TaskTable (isAtCoderVerified prop)
       → TaskTableBodyCell (isAtCoderVerified prop)
         → VotableGrade (isAtCoderVerified prop)
</details>

<details>
<summary>🧰 Tools</summary>

<details>
<summary>🪛 markdownlint-cli2 (0.22.0)</summary>

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

(MD040, fenced-code-language)

</details>

</details>

<details>
<summary>🤖 Prompt for AI Agents</summary>

Verify each finding against the current code and only fix it if needed.

In @docs/dev-notes/2026-03-27/atcoder-verified-voting/plan.md around lines 563 -
569, The fenced code block showing the props chain lacks a language tag
(markdownlint MD040); update that block in the markdown to include a language
specifier (e.g., "text" or "plaintext") so the block becomes a labeled fenced
code block; locate the block that lists "page.server.ts (isAtCoderVerified) →
page.svelte (data.isAtCoderVerified) → TaskTable (isAtCoderVerified prop) →
TaskTableBodyCell (isAtCoderVerified prop) → VotableGrade (isAtCoderVerified
prop)" and add the language token after the opening ``` fence.


</details>

<!-- fingerprinting:phantom:poseidon:ocelot:68485826-7454-4dc8-af0a-21e6f74a6240 -->

<!-- This is an auto-generated comment by CodeRabbit -->

Comment on lines +100 to +108
beforeEach(() => {
vi.clearAllMocks();
process.env.CONFIRM_API_URL = SAMPLE_API_URL;
});

afterEach(() => {
delete process.env.CONFIRM_API_URL;
vi.unstubAllGlobals();
});
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

process.env の操作に関する注意

  • beforeEachprocess.env.CONFIRM_API_URL を設定
  • afterEachdelete している

Vitest の並列実行時に環境変数の競合が発生する可能性があります。vi.stubEnv の使用を検討してください。

♻️ 推奨修正
 beforeEach(() => {
   vi.clearAllMocks();
-  process.env.CONFIRM_API_URL = SAMPLE_API_URL;
+  vi.stubEnv('CONFIRM_API_URL', SAMPLE_API_URL);
 });

 afterEach(() => {
-  delete process.env.CONFIRM_API_URL;
   vi.unstubAllGlobals();
 });
📝 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
beforeEach(() => {
vi.clearAllMocks();
process.env.CONFIRM_API_URL = SAMPLE_API_URL;
});
afterEach(() => {
delete process.env.CONFIRM_API_URL;
vi.unstubAllGlobals();
});
beforeEach(() => {
vi.clearAllMocks();
vi.stubEnv('CONFIRM_API_URL', SAMPLE_API_URL);
});
afterEach(() => {
vi.unstubAllGlobals();
});
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/features/account/services/atcoder_verification.test.ts` around lines 100
- 108, The tests manually mutate process.env.CONFIRM_API_URL in
beforeEach/afterEach which can cause cross-test interference when Vitest runs
tests in parallel; replace the manual set/delete with vitest's environment
stubbing by using vi.stubEnv to set CONFIRM_API_URL in beforeEach and remove
reliance on delete in afterEach (keep vi.clearAllMocks/vi.unstubAllGlobals as
needed), i.e., call vi.stubEnv({ CONFIRM_API_URL: SAMPLE_API_URL }) in the
beforeEach and stop mutating process.env directly so each test gets an isolated
env.

Comment on lines +44 to +49
if (!session) {
return fail(FORBIDDEN, { message: 'Not authenticated.' });
}
if (session.user.username !== username) {
return fail(FORBIDDEN, { message: 'Not authorized.' });
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

HTTPステータスコードのセマンティクス

  • 未認証(Line 45): 401 UNAUTHORIZEDが適切
  • 認可エラー(Line 48): 403 FORBIDDENで正しい

現状は両方403だが、クライアント側の挙動に影響しなければ許容範囲。

♻️ 修正案
+import { BAD_REQUEST, FORBIDDEN, UNAUTHORIZED } from '$lib/constants/http-response-status-codes';

 async function requireSelf(
   locals: App.Locals,
   username: string,
 ): Promise<ReturnType<typeof fail> | null> {
   const session = await locals.auth.validate();
   if (!session) {
-    return fail(FORBIDDEN, { message: 'Not authenticated.' });
+    return fail(UNAUTHORIZED, { message: 'Not authenticated.' });
   }
📝 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
if (!session) {
return fail(FORBIDDEN, { message: 'Not authenticated.' });
}
if (session.user.username !== username) {
return fail(FORBIDDEN, { message: 'Not authorized.' });
}
import { BAD_REQUEST, FORBIDDEN, UNAUTHORIZED } from '$lib/constants/http-response-status-codes';
async function requireSelf(
locals: App.Locals,
username: string,
): Promise<ReturnType<typeof fail> | null> {
const session = await locals.auth.validate();
if (!session) {
return fail(UNAUTHORIZED, { message: 'Not authenticated.' });
}
if (session.user.username !== username) {
return fail(FORBIDDEN, { message: 'Not authorized.' });
}
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/routes/users/edit/`+page.server.ts around lines 44 - 49, The first auth
check returns FORBIDDEN for an unauthenticated session; change that to the
proper 401 UNAUTHORIZED response by returning fail(UNAUTHORIZED, ...) (or
numeric 401) where the code currently does "if (!session) { return
fail(FORBIDDEN, ... ) }"; leave the subsequent authorization check "if
(session.user.username !== username) { return fail(FORBIDDEN, ... ) }" as-is so
unauthenticated vs unauthorized responses are semantically correct.

Copy link
Copy Markdown
Collaborator Author

@KATO-Hiro KATO-Hiro left a comment

Choose a reason for hiding this comment

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

LGTM
マージします

@KATO-Hiro KATO-Hiro merged commit 8757704 into staging Mar 29, 2026
3 checks passed
@KATO-Hiro KATO-Hiro deleted the feature/atcoder-account-model branch March 29, 2026 04:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants