Skip to content

fix(ci): alembic migration ガードの誤検知を解消 — ファイル名でなくスキーマ影響で判定 - #995

Merged
milechy merged 1 commit into
mainfrom
fix/ci-alembic-false-positive
Jul 17, 2026
Merged

milechy merged 1 commit into
mainfrom
fix/ci-alembic-false-positive

Conversation

@milechy

@milechy milechy commented Jul 17, 2026

Copy link
Copy Markdown
Owner

問題

models 変更時 migration 必須化 は 「models.py というファイルが変更されたか」だけで判定しており、中身を見ていませんでした。docstring / 定数 / ヘルパ関数の変更でも FAIL します。

PR #994 で顕在化しました。PHASE_1_ALLOWED_RISK_MODES の env 由来化(列・制約・テーブルの差分ゼロ)が落ちています:

$ git diff origin/main...fix/pendle-convert-api -- backend/app/auth/models.py \
    | grep -iE "mapped_column|Column\(|__tablename__|ALTER TABLE|CheckConstraint|Index\("
  → 出力なし

「赤いのが常態」は本物の schema drift を見逃す土壌になります。本スクリプト自身の設計メモも「例外(コメントのみ変更など)は未実装。まず厳密に。誤検知が問題化したら例外追加。」と書いており、その想定どおりになったので実装します。

修正

migration が無い models 変更について、差分の追加/削除行に SQLAlchemy のスキーマ関連字句があるかを見ます:

mapped_column | Column( | __tablename__ | __table_args__ | CheckConstraint |
UniqueConstraint | PrimaryKeyConstraint | ForeignKey | Index( | relationship( |
server_default | nullable= | primary_key | autoincrement | Mapped[ |
ALTER TABLE | CREATE TABLE | DROP COLUMN | sa.
  • 1つでもあれば → 従来どおり FAIL(migration 必須)
  • 1つも無ければ → skip(判定根拠をログ出力)

fail-closed 設計です。コメント内の一致でも FAIL します。「スキーマ変更かもしれない」は全て FAIL 側に倒し、確実に無関係なときだけ通します。

[skip-alembic-check] による description バイパスは実装しません — スクリプトが自分でグリーンにできる抜け道を作らず、人間の明示的な判断を残すためです。

検証(実データ)

過去に実際に backend/app/auth/models.py を変更したコミット20件で判定を確認しました。

本物のスキーマ変更は全て FAIL 判定:

コミット hits 判定
Phase-D D5b(aggressive_ack_at/_version 列追加) 6 FAIL ✅
staging-v4 月額決済(Stripe 列追加) 6 FAIL ✅
顧客 PII フィールドレベル暗号化 3 FAIL ✅
per-user privy_wallet_id 追加 1 FAIL ✅
#994 の env 化のみ 0 pass ✅(期待どおり)

bash -n 構文 OK。shellcheck の新規指摘なし(既存行の style SC2001 のみ)。

併せて修正: エラーメッセージが危険な手順を案内していた

旧メッセージは alembic revision --autogenerate を案内していましたが、本リポジトリでは使用禁止です。alembic/env.py が全モデルを import しておらず Base.metadata が不完全なため、実在するテーブルへの DROP を誤生成します(memory: project_alembic_envpy_incomplete_model_imports)。CI の指示に従うと本番を壊すため、「手書きで追加」+ 禁止理由の明示に変更しました。

影響

CI ガードのみ。アプリケーションコードへの変更なし。

🤖 Generated with Claude Code

https://claude.ai/code/session_01Edj3hjkB6XrjYezmARpQEq

## 問題

`models 変更時 migration 必須化` は「`models.py` というファイルが変更されたか」だけで判定しており、
**中身を見ていなかった**。そのため docstring / 定数 / ヘルパ関数の変更でも FAIL する。

PR #994 で顕在化: `PHASE_1_ALLOWED_RISK_MODES` を env 由来化した変更(`_allowed_risk_modes_from_env()`
の追加のみ・**列/制約/テーブルの差分ゼロ**)が落ちた。

「赤いのが常態」は本物の schema drift を見逃す土壌になる。本スクリプト自身の設計メモも
**「例外(コメントのみ変更など)は未実装。まず厳密に。誤検知が問題化したら例外追加。」**
と書いており、その想定どおり誤検知が問題化したので例外を実装する。

## 修正

migration が無い models 変更について、差分の**追加/削除行**に SQLAlchemy のスキーマ関連字句が
1 つでもあるかを見る:

```
mapped_column | Column( | __tablename__ | __table_args__ | CheckConstraint |
UniqueConstraint | PrimaryKeyConstraint | ForeignKey | Index( | relationship( |
server_default | nullable= | primary_key | autoincrement | Mapped[ |
ALTER TABLE | CREATE TABLE | DROP COLUMN | sa.
```

- 1 つでもあれば → **従来どおり FAIL**(migration 必須)
- 1 つも無ければ → skip(判定根拠をログに出す)

**fail-closed 設計**: コメント内の一致でも FAIL する。「スキーマ変更かもしれない」は全て FAIL 側に
倒し、確実に無関係なときだけ通す。PR description による bypass(`[skip-alembic-check]`)は
**実装しない** — スクリプトが自分でグリーンにできる抜け道を作らず、人間の明示的な判断を残す。

## 検証(実データ)

過去に実際に `backend/app/auth/models.py` を変更したコミット 20 件で判定を確認:

- **本物のスキーマ変更は全て FAIL 判定**:
  - Phase-D D5b(`aggressive_ack_at` / `aggressive_ack_version` 列追加)→ hits=6
  - per-user `privy_wallet_id` 追加 → hits=1
  - 顧客 PII フィールドレベル暗号化 → hits=3
  - staging-v4 月額決済(Stripe 列追加)→ hits=6
- **#994 の env 化のみ** → hits=0 = pass(期待どおり)

`bash -n` 構文 OK。shellcheck の指摘は既存行の style (SC2001) のみで新規なし。

## 併せて修正: エラーメッセージが危険な手順を案内していた

旧メッセージは `alembic revision --autogenerate` を案内していたが、**本リポジトリでは使用禁止**。
`alembic/env.py` が全モデルを import しておらず `Base.metadata` が不完全なため、実在するテーブルへの
DROP を誤生成する(memory: `project_alembic_envpy_incomplete_model_imports`)。
指示に従うと本番を壊すため、「手書きで追加」+ 禁止理由の明示に変更した。

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Edj3hjkB6XrjYezmARpQEq
@github-actions

Copy link
Copy Markdown

🛡️ Path Access Control

⚠️ 警告 (要確認)

  • ⚠️ main への直接マージ: fix/ci-alembic-false-positive → staging 経由が推奨
  • ⚠️ main への直接マージ: fix/ci-alembic-false-positive → staging 経由が推奨
  • ⚠️ main への直接マージ: fix/ci-alembic-false-positive → staging 経由が推奨
  • ⚠️ main への直接マージ: fix/ci-alembic-false-positive → staging 経由が推奨
  • ⚠️ main への直接マージ: fix/ci-alembic-false-positive → staging 経由が推奨
  • ⚠️ main への直接マージ: fix/ci-alembic-false-positive → staging 経由が推奨
  • ⚠️ main への直接マージ: fix/ci-alembic-false-positive → staging 経由が推奨
  • ⚠️ main への直接マージ: fix/ci-alembic-false-positive → staging 経由が推奨
  • ⚠️ main への直接マージ: fix/ci-alembic-false-positive → staging 経由が推奨
  • ⚠️ main への直接マージ: fix/ci-alembic-false-positive → staging 経由が推奨
  • ⚠️ main への直接マージ: fix/ci-alembic-false-positive → staging 経由が推奨
  • ⚠️ main への直接マージ: fix/ci-alembic-false-positive → staging 経由が推奨
  • ⚠️ main への直接マージ: fix/ci-alembic-false-positive → staging 経由が推奨
  • ⚠️ main への直接マージ: fix/ci-alembic-false-positive → staging 経由が推奨
  • ⚠️ main への直接マージ: fix/ci-alembic-false-positive → staging 経由が推奨
  • ⚠️ main への直接マージ: fix/ci-alembic-false-positive → staging 経由が推奨

@github-actions

Copy link
Copy Markdown

🤖 Codex Review

Security Check: ✅ PASS
Test Coverage: ✅ OK

No issues found. ✨

@milechy
milechy merged commit 9fa0767 into main Jul 17, 2026
16 checks passed
@milechy
milechy deleted the fix/ci-alembic-false-positive branch July 17, 2026 11:56
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.

1 participant